fix(benchmark): fix benchmark shell command built from environment values

[odaysec]

graphql-js/benchmark/benchmark.js

Lines 32 to 33 in ba4b411

	function exec(command, options = {}) {
	const result = cp.execSync(command, {

graphql-js/benchmark/benchmark.js

Lines 65 to 79 in ba4b411

	exec(`cp -R ${localDir('benchmark')} ${projectPath}`);
	return { revision, projectPath };
	});
	function prepareNPMPackage(revision) {
	if (revision === LOCAL) {
	const repoDir = localDir();
	const archivePath = path.join(tmpDir, 'graphql-local.tgz');
	fs.renameSync(buildNPMArchive(repoDir), archivePath);
	return archivePath;
	}
	// Returns the complete git hash for a given git revision reference.
	const hash = exec(`git rev-parse "${revision}"`);

graphql-js/benchmark/benchmark.js

Lines 89 to 90 in ba4b411

	exec(`git archive "${hash}" | tar -xC "${repoDir}"`);
	exec('npm --quiet ci --ignore-scripts', { cwd: repoDir });

Fix the issue should avoid passing dynamically constructed shell commands to cp.execSync. Instead, we can use cp.execFileSync, which allows us to pass the command and its arguments as separate parameters. This approach ensures that the shell does not interpret special characters in the arguments, mitigating the risk of command injection or misinterpretation.

1. Replace the exec function to use cp.execFileSync instead of cp.execSync.
2. Update all calls to exec to pass the command and arguments as separate parameters.

Recommendation

If possible, use hard-coded string literals to specify the shell command to run, and provide the dynamic arguments to the shell command separately to avoid interpretation by the shell. Alternatively, if the shell command must be constructed dynamically, then add code to ensure that special characters in environment values do not alter the shell command unexpectedly.

[linux-foundation-easycla]

• ✅login: odaysec / (d398d58)

The committers listed above are authorized under a signed CLA.

[benjie]

Can you explain how hash and repoDir variables could be supplied with malicious values that would cause command injection without the person who is running the code having done it deliberately to themselves?