Implement flak8-bandit shell injection rules

_typos.toml

@@ -5,3 +5,4 @@ extend-exclude = ["snapshots", "black"]
 trivias = "trivias"
 hel = "hel"
 whos = "whos"
+spawnve = "spawnve"

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

@@ -2649,6 +2649,16 @@ where
                         self, func, args, keywords,
                     );
                 }
+                if self.settings.rules.any_enabled(&[
+                    Rule::SubprocessWithoutShellEqualsTrue,
+                    Rule::SubprocessPopenWithShellEqualsTrue,
+                    Rule::CallWithShellEqualsTrue,
+                    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::CallWithShellEqualsTrue,
+        (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::CallWithShellEqualsTrue,
+    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/mod.rs

@@ -18,6 +18,7 @@ mod tests {
 
     #[test_case(Rule::Assert, Path::new("S101.py"); "S101")]
     #[test_case(Rule::BadFilePermissions, Path::new("S103.py"); "S103")]
+    #[test_case(Rule::CallWithShellEqualsTrue, Path::new("S604.py"); "S604")]
     #[test_case(Rule::ExecBuiltin, Path::new("S102.py"); "S102")]
     #[test_case(Rule::HardcodedBindAllInterfaces, Path::new("S104.py"); "S104")]
     #[test_case(Rule::HardcodedPasswordDefault, Path::new("S107.py"); "S107")]
@@ -32,6 +33,11 @@ mod tests {
     #[test_case(Rule::RequestWithoutTimeout, Path::new("S113.py"); "S113")]
     #[test_case(Rule::SnmpInsecureVersion, Path::new("S508.py"); "S508")]
     #[test_case(Rule::SnmpWeakCryptography, Path::new("S509.py"); "S509")]
+    #[test_case(Rule::StartProcessWithAShell, Path::new("S605.py"); "S605")]
+    #[test_case(Rule::StartProcessWithNoShell, Path::new("S606.py"); "S606")]
+    #[test_case(Rule::StartProcessWithPartialPath, Path::new("S607.py"); "S607")]
+    #[test_case(Rule::SubprocessPopenWithShellEqualsTrue, Path::new("S602.py"); "S602")]
+    #[test_case(Rule::SubprocessWithoutShellEqualsTrue, Path::new("S603.py"); "S603")]
     #[test_case(Rule::SuspiciousPickleUsage, Path::new("S301.py"); "S301")]
     #[test_case(Rule::SuspiciousTelnetUsage, Path::new("S312.py"); "S312")]
     #[test_case(Rule::TryExceptContinue, Path::new("S112.py"); "S112")]

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, CallWithShellEqualsTrue, 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;