Skip to content

Commit

Permalink
[flake8-bandit] Report all references to suspicious functions (S3) (
Browse files Browse the repository at this point in the history
#15541)

## Summary

Resolves #15522.

## Test Plan

`cargo nextest run` and `cargo insta test`.

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
  • Loading branch information
InSyncWithFoo and dhruvmanila authored Jan 20, 2025
1 parent 4eb465e commit 5cd1f79
Show file tree
Hide file tree
Showing 16 changed files with 789 additions and 30 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import pickle

pickle.loads()


# https://github.com/astral-sh/ruff/issues/15522
map(pickle.load, [])
foo = pickle.load
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,8 @@ def eval(self):

def foo(self):
self.eval() # OK


# https://github.com/astral-sh/ruff/issues/15522
map(eval, [])
foo = eval
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,8 @@ def some_func():
@mark_safe
def some_func():
return '<script>alert("evil!")</script>'


# https://github.com/astral-sh/ruff/issues/15522
map(mark_safe, [])
foo = mark_safe
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,8 @@
urllib.request.urlopen(urllib.request.Request(f'http://www.google.com'))
urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz'))
urllib.request.urlopen(urllib.request.Request(url))


# https://github.com/astral-sh/ruff/issues/15522
map(urllib.request.urlopen, [])
foo = urllib.request.urlopen
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,8 @@
# Unrelated
os.urandom()
a_lib.random()


# https://github.com/astral-sh/ruff/issues/15522
map(random.randrange, [])
foo = random.randrange
13 changes: 13 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bandit/S312.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
from telnetlib import Telnet

Telnet("localhost", 23)


# https://github.com/astral-sh/ruff/issues/15522
map(Telnet, [])
foo = Telnet

import telnetlib
_ = telnetlib.Telnet

from typing import Annotated
foo: Annotated[Telnet, telnetlib.Telnet()]

def _() -> Telnet: ...
53 changes: 53 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,31 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::Airflow3MovedToProvider) {
airflow::rules::moved_to_provider_in_3(checker, expr);
}
if checker.any_enabled(&[
Rule::SuspiciousPickleUsage,
Rule::SuspiciousMarshalUsage,
Rule::SuspiciousInsecureHashUsage,
Rule::SuspiciousInsecureCipherUsage,
Rule::SuspiciousInsecureCipherModeUsage,
Rule::SuspiciousMktempUsage,
Rule::SuspiciousEvalUsage,
Rule::SuspiciousMarkSafeUsage,
Rule::SuspiciousURLOpenUsage,
Rule::SuspiciousNonCryptographicRandomUsage,
Rule::SuspiciousTelnetUsage,
Rule::SuspiciousXMLCElementTreeUsage,
Rule::SuspiciousXMLElementTreeUsage,
Rule::SuspiciousXMLExpatReaderUsage,
Rule::SuspiciousXMLExpatBuilderUsage,
Rule::SuspiciousXMLSaxUsage,
Rule::SuspiciousXMLMiniDOMUsage,
Rule::SuspiciousXMLPullDOMUsage,
Rule::SuspiciousXMLETreeUsage,
Rule::SuspiciousFTPLibUsage,
Rule::SuspiciousUnverifiedContextUsage,
]) {
flake8_bandit::rules::suspicious_function_reference(checker, expr);
}

// Ex) List[...]
if checker.any_enabled(&[
Expand Down Expand Up @@ -315,6 +340,34 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}
Expr::Attribute(attribute) => {
if attribute.ctx == ExprContext::Load {
if checker.any_enabled(&[
Rule::SuspiciousPickleUsage,
Rule::SuspiciousMarshalUsage,
Rule::SuspiciousInsecureHashUsage,
Rule::SuspiciousInsecureCipherUsage,
Rule::SuspiciousInsecureCipherModeUsage,
Rule::SuspiciousMktempUsage,
Rule::SuspiciousEvalUsage,
Rule::SuspiciousMarkSafeUsage,
Rule::SuspiciousURLOpenUsage,
Rule::SuspiciousNonCryptographicRandomUsage,
Rule::SuspiciousTelnetUsage,
Rule::SuspiciousXMLCElementTreeUsage,
Rule::SuspiciousXMLElementTreeUsage,
Rule::SuspiciousXMLExpatReaderUsage,
Rule::SuspiciousXMLExpatBuilderUsage,
Rule::SuspiciousXMLSaxUsage,
Rule::SuspiciousXMLMiniDOMUsage,
Rule::SuspiciousXMLPullDOMUsage,
Rule::SuspiciousXMLETreeUsage,
Rule::SuspiciousFTPLibUsage,
Rule::SuspiciousUnverifiedContextUsage,
]) {
flake8_bandit::rules::suspicious_function_reference(checker, expr);
}
}

// Ex) typing.List[...]
if checker.any_enabled(&[
Rule::FutureRewritableTypeAnnotation,
Expand Down
24 changes: 24 additions & 0 deletions crates/ruff_linter/src/rules/flake8_bandit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ mod tests {

use crate::assert_messages;
use crate::registry::Rule;
use crate::settings::types::PreviewMode;
use crate::settings::LinterSettings;
use crate::test::test_path;

Expand Down Expand Up @@ -82,6 +83,29 @@ mod tests {
Ok(())
}

#[test_case(Rule::SuspiciousPickleUsage, Path::new("S301.py"))]
#[test_case(Rule::SuspiciousEvalUsage, Path::new("S307.py"))]
#[test_case(Rule::SuspiciousMarkSafeUsage, Path::new("S308.py"))]
#[test_case(Rule::SuspiciousURLOpenUsage, Path::new("S310.py"))]
#[test_case(Rule::SuspiciousNonCryptographicRandomUsage, Path::new("S311.py"))]
#[test_case(Rule::SuspiciousTelnetUsage, Path::new("S312.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("flake8_bandit").join(path).as_path(),
&LinterSettings {
preview: PreviewMode::Enabled,
..LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test]
fn check_hardcoded_tmp_additional_dirs() -> Result<()> {
let diagnostics = test_path(
Expand Down
Loading

0 comments on commit 5cd1f79

Please sign in to comment.