diff --git a/_typos.toml b/_typos.toml index d2ef48f71b..a14ac7083f 100644 --- a/_typos.toml +++ b/_typos.toml @@ -5,3 +5,4 @@ extend-exclude = ["snapshots", "black"] trivias = "trivias" hel = "hel" whos = "whos" +spawnve = "spawnve" diff --git a/crates/ruff/resources/test/fixtures/flake8_bandit/S602.py b/crates/ruff/resources/test/fixtures/flake8_bandit/S602.py new file mode 100644 index 0000000000..ef0626986e --- /dev/null +++ b/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) diff --git a/crates/ruff/resources/test/fixtures/flake8_bandit/S603.py b/crates/ruff/resources/test/fixtures/flake8_bandit/S603.py new file mode 100644 index 0000000000..da9b2dae3f --- /dev/null +++ b/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") diff --git a/crates/ruff/resources/test/fixtures/flake8_bandit/S604.py b/crates/ruff/resources/test/fixtures/flake8_bandit/S604.py new file mode 100644 index 0000000000..3be26d5495 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_bandit/S604.py @@ -0,0 +1,5 @@ +def foo(shell): + pass + + +foo(shell=True) diff --git a/crates/ruff/resources/test/fixtures/flake8_bandit/S605.py b/crates/ruff/resources/test/fixtures/flake8_bandit/S605.py new file mode 100644 index 0000000000..de9499ec54 --- /dev/null +++ b/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, ""]) diff --git a/crates/ruff/resources/test/fixtures/flake8_bandit/S606.py b/crates/ruff/resources/test/fixtures/flake8_bandit/S606.py new file mode 100644 index 0000000000..e6c4bfe17d --- /dev/null +++ b/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") diff --git a/crates/ruff/resources/test/fixtures/flake8_bandit/S607.py b/crates/ruff/resources/test/fixtures/flake8_bandit/S607.py new file mode 100644 index 0000000000..0bcb8cae0a --- /dev/null +++ b/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") diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 0c0b480d6c..3029ec57aa 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2651,6 +2651,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) { diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index d7aa952b4b..ae67c325f2 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -507,6 +507,12 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (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, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 8811672ffd..290d9a8699 100644 --- a/crates/ruff/src/registry.rs +++ b/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, diff --git a/crates/ruff/src/rules/flake8_bandit/mod.rs b/crates/ruff/src/rules/flake8_bandit/mod.rs index 97ba897b30..91cd61a150 100644 --- a/crates/ruff/src/rules/flake8_bandit/mod.rs +++ b/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")] diff --git a/crates/ruff/src/rules/flake8_bandit/rules/mod.rs b/crates/ruff/src/rules/flake8_bandit/rules/mod.rs index 4809103c97..aa836f6eba 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/mod.rs +++ b/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; diff --git a/crates/ruff/src/rules/flake8_bandit/rules/shell_injection.rs b/crates/ruff/src/rules/flake8_bandit/rules/shell_injection.rs new file mode 100644 index 0000000000..618de28e86 --- /dev/null +++ b/crates/ruff/src/rules/flake8_bandit/rules/shell_injection.rs @@ -0,0 +1,372 @@ +//! Checks relating to shell injection. + +use num_bigint::BigInt; +use once_cell::sync::Lazy; +use regex::Regex; +use rustpython_parser::ast::{Constant, Expr, ExprKind, Keyword}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::types::Range; +use ruff_python_semantic::context::Context; + +use crate::{ + checkers::ast::Checker, registry::Rule, rules::flake8_bandit::helpers::string_literal, +}; + +static FULL_PATH_REGEX: Lazy = Lazy::new(|| Regex::new(r"^([A-Za-z]:|[\\/.])").unwrap()); + +#[violation] +pub struct SubprocessPopenWithShellEqualsTrue { + seems_safe: bool, +} + +impl Violation for SubprocessPopenWithShellEqualsTrue { + #[derive_message_formats] + fn message(&self) -> String { + if self.seems_safe { + format!( + "`subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell`" + ) + } else { + format!("`subprocess` call with `shell=True` identified, security issue") + } + } +} + +#[violation] +pub struct SubprocessWithoutShellEqualsTrue; + +impl Violation for SubprocessWithoutShellEqualsTrue { + #[derive_message_formats] + fn message(&self) -> String { + format!("`subprocess` call: check for execution of untrusted input") + } +} + +#[violation] +pub struct CallWithShellEqualsTrue; + +impl Violation for CallWithShellEqualsTrue { + #[derive_message_formats] + fn message(&self) -> String { + format!("Function call with `shell=True` parameter identified, security issue") + } +} + +#[violation] +pub struct StartProcessWithAShell { + seems_safe: bool, +} + +impl Violation for StartProcessWithAShell { + #[derive_message_formats] + fn message(&self) -> String { + if self.seems_safe { + format!("Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`") + } else { + format!("Starting a process with a shell, possible injection detected") + } + } +} + +#[violation] +pub struct StartProcessWithNoShell; + +impl Violation for StartProcessWithNoShell { + #[derive_message_formats] + fn message(&self) -> String { + format!("Starting a process without a shell") + } +} + +#[violation] +pub struct StartProcessWithPartialPath; + +impl Violation for StartProcessWithPartialPath { + #[derive_message_formats] + fn message(&self) -> String { + format!("Starting a process with a partial executable path") + } +} + +#[derive(Copy, Clone, Debug)] +enum CallKind { + Subprocess, + Shell, + NoShell, +} + +/// Return the [`CallKind`] of the given function call. +fn get_call_kind(func: &Expr, context: &Context) -> Option { + context + .resolve_call_path(func) + .and_then(|call_path| match call_path.as_slice() { + &[module, submodule] => match module { + "os" => match submodule { + "execl" | "execle" | "execlp" | "execlpe" | "execv" | "execve" | "execvp" + | "execvpe" | "spawnl" | "spawnle" | "spawnlp" | "spawnlpe" | "spawnv" + | "spawnve" | "spawnvp" | "spawnvpe" | "startfile" => Some(CallKind::NoShell), + "system" | "popen" | "popen2" | "popen3" | "popen4" => Some(CallKind::Shell), + _ => None, + }, + "subprocess" => match submodule { + "Popen" | "call" | "check_call" | "check_output" | "run" => { + Some(CallKind::Subprocess) + } + _ => None, + }, + "popen2" => match submodule { + "popen2" | "popen3" | "popen4" | "Popen3" | "Popen4" => Some(CallKind::Shell), + _ => None, + }, + "commands" => match submodule { + "getoutput" | "getstatusoutput" => Some(CallKind::Shell), + _ => None, + }, + _ => None, + }, + _ => None, + }) +} + +#[derive(Copy, Clone, Debug)] +enum Truthiness { + // The `shell` keyword argument is set and evaluates to `False`. + Falsey, + // The `shell` keyword argument is set and evaluates to `True`. + Truthy, + // The `shell` keyword argument is set, but its value is unknown. + Unknown, +} + +impl From<&Keyword> for Truthiness { + fn from(value: &Keyword) -> Self { + match &value.node.value.node { + ExprKind::Constant { + value: Constant::Bool(b), + .. + } => { + if *b { + Truthiness::Truthy + } else { + Truthiness::Falsey + } + } + ExprKind::Constant { + value: Constant::Int(int), + .. + } => { + if int == &BigInt::from(0u8) { + Truthiness::Falsey + } else { + Truthiness::Truthy + } + } + ExprKind::Constant { + value: Constant::Float(float), + .. + } => { + if (float - 0.0).abs() < f64::EPSILON { + Truthiness::Falsey + } else { + Truthiness::Truthy + } + } + ExprKind::Constant { + value: Constant::None, + .. + } => Truthiness::Falsey, + ExprKind::List { elts, .. } + | ExprKind::Set { elts, .. } + | ExprKind::Tuple { elts, .. } => { + if elts.is_empty() { + Truthiness::Falsey + } else { + Truthiness::Truthy + } + } + ExprKind::Dict { keys, .. } => { + if keys.is_empty() { + Truthiness::Falsey + } else { + Truthiness::Truthy + } + } + _ => Truthiness::Unknown, + } + } +} + +#[derive(Copy, Clone, Debug)] +struct ShellKeyword<'a> { + /// Whether the `shell` keyword argument is set and evaluates to `True`. + truthiness: Truthiness, + /// The `shell` keyword argument. + keyword: &'a Keyword, +} + +/// Return the `shell` keyword argument to the given function call, if any. +fn find_shell_keyword(keywords: &[Keyword]) -> Option { + keywords + .iter() + .find(|keyword| { + keyword + .node + .arg + .as_ref() + .map_or(false, |arg| arg == "shell") + }) + .map(|keyword| ShellKeyword { + truthiness: keyword.into(), + keyword, + }) +} + +/// Return `true` if the value provided to the `shell` call seems safe. This is based on Bandit's +/// definition: string literals are considered okay, but dynamically-computed values are not. +fn shell_call_seems_safe(arg: &Expr) -> bool { + matches!( + arg.node, + ExprKind::Constant { + value: Constant::Str(_), + .. + } + ) +} + +/// Return the [`Expr`] as a string literal, if it's a string or a list of strings. +fn try_string_literal(expr: &Expr) -> Option<&str> { + match &expr.node { + ExprKind::List { elts, .. } => { + if elts.is_empty() { + None + } else { + string_literal(&elts[0]) + } + } + _ => string_literal(expr), + } +} + +/// S602, S603, S604, S605, S606, S607 +pub fn shell_injection(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: &[Keyword]) { + let call_kind = get_call_kind(func, &checker.ctx); + + if matches!(call_kind, Some(CallKind::Subprocess)) { + if let Some(arg) = args.first() { + match find_shell_keyword(keywords) { + // S602 + Some(ShellKeyword { + truthiness: Truthiness::Truthy, + keyword, + }) => { + if checker + .settings + .rules + .enabled(Rule::SubprocessPopenWithShellEqualsTrue) + { + checker.diagnostics.push(Diagnostic::new( + SubprocessPopenWithShellEqualsTrue { + seems_safe: shell_call_seems_safe(arg), + }, + Range::from(keyword), + )); + } + } + // S603 + Some(ShellKeyword { + truthiness: Truthiness::Falsey | Truthiness::Unknown, + keyword, + }) => { + if checker + .settings + .rules + .enabled(Rule::SubprocessWithoutShellEqualsTrue) + { + checker.diagnostics.push(Diagnostic::new( + SubprocessWithoutShellEqualsTrue, + Range::from(keyword), + )); + } + } + // S603 + None => { + if checker + .settings + .rules + .enabled(Rule::SubprocessWithoutShellEqualsTrue) + { + checker.diagnostics.push(Diagnostic::new( + SubprocessWithoutShellEqualsTrue, + Range::from(arg), + )); + } + } + } + } + } else if let Some(ShellKeyword { + truthiness: Truthiness::Truthy, + keyword, + }) = find_shell_keyword(keywords) + { + // S604 + if checker + .settings + .rules + .enabled(Rule::CallWithShellEqualsTrue) + { + checker.diagnostics.push(Diagnostic::new( + CallWithShellEqualsTrue, + Range::from(keyword), + )); + } + } + + // S605 + if matches!(call_kind, Some(CallKind::Shell)) { + if let Some(arg) = args.first() { + if checker.settings.rules.enabled(Rule::StartProcessWithAShell) { + checker.diagnostics.push(Diagnostic::new( + StartProcessWithAShell { + seems_safe: shell_call_seems_safe(arg), + }, + Range::from(arg), + )); + } + } + } + + // S606 + if matches!(call_kind, Some(CallKind::NoShell)) { + if checker + .settings + .rules + .enabled(Rule::StartProcessWithNoShell) + { + checker + .diagnostics + .push(Diagnostic::new(StartProcessWithNoShell, Range::from(func))); + } + } + + // S607 + if call_kind.is_some() { + if let Some(arg) = args.first() { + if checker + .settings + .rules + .enabled(Rule::StartProcessWithPartialPath) + { + if let Some(value) = try_string_literal(arg) { + if FULL_PATH_REGEX.find(value).is_none() { + checker.diagnostics.push(Diagnostic::new( + StartProcessWithPartialPath, + Range::from(arg), + )); + } + } + } + } + } +} diff --git a/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S602_S602.py.snap b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S602_S602.py.snap new file mode 100644 index 0000000000..3f8121ff89 --- /dev/null +++ b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S602_S602.py.snap @@ -0,0 +1,117 @@ +--- +source: crates/ruff/src/rules/flake8_bandit/mod.rs +--- +S602.py:4:15: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell` + | +4 | # Check different Popen wrappers are checked. +5 | Popen("true", shell=True) + | ^^^^^^^^^^ S602 +6 | call("true", shell=True) +7 | check_call("true", shell=True) + | + +S602.py:5:14: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell` + | +5 | # Check different Popen wrappers are checked. +6 | Popen("true", shell=True) +7 | call("true", shell=True) + | ^^^^^^^^^^ S602 +8 | check_call("true", shell=True) +9 | check_output("true", shell=True) + | + +S602.py:6:20: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell` + | + 6 | Popen("true", shell=True) + 7 | call("true", shell=True) + 8 | check_call("true", shell=True) + | ^^^^^^^^^^ S602 + 9 | check_output("true", shell=True) +10 | run("true", shell=True) + | + +S602.py:7:22: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell` + | + 7 | call("true", shell=True) + 8 | check_call("true", shell=True) + 9 | check_output("true", shell=True) + | ^^^^^^^^^^ S602 +10 | run("true", shell=True) + | + +S602.py:8:13: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell` + | + 8 | check_call("true", shell=True) + 9 | check_output("true", shell=True) +10 | run("true", shell=True) + | ^^^^^^^^^^ S602 +11 | +12 | # Check values that truthy values are treated as true. + | + +S602.py:11:15: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell` + | +11 | # Check values that truthy values are treated as true. +12 | Popen("true", shell=1) + | ^^^^^^^ S602 +13 | Popen("true", shell=[1]) +14 | Popen("true", shell={1: 1}) + | + +S602.py:12:15: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell` + | +12 | # Check values that truthy values are treated as true. +13 | Popen("true", shell=1) +14 | Popen("true", shell=[1]) + | ^^^^^^^^^ S602 +15 | Popen("true", shell={1: 1}) +16 | Popen("true", shell=(1,)) + | + +S602.py:13:15: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell` + | +13 | Popen("true", shell=1) +14 | Popen("true", shell=[1]) +15 | Popen("true", shell={1: 1}) + | ^^^^^^^^^^^^ S602 +16 | Popen("true", shell=(1,)) + | + +S602.py:14:15: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell` + | +14 | Popen("true", shell=[1]) +15 | Popen("true", shell={1: 1}) +16 | Popen("true", shell=(1,)) + | ^^^^^^^^^^ S602 +17 | +18 | # Check command argument looks unsafe. + | + +S602.py:18:19: S602 `subprocess` call with `shell=True` identified, security issue + | +18 | # Check command argument looks unsafe. +19 | var_string = "true" +20 | Popen(var_string, shell=True) + | ^^^^^^^^^^ S602 +21 | Popen([var_string], shell=True) +22 | Popen([var_string, ""], shell=True) + | + +S602.py:19:21: S602 `subprocess` call with `shell=True` identified, security issue + | +19 | var_string = "true" +20 | Popen(var_string, shell=True) +21 | Popen([var_string], shell=True) + | ^^^^^^^^^^ S602 +22 | Popen([var_string, ""], shell=True) + | + +S602.py:20:25: S602 `subprocess` call with `shell=True` identified, security issue + | +20 | Popen(var_string, shell=True) +21 | Popen([var_string], shell=True) +22 | Popen([var_string, ""], shell=True) + | ^^^^^^^^^^ S602 + | + + diff --git a/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S603_S603.py.snap b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S603_S603.py.snap new file mode 100644 index 0000000000..a258112aa6 --- /dev/null +++ b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S603_S603.py.snap @@ -0,0 +1,106 @@ +--- +source: crates/ruff/src/rules/flake8_bandit/mod.rs +--- +S603.py:4:15: S603 `subprocess` call: check for execution of untrusted input + | +4 | # Different Popen wrappers are checked. +5 | Popen("true", shell=False) + | ^^^^^^^^^^^ S603 +6 | call("true", shell=False) +7 | check_call("true", shell=False) + | + +S603.py:5:14: S603 `subprocess` call: check for execution of untrusted input + | +5 | # Different Popen wrappers are checked. +6 | Popen("true", shell=False) +7 | call("true", shell=False) + | ^^^^^^^^^^^ S603 +8 | check_call("true", shell=False) +9 | check_output("true", shell=False) + | + +S603.py:6:20: S603 `subprocess` call: check for execution of untrusted input + | + 6 | Popen("true", shell=False) + 7 | call("true", shell=False) + 8 | check_call("true", shell=False) + | ^^^^^^^^^^^ S603 + 9 | check_output("true", shell=False) +10 | run("true", shell=False) + | + +S603.py:7:22: S603 `subprocess` call: check for execution of untrusted input + | + 7 | call("true", shell=False) + 8 | check_call("true", shell=False) + 9 | check_output("true", shell=False) + | ^^^^^^^^^^^ S603 +10 | run("true", shell=False) + | + +S603.py:8:13: S603 `subprocess` call: check for execution of untrusted input + | + 8 | check_call("true", shell=False) + 9 | check_output("true", shell=False) +10 | run("true", shell=False) + | ^^^^^^^^^^^ S603 +11 | +12 | # Values that falsey values are treated as false. + | + +S603.py:11:15: S603 `subprocess` call: check for execution of untrusted input + | +11 | # Values that falsey values are treated as false. +12 | Popen("true", shell=0) + | ^^^^^^^ S603 +13 | Popen("true", shell=[]) +14 | Popen("true", shell={}) + | + +S603.py:12:15: S603 `subprocess` call: check for execution of untrusted input + | +12 | # Values that falsey values are treated as false. +13 | Popen("true", shell=0) +14 | Popen("true", shell=[]) + | ^^^^^^^^ S603 +15 | Popen("true", shell={}) +16 | Popen("true", shell=None) + | + +S603.py:13:15: S603 `subprocess` call: check for execution of untrusted input + | +13 | Popen("true", shell=0) +14 | Popen("true", shell=[]) +15 | Popen("true", shell={}) + | ^^^^^^^^ S603 +16 | Popen("true", shell=None) + | + +S603.py:14:15: S603 `subprocess` call: check for execution of untrusted input + | +14 | Popen("true", shell=[]) +15 | Popen("true", shell={}) +16 | Popen("true", shell=None) + | ^^^^^^^^^^ S603 +17 | +18 | # Unknown values are treated as falsey. + | + +S603.py:17:15: S603 `subprocess` call: check for execution of untrusted input + | +17 | # Unknown values are treated as falsey. +18 | Popen("true", shell=True if True else False) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S603 +19 | +20 | # No value is also caught. + | + +S603.py:20:7: S603 `subprocess` call: check for execution of untrusted input + | +20 | # No value is also caught. +21 | Popen("true") + | ^^^^^^ S603 + | + + diff --git a/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S604_S604.py.snap b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S604_S604.py.snap new file mode 100644 index 0000000000..83e9248dfb --- /dev/null +++ b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S604_S604.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff/src/rules/flake8_bandit/mod.rs +--- +S604.py:5:5: S604 Function call with `shell=True` parameter identified, security issue + | +5 | foo(shell=True) + | ^^^^^^^^^^ S604 + | + + diff --git a/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S605_S605.py.snap b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S605_S605.py.snap new file mode 100644 index 0000000000..b0b3e45a3d --- /dev/null +++ b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S605_S605.py.snap @@ -0,0 +1,147 @@ +--- +source: crates/ruff/src/rules/flake8_bandit/mod.rs +--- +S605.py:7:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | + 7 | # Check all shell functions. + 8 | os.system("true") + | ^^^^^^ S605 + 9 | os.popen("true") +10 | os.popen2("true") + | + +S605.py:8:10: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | + 8 | # Check all shell functions. + 9 | os.system("true") +10 | os.popen("true") + | ^^^^^^ S605 +11 | os.popen2("true") +12 | os.popen3("true") + | + +S605.py:9:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | + 9 | os.system("true") +10 | os.popen("true") +11 | os.popen2("true") + | ^^^^^^ S605 +12 | os.popen3("true") +13 | os.popen4("true") + | + +S605.py:10:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +10 | os.popen("true") +11 | os.popen2("true") +12 | os.popen3("true") + | ^^^^^^ S605 +13 | os.popen4("true") +14 | popen2.popen2("true") + | + +S605.py:11:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +11 | os.popen2("true") +12 | os.popen3("true") +13 | os.popen4("true") + | ^^^^^^ S605 +14 | popen2.popen2("true") +15 | popen2.popen3("true") + | + +S605.py:12:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +12 | os.popen3("true") +13 | os.popen4("true") +14 | popen2.popen2("true") + | ^^^^^^ S605 +15 | popen2.popen3("true") +16 | popen2.popen4("true") + | + +S605.py:13:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +13 | os.popen4("true") +14 | popen2.popen2("true") +15 | popen2.popen3("true") + | ^^^^^^ S605 +16 | popen2.popen4("true") +17 | popen2.Popen3("true") + | + +S605.py:14:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +14 | popen2.popen2("true") +15 | popen2.popen3("true") +16 | popen2.popen4("true") + | ^^^^^^ S605 +17 | popen2.Popen3("true") +18 | popen2.Popen4("true") + | + +S605.py:15:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +15 | popen2.popen3("true") +16 | popen2.popen4("true") +17 | popen2.Popen3("true") + | ^^^^^^ S605 +18 | popen2.Popen4("true") +19 | commands.getoutput("true") + | + +S605.py:16:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +16 | popen2.popen4("true") +17 | popen2.Popen3("true") +18 | popen2.Popen4("true") + | ^^^^^^ S605 +19 | commands.getoutput("true") +20 | commands.getstatusoutput("true") + | + +S605.py:17:20: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +17 | popen2.Popen3("true") +18 | popen2.Popen4("true") +19 | commands.getoutput("true") + | ^^^^^^ S605 +20 | commands.getstatusoutput("true") + | + +S605.py:18:26: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +18 | popen2.Popen4("true") +19 | commands.getoutput("true") +20 | commands.getstatusoutput("true") + | ^^^^^^ S605 + | + +S605.py:23:11: S605 Starting a process with a shell, possible injection detected + | +23 | # Check command argument looks unsafe. +24 | var_string = "true" +25 | os.system(var_string) + | ^^^^^^^^^^ S605 +26 | os.system([var_string]) +27 | os.system([var_string, ""]) + | + +S605.py:24:11: S605 Starting a process with a shell, possible injection detected + | +24 | var_string = "true" +25 | os.system(var_string) +26 | os.system([var_string]) + | ^^^^^^^^^^^^ S605 +27 | os.system([var_string, ""]) + | + +S605.py:25:11: S605 Starting a process with a shell, possible injection detected + | +25 | os.system(var_string) +26 | os.system([var_string]) +27 | os.system([var_string, ""]) + | ^^^^^^^^^^^^^^^^ S605 + | + + diff --git a/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S606_S606.py.snap b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S606_S606.py.snap new file mode 100644 index 0000000000..f237bfd6b4 --- /dev/null +++ b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S606_S606.py.snap @@ -0,0 +1,170 @@ +--- +source: crates/ruff/src/rules/flake8_bandit/mod.rs +--- +S606.py:4:1: S606 Starting a process without a shell + | +4 | # Check all shell functions. +5 | os.execl("true") + | ^^^^^^^^ S606 +6 | os.execle("true") +7 | os.execlp("true") + | + +S606.py:5:1: S606 Starting a process without a shell + | +5 | # Check all shell functions. +6 | os.execl("true") +7 | os.execle("true") + | ^^^^^^^^^ S606 +8 | os.execlp("true") +9 | os.execlpe("true") + | + +S606.py:6:1: S606 Starting a process without a shell + | + 6 | os.execl("true") + 7 | os.execle("true") + 8 | os.execlp("true") + | ^^^^^^^^^ S606 + 9 | os.execlpe("true") +10 | os.execv("true") + | + +S606.py:7:1: S606 Starting a process without a shell + | + 7 | os.execle("true") + 8 | os.execlp("true") + 9 | os.execlpe("true") + | ^^^^^^^^^^ S606 +10 | os.execv("true") +11 | os.execve("true") + | + +S606.py:8:1: S606 Starting a process without a shell + | + 8 | os.execlp("true") + 9 | os.execlpe("true") +10 | os.execv("true") + | ^^^^^^^^ S606 +11 | os.execve("true") +12 | os.execvp("true") + | + +S606.py:9:1: S606 Starting a process without a shell + | + 9 | os.execlpe("true") +10 | os.execv("true") +11 | os.execve("true") + | ^^^^^^^^^ S606 +12 | os.execvp("true") +13 | os.execvpe("true") + | + +S606.py:10:1: S606 Starting a process without a shell + | +10 | os.execv("true") +11 | os.execve("true") +12 | os.execvp("true") + | ^^^^^^^^^ S606 +13 | os.execvpe("true") +14 | os.spawnl("true") + | + +S606.py:11:1: S606 Starting a process without a shell + | +11 | os.execve("true") +12 | os.execvp("true") +13 | os.execvpe("true") + | ^^^^^^^^^^ S606 +14 | os.spawnl("true") +15 | os.spawnle("true") + | + +S606.py:12:1: S606 Starting a process without a shell + | +12 | os.execvp("true") +13 | os.execvpe("true") +14 | os.spawnl("true") + | ^^^^^^^^^ S606 +15 | os.spawnle("true") +16 | os.spawnlp("true") + | + +S606.py:13:1: S606 Starting a process without a shell + | +13 | os.execvpe("true") +14 | os.spawnl("true") +15 | os.spawnle("true") + | ^^^^^^^^^^ S606 +16 | os.spawnlp("true") +17 | os.spawnlpe("true") + | + +S606.py:14:1: S606 Starting a process without a shell + | +14 | os.spawnl("true") +15 | os.spawnle("true") +16 | os.spawnlp("true") + | ^^^^^^^^^^ S606 +17 | os.spawnlpe("true") +18 | os.spawnv("true") + | + +S606.py:15:1: S606 Starting a process without a shell + | +15 | os.spawnle("true") +16 | os.spawnlp("true") +17 | os.spawnlpe("true") + | ^^^^^^^^^^^ S606 +18 | os.spawnv("true") +19 | os.spawnve("true") + | + +S606.py:16:1: S606 Starting a process without a shell + | +16 | os.spawnlp("true") +17 | os.spawnlpe("true") +18 | os.spawnv("true") + | ^^^^^^^^^ S606 +19 | os.spawnve("true") +20 | os.spawnvp("true") + | + +S606.py:17:1: S606 Starting a process without a shell + | +17 | os.spawnlpe("true") +18 | os.spawnv("true") +19 | os.spawnve("true") + | ^^^^^^^^^^ S606 +20 | os.spawnvp("true") +21 | os.spawnvpe("true") + | + +S606.py:18:1: S606 Starting a process without a shell + | +18 | os.spawnv("true") +19 | os.spawnve("true") +20 | os.spawnvp("true") + | ^^^^^^^^^^ S606 +21 | os.spawnvpe("true") +22 | os.startfile("true") + | + +S606.py:19:1: S606 Starting a process without a shell + | +19 | os.spawnve("true") +20 | os.spawnvp("true") +21 | os.spawnvpe("true") + | ^^^^^^^^^^^ S606 +22 | os.startfile("true") + | + +S606.py:20:1: S606 Starting a process without a shell + | +20 | os.spawnvp("true") +21 | os.spawnvpe("true") +22 | os.startfile("true") + | ^^^^^^^^^^^^ S606 + | + + diff --git a/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S607_S607.py.snap b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S607_S607.py.snap new file mode 100644 index 0000000000..fde8ded402 --- /dev/null +++ b/crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S607_S607.py.snap @@ -0,0 +1,223 @@ +--- +source: crates/ruff/src/rules/flake8_bandit/mod.rs +--- +S607.py:9:11: S607 Starting a process with a partial executable path + | + 9 | subprocess.check_output("true") +10 | subprocess.run("true") +11 | os.system("true") + | ^^^^^^ S607 +12 | os.popen("true") +13 | os.popen2("true") + | + +S607.py:10:10: S607 Starting a process with a partial executable path + | +10 | subprocess.run("true") +11 | os.system("true") +12 | os.popen("true") + | ^^^^^^ S607 +13 | os.popen2("true") +14 | os.popen3("true") + | + +S607.py:11:11: S607 Starting a process with a partial executable path + | +11 | os.system("true") +12 | os.popen("true") +13 | os.popen2("true") + | ^^^^^^ S607 +14 | os.popen3("true") +15 | os.popen4("true") + | + +S607.py:12:11: S607 Starting a process with a partial executable path + | +12 | os.popen("true") +13 | os.popen2("true") +14 | os.popen3("true") + | ^^^^^^ S607 +15 | os.popen4("true") +16 | popen2.popen2("true") + | + +S607.py:13:11: S607 Starting a process with a partial executable path + | +13 | os.popen2("true") +14 | os.popen3("true") +15 | os.popen4("true") + | ^^^^^^ S607 +16 | popen2.popen2("true") +17 | popen2.popen3("true") + | + +S607.py:21:10: S607 Starting a process with a partial executable path + | +21 | commands.getoutput("true") +22 | commands.getstatusoutput("true") +23 | os.execl("true") + | ^^^^^^ S607 +24 | os.execle("true") +25 | os.execlp("true") + | + +S607.py:22:11: S607 Starting a process with a partial executable path + | +22 | commands.getstatusoutput("true") +23 | os.execl("true") +24 | os.execle("true") + | ^^^^^^ S607 +25 | os.execlp("true") +26 | os.execlpe("true") + | + +S607.py:23:11: S607 Starting a process with a partial executable path + | +23 | os.execl("true") +24 | os.execle("true") +25 | os.execlp("true") + | ^^^^^^ S607 +26 | os.execlpe("true") +27 | os.execv("true") + | + +S607.py:24:12: S607 Starting a process with a partial executable path + | +24 | os.execle("true") +25 | os.execlp("true") +26 | os.execlpe("true") + | ^^^^^^ S607 +27 | os.execv("true") +28 | os.execve("true") + | + +S607.py:25:10: S607 Starting a process with a partial executable path + | +25 | os.execlp("true") +26 | os.execlpe("true") +27 | os.execv("true") + | ^^^^^^ S607 +28 | os.execve("true") +29 | os.execvp("true") + | + +S607.py:26:11: S607 Starting a process with a partial executable path + | +26 | os.execlpe("true") +27 | os.execv("true") +28 | os.execve("true") + | ^^^^^^ S607 +29 | os.execvp("true") +30 | os.execvpe("true") + | + +S607.py:27:11: S607 Starting a process with a partial executable path + | +27 | os.execv("true") +28 | os.execve("true") +29 | os.execvp("true") + | ^^^^^^ S607 +30 | os.execvpe("true") +31 | os.spawnl("true") + | + +S607.py:28:12: S607 Starting a process with a partial executable path + | +28 | os.execve("true") +29 | os.execvp("true") +30 | os.execvpe("true") + | ^^^^^^ S607 +31 | os.spawnl("true") +32 | os.spawnle("true") + | + +S607.py:29:11: S607 Starting a process with a partial executable path + | +29 | os.execvp("true") +30 | os.execvpe("true") +31 | os.spawnl("true") + | ^^^^^^ S607 +32 | os.spawnle("true") +33 | os.spawnlp("true") + | + +S607.py:30:12: S607 Starting a process with a partial executable path + | +30 | os.execvpe("true") +31 | os.spawnl("true") +32 | os.spawnle("true") + | ^^^^^^ S607 +33 | os.spawnlp("true") +34 | os.spawnlpe("true") + | + +S607.py:31:12: S607 Starting a process with a partial executable path + | +31 | os.spawnl("true") +32 | os.spawnle("true") +33 | os.spawnlp("true") + | ^^^^^^ S607 +34 | os.spawnlpe("true") +35 | os.spawnv("true") + | + +S607.py:32:13: S607 Starting a process with a partial executable path + | +32 | os.spawnle("true") +33 | os.spawnlp("true") +34 | os.spawnlpe("true") + | ^^^^^^ S607 +35 | os.spawnv("true") +36 | os.spawnve("true") + | + +S607.py:33:11: S607 Starting a process with a partial executable path + | +33 | os.spawnlp("true") +34 | os.spawnlpe("true") +35 | os.spawnv("true") + | ^^^^^^ S607 +36 | os.spawnve("true") +37 | os.spawnvp("true") + | + +S607.py:34:12: S607 Starting a process with a partial executable path + | +34 | os.spawnlpe("true") +35 | os.spawnv("true") +36 | os.spawnve("true") + | ^^^^^^ S607 +37 | os.spawnvp("true") +38 | os.spawnvpe("true") + | + +S607.py:35:12: S607 Starting a process with a partial executable path + | +35 | os.spawnv("true") +36 | os.spawnve("true") +37 | os.spawnvp("true") + | ^^^^^^ S607 +38 | os.spawnvpe("true") +39 | os.startfile("true") + | + +S607.py:36:13: S607 Starting a process with a partial executable path + | +36 | os.spawnve("true") +37 | os.spawnvp("true") +38 | os.spawnvpe("true") + | ^^^^^^ S607 +39 | os.startfile("true") + | + +S607.py:37:14: S607 Starting a process with a partial executable path + | +37 | os.spawnvp("true") +38 | os.spawnvpe("true") +39 | os.startfile("true") + | ^^^^^^ S607 +40 | +41 | # Check it does not fail for full paths. + | + + diff --git a/ruff.schema.json b/ruff.schema.json index fb4ac341c8..65489a1dd6 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2163,6 +2163,12 @@ "S509", "S6", "S60", + "S602", + "S603", + "S604", + "S605", + "S606", + "S607", "S608", "S61", "S612",