In my view, Command is actually performing this, the problem is when it is used with sh -c, meaning the Command runs the interpreter. I think one should never actually run the interpreter, rather than taking care of the parameters.

Hello, @alexandruradovici
I would agree that running the "sh" interpreter enables the command injection here. There are two general approaches: first, you would sanitize the arguments to prevent anything malicious from happening. Second, you use a less powerful command, such as invoking 'ls' directly without 'sh', or using fs::read_dir()...both compliant solutions use this second technique.

Are you suggesting a change in the wording, or in the code examples?

My suggestion is to use a more powerful command and advise agains running the interpreter (sh or equivalent) with any command. I would not relate it to the arguments, but to the what kind of commands should or should not be ran.

I wonder how we should do this.

Something that seems simple enough to do is a deny-list:

• In the worst case, we would be denying non-injection-allowing programs. This is fine, as a failure on our labeling would not lead to a safety hazard.
• An allow list would have the opposite problem: if we fail to account for injectability in a program we mark as "okay", we would be giving the 👍🏻 to a safety hazard. That's why I think an allow list would probably be bad to have.
• For all other command-line programs, we give the user some criteria by which they might be able to know if running it is okay or not (in a non-ambiguous way)

That criteria could be something along the lines of...

• "The program must not be turing-complete given its input": this would deny all possible injection mechanisms, but some valid programs could be denied as well. For example, a program that does arbitrary computation inside its own "isolated world" would be fine to run, but would not be allowed by this rule.
• "The program must not be able to transitively spawn an arbitrary number of other processes": this would be a lot harder to assess, but feels a lot closer to what we want to avoid.

I would not relate it to the arguments, but to the what kind of commands should or should not be ran.

Ultimately, This is a Rust rule. I don't want the rule to blacklist some commands, because we can't make that blacklist complete. Too many shell commands + too many OS platforms. Creating a whitelist of 'benign' commands is safer, but no less Herculean a problem.

You also have the problem of when arguments are tokenized on Windows (see the BatBadBut vul).

We could try to limit commands in terms of Turing-completeness, but that also sounds like a huge rabbit hole. Which of these commands would we be forbidden from running: bash, python, sqlite3, jq, gcc ?

The only other option I see would be to forbid Commands entirely (at least for security-critical code). Allow exceptions only on an individual basis. Or allow Commands only in unsafe code.

I would not relate it to the arguments, but to the what kind of commands should or should not be ran.

Ultimately, This is a Rust rule. I don't want the rule to blacklist some commands, because we can't make that blacklist complete. Too many shell commands + too many OS platforms. Creating a whitelist of 'benign' commands is safer, but no less Herculean a problem.

You also have the problem of when arguments are tokenized on Windows (see the BatBadBut vul).

We could try to limit commands in terms of Turing-completeness, but that also sounds like a huge rabbit hole. Which of these commands would we be forbidden from running: bash, python, sqlite3, jq, gcc ?

The only other option I see would be to forbid Commands entirely (at least for security-critical code). Allow exceptions only on an individual basis. Or allow Commands only in unsafe code.

As an addition to this, I do agree that creating a whitelist is much more feasible, especially when taking into accounts commands such as python3, ruby, gcc, etc.

The way I think should be handled is to pair it with an appropriate enum group of allowed commands (and their arguments). I think this could also be beneficial in case a program needs to run specific commands in parts of the code so that these could be somewhat module-locked (ie. python3 can be run exclusively in a test module paired with #[cfg(test)] or one where you need to externally call Python without using PyO3 or equivalent).

Unfortunately which commands should be whitelisted will mostly rely on a case-by-case basis so we cannot really outright suggest "accept this and not this", but within a program's scope the team in charge of designing and building said program should have mapped adequately which commands they'll need to call; the advantage of grouping them in an enum will also make verifying them through match blocks a bit easier than manually scanning the whole codebase.

Following the examples from CERT J, it would also be good to show the corresponding non-compliant code in Windows for the final guideline

I've tried to make the code more platform-independent. AFAICT the Windows-specific code would be identical, except s/ls/dir/g;.

I would not relate it to the arguments, but to the what kind of commands should or should not be ran.

Ultimately, This is a Rust rule. I don't want the rule to blacklist some commands, because we can't make that blacklist complete. Too many shell commands + too many OS platforms. Creating a whitelist of 'benign' commands is safer, but no less Herculean a problem.

You also have the problem of when arguments are tokenized on Windows (see the BatBadBut vul).

We could try to limit commands in terms of Turing-completeness, but that also sounds like a huge rabbit hole. Which of these commands would we be forbidden from running: bash, python, sqlite3, jq, gcc ?

The only other option I see would be to forbid Commands entirely (at least for security-critical code). Allow exceptions only on an individual basis. Or allow Commands only in unsafe code.

As an addition to this, I do agree that creating a whitelist is much more feasible, especially when taking into accounts commands such as python3, ruby, gcc, etc.

The way I think should be handled is to pair it with an appropriate enum group of allowed commands (and their arguments). I think this could also be beneficial in case a program needs to run specific commands in parts of the code so that these could be somewhat module-locked (ie. python3 can be run exclusively in a test module paired with #[cfg(test)] or one where you need to externally call Python without using PyO3 or equivalent).

Unfortunately which commands should be whitelisted will mostly rely on a case-by-case basis so we cannot really outright suggest "accept this and not this", but within a program's scope the team in charge of designing and building said program should have mapped adequately which commands they'll need to call; the advantage of grouping them in an enum will also make verifying them through match blocks a bit easier than manually scanning the whole codebase.