Implement flak8-bandit shell injection rules

This includes rules S602 - S607.

crates/ruff/resources/test/fixtures/flake8_bandit/S602.py

@@ -0,0 +1,20 @@
+from subprocess import Popen, call, check_call, check_output, run
+
+# Check different Popen wrappers are checked
+Popen("true", shell=True)
+call("true", shell=True)
+check_call("true", shell=True)
+check_output("true", shell=True)
+run("true", shell=True)
+
+# Check values that truthy values are treated as true
+Popen("true", shell=1)
+Popen("true", shell=[1])
+Popen("true", shell={1: 1})
+Popen("true", shell=(1,))
+
+# Check command argument looks unsafe
+var_string = "true"
+Popen(var_string, shell=True)
+Popen([var_string], shell=True)
+Popen([var_string, ""], shell=True)

crates/ruff/resources/test/fixtures/flake8_bandit/S603.py

@@ -0,0 +1,20 @@
+from subprocess import Popen, call, check_call, check_output, run
+
+# Different Popen wrappers are checked
+Popen("true", shell=False)
+call("true", shell=False)
+check_call("true", shell=False)
+check_output("true", shell=False)
+run("true", shell=False)
+
+# Values that falsey values are treated as false
+Popen("true", shell=0)
+Popen("true", shell=[])
+Popen("true", shell={})
+Popen("true", shell=None)
+
+# Unknown values are treated as falsey
+Popen("true", shell=True if True else False)
+
+# No value is also caught
+Popen("true")

crates/ruff/resources/test/fixtures/flake8_bandit/S604.py

@@ -0,0 +1,5 @@
+def foo(shell):
+    pass
+
+
+foo(shell=True)

crates/ruff/resources/test/fixtures/flake8_bandit/S605.py

@@ -0,0 +1,25 @@
+import os
+
+import commands
+import popen2
+
+# Check all shell functions
+os.system("true")
+os.popen("true")
+os.popen2("true")
+os.popen3("true")
+os.popen4("true")
+popen2.popen2("true")
+popen2.popen3("true")
+popen2.popen4("true")
+popen2.Popen3("true")
+popen2.Popen4("true")
+commands.getoutput("true")
+commands.getstatusoutput("true")
+
+
+# Check command argument looks unsafe
+var_string = "true"
+os.system(var_string)
+os.system([var_string])
+os.system([var_string, ""])

crates/ruff/resources/test/fixtures/flake8_bandit/S606.py

@@ -0,0 +1,20 @@
+import os
+
+# Check all shell functions
+os.execl("true")
+os.execle("true")
+os.execlp("true")
+os.execlpe("true")
+os.execv("true")
+os.execve("true")
+os.execvp("true")
+os.execvpe("true")
+os.spawnl("true")
+os.spawnle("true")
+os.spawnlp("true")
+os.spawnlpe("true")
+os.spawnv("true")
+os.spawnve("true")
+os.spawnvp("true")
+os.spawnvpe("true")
+os.startfile("true")

crates/ruff/resources/test/fixtures/flake8_bandit/S607.py

@@ -0,0 +1,44 @@
+import os
+
+# Check all functions
+subprocess.Popen("true")
+subprocess.call("true")
+subprocess.check_call("true")
+subprocess.check_output("true")
+subprocess.run("true")
+os.system("true")
+os.popen("true")
+os.popen2("true")
+os.popen3("true")
+os.popen4("true")
+popen2.popen2("true")
+popen2.popen3("true")
+popen2.popen4("true")
+popen2.Popen3("true")
+popen2.Popen4("true")
+commands.getoutput("true")
+commands.getstatusoutput("true")
+os.execl("true")
+os.execle("true")
+os.execlp("true")
+os.execlpe("true")
+os.execv("true")
+os.execve("true")
+os.execvp("true")
+os.execvpe("true")
+os.spawnl("true")
+os.spawnle("true")
+os.spawnlp("true")
+os.spawnlpe("true")
+os.spawnv("true")
+os.spawnve("true")
+os.spawnvp("true")
+os.spawnvpe("true")
+os.startfile("true")
+
+# Check it does not fail for full paths
+os.system("/bin/ls")
+os.system("./bin/ls")
+os.system(["/bin/ls"])
+os.system(["/bin/ls", "/tmp"])
+os.system(r"C:\\bin\ls")

crates/ruff/src/checkers/ast/mod.rs

@@ -2615,6 +2615,16 @@ where
                         self, func, args, keywords,
                     );
                 }
+                if self.settings.rules.any_enabled(&[
+                    Rule::SubprocessWithoutShellEqualsTrue,
+                    Rule::SubprocessPopenWithShellEqualsTrue,
+                    Rule::AnyOtherFunctionWithShellEqualsTrue,
+                    Rule::StartProcessWithAShell,
+                    Rule::StartProcessWithNoShell,
+                    Rule::StartProcessWithPartialPath,
+                ]) {
+                    flake8_bandit::rules::shell_injection(self, func, args, keywords);
+                }
 
                 // flake8-comprehensions
                 if self.settings.rules.enabled(Rule::UnnecessaryGeneratorList) {

crates/ruff/src/codes.rs

@@ -507,6 +507,12 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Flake8Bandit, "506") => Rule::UnsafeYAMLLoad,
         (Flake8Bandit, "508") => Rule::SnmpInsecureVersion,
         (Flake8Bandit, "509") => Rule::SnmpWeakCryptography,
+        (Flake8Bandit, "602") => Rule::SubprocessPopenWithShellEqualsTrue,
+        (Flake8Bandit, "603") => Rule::SubprocessWithoutShellEqualsTrue,
+        (Flake8Bandit, "604") => Rule::AnyOtherFunctionWithShellEqualsTrue,
+        (Flake8Bandit, "605") => Rule::StartProcessWithAShell,
+        (Flake8Bandit, "606") => Rule::StartProcessWithNoShell,
+        (Flake8Bandit, "607") => Rule::StartProcessWithPartialPath,
         (Flake8Bandit, "608") => Rule::HardcodedSQLExpression,
         (Flake8Bandit, "612") => Rule::LoggingConfigInsecureListen,
         (Flake8Bandit, "701") => Rule::Jinja2AutoescapeFalse,

crates/ruff/src/registry.rs

@@ -446,6 +446,12 @@ ruff_macros::register_rules!(
     rules::flake8_bandit::rules::RequestWithoutTimeout,
     rules::flake8_bandit::rules::SnmpInsecureVersion,
     rules::flake8_bandit::rules::SnmpWeakCryptography,
+    rules::flake8_bandit::rules::SubprocessPopenWithShellEqualsTrue,
+    rules::flake8_bandit::rules::SubprocessWithoutShellEqualsTrue,
+    rules::flake8_bandit::rules::AnyOtherFunctionWithShellEqualsTrue,
+    rules::flake8_bandit::rules::StartProcessWithAShell,
+    rules::flake8_bandit::rules::StartProcessWithNoShell,
+    rules::flake8_bandit::rules::StartProcessWithPartialPath,
     rules::flake8_bandit::rules::SuspiciousEvalUsage,
     rules::flake8_bandit::rules::SuspiciousFTPLibUsage,
     rules::flake8_bandit::rules::SuspiciousInsecureCipherUsage,

crates/ruff/src/rules/flake8_bandit/rules/mod.rs

@@ -22,6 +22,11 @@ pub use request_with_no_cert_validation::{
     request_with_no_cert_validation, RequestWithNoCertValidation,
 };
 pub use request_without_timeout::{request_without_timeout, RequestWithoutTimeout};
+pub use shell_injection::{
+    shell_injection, AnyOtherFunctionWithShellEqualsTrue, StartProcessWithAShell,
+    StartProcessWithNoShell, StartProcessWithPartialPath, SubprocessPopenWithShellEqualsTrue,
+    SubprocessWithoutShellEqualsTrue,
+};
 pub use snmp_insecure_version::{snmp_insecure_version, SnmpInsecureVersion};
 pub use snmp_weak_cryptography::{snmp_weak_cryptography, SnmpWeakCryptography};
 pub use suspicious_function_call::{
@@ -52,6 +57,7 @@ mod jinja2_autoescape_false;
 mod logging_config_insecure_listen;
 mod request_with_no_cert_validation;
 mod request_without_timeout;
+mod shell_injection;
 mod snmp_insecure_version;
 mod snmp_weak_cryptography;
 mod suspicious_function_call;