S4721 - Fixed Command injection is still flagged

[gian1200 (Giancarlo Calderón Cárdenas)]

Environment:

• Community Edition, Version 10.1 (build 73491)
• Local Sonar on pod with Podman and WSL

Description:
execSync with "shell": false is still reported

Code:

import { execSync } from "child_process";
import log4js from "log4js";
const logger = log4js.getLogger("scripts.utils");
logger.level = "INFO";

export const ejecutarComando = (cli_command) => {
	try {
		logger.info(`Comando: ${cli_command}`);
		execSync(cli_command, {
			"encoding": "utf8",
			"shell": false,
			"stdio": "inherit"
		});
		return true;
	} catch (error) {
		logger.error(error.message);
		return false;
	}
};

[ilia]

Hello Giancarlo,

Thanks for raising this issue.

Contrary to spawn or execFile that accept a boolean value for the shell option disabling it, for execSync, the shell option only allows to provide a custom shell. Therefore, we raise a security hotspot when execSync() is used.

As you can see in the rule definition, exec() and execSync() are flagged as sensitive regardless of the shell option.

Best,
Ilia

[gian1200 (Giancarlo Calderón Cárdenas)]

I see… So, in my scenario, changing execSync with spawnSync (with the right editions) should be the proper action, right?

As a suggestion it’d be nice to add a few more options in the Sensitive Code Example section. It’s not quite instantly clear that execSync with "shell": false is also sensitive.

Now that you mention it, it make sense, because shell’s default value is false.

However, if “false” is the default value for all the methods, then no explicit definition of “shell” should be compliant, too (for spawnSync).

[gian1200 (Giancarlo Calderón Cárdenas)]

In case someone else comes here, at the end I solved my issue with cross-spawn. Something like:

import spawn from "cross-spawn";

...

const commandArray = cli_command.split(" ");
const result = spawn.sync(commandArray.shift(), commandArray, {
	"encoding": "utf8",
	"shell": false,
	"stdio": "inherit"
});

if (result.error) {
	throw result.error;
}

...