[flake8-bandit] Report all references to suspicious functions (S3)

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S301.py

@@ -1,3 +1,8 @@
 import pickle
 
 pickle.loads()
+
+
+# https://github.com/astral-sh/ruff/issues/15522
+map(pickle.load, [])
+foo = pickle.load

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S307.py

@@ -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

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S308.py

@@ -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

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S310.py

@@ -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

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S311.py

@@ -20,3 +20,8 @@
 # Unrelated
 os.urandom()
 a_lib.random()
+
+
+# https://github.com/astral-sh/ruff/issues/15522
+map(random.randrange, [])
+foo = random.randrange

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S312.py

@@ -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: ...

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -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(&[
@@ -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,

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

@@ -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;
 
@@ -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(

crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs

@@ -4,8 +4,8 @@
 use itertools::Either;
 use ruff_diagnostics::{Diagnostic, DiagnosticKind, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
-use ruff_python_ast::{self as ast, Decorator, Expr, ExprCall, Operator};
-use ruff_text_size::Ranged;
+use ruff_python_ast::{self as ast, Arguments, Decorator, Expr, ExprCall, Operator};
+use ruff_text_size::{Ranged, TextRange};
 
 use crate::checkers::ast::Checker;
 use crate::registry::AsRule;
@@ -800,7 +800,7 @@ pub(crate) struct SuspiciousTelnetUsage;
 impl Violation for SuspiciousTelnetUsage {
     #[derive_message_formats]
     fn message(&self) -> String {
-        "Telnet-related functions are being called. Telnet is considered insecure. Use SSH or some other encrypted protocol.".to_string()
+        "Telnet is considered insecure. Use SSH or some other encrypted protocol.".to_string()
     }
 }
 
@@ -825,8 +825,42 @@ impl Violation for SuspiciousFTPLibUsage {
     }
 }
 
-/// S301, S302, S303, S304, S305, S306, S307, S308, S310, S311, S312, S313, S314, S315, S316, S317, S318, S319, S320, S321, S323
 pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
+    suspicious_function(
+        checker,
+        call.func.as_ref(),
+        Some(&call.arguments),
+        call.range,
+    );
+}
+
+pub(crate) fn suspicious_function_reference(checker: &mut Checker, func: &Expr) {
+    if checker.settings.preview.is_disabled() {
+        return;
+    }
+
+    match checker.semantic().current_expression_parent() {
+        Some(Expr::Call(parent)) => {
+            if parent.func.range().contains_range(func.range()) {
+                return;
+            }
+        }
+        Some(Expr::Attribute(_)) => {
+            return;
+        }
+        _ => {}
+    }
+
+    suspicious_function(checker, func, None, func.range());
+}
+
+/// S301, S302, S303, S304, S305, S306, S307, S308, S310, S311, S312, S313, S314, S315, S316, S317, S318, S319, S320, S321, S323
+fn suspicious_function(
+    checker: &mut Checker,
+    func: &Expr,
+    arguments: Option<&Arguments>,
+    range: TextRange,
+) {
     /// Returns `true` if the iterator starts with the given prefix.
     fn has_prefix(mut chars: impl Iterator<Item = char>, prefix: &str) -> bool {
         for expected in prefix.chars() {
@@ -877,7 +911,11 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
         }
     }
 
-    let Some(diagnostic_kind) = checker.semantic().resolve_qualified_name(call.func.as_ref()).and_then(|qualified_name| {
+    if checker.semantic().in_type_definition() {
+        return;
+    }
+
+    let Some(diagnostic_kind) = checker.semantic().resolve_qualified_name(func).and_then(|qualified_name| {
         match qualified_name.segments() {
             // Pickle
             ["pickle" | "dill", "load" | "loads" | "Unpickler"] |
@@ -904,9 +942,13 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
             // URLOpen (`Request`)
             ["urllib", "request", "Request"] |
             ["six", "moves", "urllib", "request", "Request"] => {
+                let Some(arguments) = arguments else {
+                    return Some(SuspiciousURLOpenUsage.into());
+                };
+
                 // If the `url` argument is a string literal or an f-string, allow `http` and `https` schemes.
-                if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
-                    if call.arguments.find_argument_value("url", 0).and_then(leading_chars).is_some_and(has_http_prefix) {
+                if arguments.args.iter().all(|arg| !arg.is_starred_expr()) && arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
+                    if arguments.find_argument_value("url", 0).and_then(leading_chars).is_some_and(has_http_prefix) {
                         return None;
                     }
                 }
@@ -915,8 +957,12 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
             // URLOpen (`urlopen`, `urlretrieve`)
             ["urllib", "request", "urlopen" | "urlretrieve" ] |
             ["six", "moves", "urllib", "request", "urlopen" | "urlretrieve" ] => {
-                if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
-                    match call.arguments.find_argument_value("url", 0) {
+                let Some(arguments) = arguments else {
+                    return Some(SuspiciousURLOpenUsage.into());
+                };
+
+                if arguments.args.iter().all(|arg| !arg.is_starred_expr()) && arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
+                    match arguments.find_argument_value("url", 0) {
                         // If the `url` argument is a `urllib.request.Request` object, allow `http` and `https` schemes.
                         Some(Expr::Call(ExprCall { func, arguments, .. })) => {
                             if checker.semantic().resolve_qualified_name(func.as_ref()).is_some_and(|name| name.segments() == ["urllib", "request", "Request"]) {
@@ -962,41 +1008,25 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
             // XMLETree
             ["lxml", "etree", "parse" | "fromstring" | "RestrictedElement" | "GlobalParserTLS" | "getDefaultParser" | "check_docinfo"] => Some(SuspiciousXMLETreeUsage.into()),
             // Telnet
-            ["telnetlib", ..] => Some(SuspiciousTelnetUsage.into()),
+            ["telnetlib", _, ..] => Some(SuspiciousTelnetUsage.into()),
             // FTPLib
-            ["ftplib", ..] => Some(SuspiciousFTPLibUsage.into()),
+            ["ftplib", _, ..] => Some(SuspiciousFTPLibUsage.into()),
             _ => None
         }
     }) else {
         return;
     };
 
-    let diagnostic = Diagnostic::new::<DiagnosticKind>(diagnostic_kind, call.range());
+    let diagnostic = Diagnostic::new::<DiagnosticKind>(diagnostic_kind, range);
     if checker.enabled(diagnostic.kind.rule()) {
         checker.diagnostics.push(diagnostic);
     }
 }
 
 /// S308
 pub(crate) fn suspicious_function_decorator(checker: &mut Checker, decorator: &Decorator) {
-    let Some(diagnostic_kind) = checker
-        .semantic()
-        .resolve_qualified_name(&decorator.expression)
-        .and_then(|qualified_name| {
-            match qualified_name.segments() {
-                // MarkSafe
-                ["django", "utils", "safestring" | "html", "mark_safe"] => {
-                    Some(SuspiciousMarkSafeUsage.into())
-                }
-                _ => None,
-            }
-        })
-    else {
-        return;
-    };
-
-    let diagnostic = Diagnostic::new::<DiagnosticKind>(diagnostic_kind, decorator.range());
-    if checker.enabled(diagnostic.kind.rule()) {
-        checker.diagnostics.push(diagnostic);
+    // In preview mode, references are handled collectively by `suspicious_function_reference`
+    if checker.settings.preview.is_disabled() {
+        suspicious_function(checker, &decorator.expression, None, decorator.range);
     }
 }

crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S312_S312.py.snap

@@ -1,10 +1,19 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
 ---
-S312.py:3:1: S312 Telnet-related functions are being called. Telnet is considered insecure. Use SSH or some other encrypted protocol.
+S312.py:3:1: S312 Telnet is considered insecure. Use SSH or some other encrypted protocol.
   |
 1 | from telnetlib import Telnet
 2 |
 3 | Telnet("localhost", 23)
   | ^^^^^^^^^^^^^^^^^^^^^^^ S312
   |
+
+S312.py:14:24: S312 Telnet is considered insecure. Use SSH or some other encrypted protocol.
+   |
+13 | from typing import Annotated
+14 | foo: Annotated[Telnet, telnetlib.Telnet()]
+   |                        ^^^^^^^^^^^^^^^^^^ S312
+15 |
+16 | def _() -> Telnet: ...
+   |

crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__preview__S301_S301.py.snap

@@ -0,0 +1,26 @@
+---
+source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
+---
+S301.py:3:1: S301 `pickle` and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue
+  |
+1 | import pickle
+2 |
+3 | pickle.loads()
+  | ^^^^^^^^^^^^^^ S301
+  |
+
+S301.py:7:5: S301 `pickle` and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue
+  |
+6 | # https://github.com/astral-sh/ruff/issues/15522
+7 | map(pickle.load, [])
+  |     ^^^^^^^^^^^ S301
+8 | foo = pickle.load
+  |
+
+S301.py:8:7: S301 `pickle` and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue
+  |
+6 | # https://github.com/astral-sh/ruff/issues/15522
+7 | map(pickle.load, [])
+8 | foo = pickle.load
+  |       ^^^^^^^^^^^ S301
+  |