Skip to content

Commit

Permalink
Implement S609, linux_commands_wildcard_injection
Browse files Browse the repository at this point in the history
  • Loading branch information
scop committed May 18, 2023
1 parent fdd8941 commit 46936b3
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 2 deletions.
8 changes: 8 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_bandit/S609.py
@@ -0,0 +1,8 @@
import os
import subprocess

os.popen("chmod +w foo*")
subprocess.Popen("/bin/chown root: *", shell=True)
subprocess.Popen(["/usr/local/bin/rsync", "*", "some_where:"], shell=True)
subprocess.Popen("/usr/local/bin/rsync * no_injection_here:")
os.system("tar cf foo.tar bar/*")
1 change: 1 addition & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Expand Up @@ -2862,6 +2862,7 @@ where
Rule::StartProcessWithAShell,
Rule::StartProcessWithNoShell,
Rule::StartProcessWithPartialPath,
Rule::UnixCommandWildcardInjection,
]) {
flake8_bandit::rules::shell_injection(self, func, args, keywords);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Expand Up @@ -514,6 +514,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Bandit, "606") => (RuleGroup::Unspecified, Rule::StartProcessWithNoShell),
(Flake8Bandit, "607") => (RuleGroup::Unspecified, Rule::StartProcessWithPartialPath),
(Flake8Bandit, "608") => (RuleGroup::Unspecified, Rule::HardcodedSQLExpression),
(Flake8Bandit, "609") => (RuleGroup::Unspecified, Rule::UnixCommandWildcardInjection),
(Flake8Bandit, "612") => (RuleGroup::Unspecified, Rule::LoggingConfigInsecureListen),
(Flake8Bandit, "701") => (RuleGroup::Unspecified, Rule::Jinja2AutoescapeFalse),

Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Expand Up @@ -433,6 +433,7 @@ ruff_macros::register_rules!(
rules::flake8_bandit::rules::StartProcessWithAShell,
rules::flake8_bandit::rules::StartProcessWithNoShell,
rules::flake8_bandit::rules::StartProcessWithPartialPath,
rules::flake8_bandit::rules::UnixCommandWildcardInjection,
rules::flake8_bandit::rules::SuspiciousEvalUsage,
rules::flake8_bandit::rules::SuspiciousFTPLibUsage,
rules::flake8_bandit::rules::SuspiciousInsecureCipherUsage,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flake8_bandit/mod.rs
Expand Up @@ -42,6 +42,7 @@ mod tests {
#[test_case(Rule::SuspiciousTelnetUsage, Path::new("S312.py"); "S312")]
#[test_case(Rule::TryExceptContinue, Path::new("S112.py"); "S112")]
#[test_case(Rule::TryExceptPass, Path::new("S110.py"); "S110")]
#[test_case(Rule::UnixCommandWildcardInjection, Path::new("S609.py"); "S609")]
#[test_case(Rule::UnsafeYAMLLoad, Path::new("S506.py"); "S506")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/src/rules/flake8_bandit/rules/mod.rs
Expand Up @@ -27,7 +27,7 @@ pub(crate) use request_without_timeout::{request_without_timeout, RequestWithout
pub(crate) use shell_injection::{
shell_injection, CallWithShellEqualsTrue, StartProcessWithAShell, StartProcessWithNoShell,
StartProcessWithPartialPath, SubprocessPopenWithShellEqualsTrue,
SubprocessWithoutShellEqualsTrue,
SubprocessWithoutShellEqualsTrue, UnixCommandWildcardInjection,
};
pub(crate) use snmp_insecure_version::{snmp_insecure_version, SnmpInsecureVersion};
pub(crate) use snmp_weak_cryptography::{snmp_weak_cryptography, SnmpWeakCryptography};
Expand Down
41 changes: 40 additions & 1 deletion crates/ruff/src/rules/flake8_bandit/rules/shell_injection.rs
@@ -1,5 +1,6 @@
//! Checks relating to shell injection.

use itertools::Itertools;
use once_cell::sync::Lazy;
use regex::Regex;
use rustpython_parser::ast::{self, Constant, Expr, Keyword, Ranged};
Expand All @@ -14,6 +15,8 @@ use crate::{
};

static FULL_PATH_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"^([A-Za-z]:|[\\/.])").unwrap());
static WILDCARD_COMMAND_REGEX: Lazy<Regex> =
Lazy::new(|| Regex::new(r"(^|[\s/])(chown|chmod|tar|rsync)\s.*\*").unwrap());

#[violation]
pub struct SubprocessPopenWithShellEqualsTrue {
Expand Down Expand Up @@ -89,6 +92,16 @@ impl Violation for StartProcessWithPartialPath {
}
}

#[violation]
pub struct UnixCommandWildcardInjection;

impl Violation for UnixCommandWildcardInjection {
#[derive_message_formats]
fn message(&self) -> String {
format!("Possible wildcard injection in call")
}
}

#[derive(Copy, Clone, Debug)]
enum CallKind {
Subprocess,
Expand Down Expand Up @@ -174,14 +187,15 @@ fn try_string_literal(expr: &Expr) -> Option<&str> {
}
}

/// S602, S603, S604, S605, S606, S607
/// S602, S603, S604, S605, S606, S607, S609
pub(crate) fn shell_injection(
checker: &mut Checker,
func: &Expr,
args: &[Expr],
keywords: &[Keyword],
) {
let call_kind = get_call_kind(func, &checker.ctx);
let mut subprocess_with_shell = false;

if matches!(call_kind, Some(CallKind::Subprocess)) {
if let Some(arg) = args.first() {
Expand All @@ -191,6 +205,7 @@ pub(crate) fn shell_injection(
truthiness: Truthiness::Truthy,
keyword,
}) => {
subprocess_with_shell = true;
if checker
.settings
.rules
Expand Down Expand Up @@ -297,4 +312,28 @@ pub(crate) fn shell_injection(
}
}
}

// S609
if checker
.settings
.rules
.enabled(Rule::UnixCommandWildcardInjection)
&& args.len() != 0
&& (matches!(call_kind, Some(CallKind::Shell)) || subprocess_with_shell)
{
if let Some(cmd) = args.first() {
let cmd_string = match cmd {
Expr::List(ast::ExprList { elts, .. }) => elts
.iter()
.map(|elt| string_literal(elt).unwrap_or_else(|| ""))
.join(" "),
_ => String::from(string_literal(cmd).unwrap_or_else(|| "")),
};
if WILDCARD_COMMAND_REGEX.is_match(cmd_string.as_str()) {
checker
.diagnostics
.push(Diagnostic::new(UnixCommandWildcardInjection, func.range()));
}
}
}
}
@@ -0,0 +1,41 @@
---
source: crates/ruff/src/rules/flake8_bandit/mod.rs
---
S609.py:4:1: S609 Possible wildcard injection in call
|
4 | import subprocess
5 |
6 | os.popen("chmod +w foo*")
| ^^^^^^^^ S609
7 | subprocess.Popen("/bin/chown root: *", shell=True)
8 | subprocess.Popen(["/usr/local/bin/rsync", "*", "some_where:"], shell=True)
|

S609.py:5:1: S609 Possible wildcard injection in call
|
5 | os.popen("chmod +w foo*")
6 | subprocess.Popen("/bin/chown root: *", shell=True)
| ^^^^^^^^^^^^^^^^ S609
7 | subprocess.Popen(["/usr/local/bin/rsync", "*", "some_where:"], shell=True)
8 | subprocess.Popen("/usr/local/bin/rsync * no_injection_here:")
|

S609.py:6:1: S609 Possible wildcard injection in call
|
6 | os.popen("chmod +w foo*")
7 | subprocess.Popen("/bin/chown root: *", shell=True)
8 | subprocess.Popen(["/usr/local/bin/rsync", "*", "some_where:"], shell=True)
| ^^^^^^^^^^^^^^^^ S609
9 | subprocess.Popen("/usr/local/bin/rsync * no_injection_here:")
10 | os.system("tar cf foo.tar bar/*")
|

S609.py:8:1: S609 Possible wildcard injection in call
|
8 | subprocess.Popen(["/usr/local/bin/rsync", "*", "some_where:"], shell=True)
9 | subprocess.Popen("/usr/local/bin/rsync * no_injection_here:")
10 | os.system("tar cf foo.tar bar/*")
| ^^^^^^^^^ S609
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 46936b3

Please sign in to comment.