[flake8-bandit] Add Rule for S324 (Insecure hash functions in hashlib) (#1661)

ref: https://github.com/charliermarsh/ruff/issues/1646

README.md

@@ -771,6 +771,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/4.1.1/) on
 | S106 | HardcodedPasswordFuncArg | Possible hardcoded password: `"..."` |  |
 | S107 | HardcodedPasswordDefault | Possible hardcoded password: `"..."` |  |
 | S108 | HardcodedTempFile | Probable insecure usage of temp file/directory: `"..."` |  |
+| S324 | HashlibInsecureHashFunction | Probable use of insecure hash functions in hashlib: `"..."` |  |
 
 ### flake8-blind-except (BLE)
 

resources/test/fixtures/flake8_bandit/S324.py

@@ -0,0 +1,52 @@
+import hashlib
+from hashlib import new as hashlib_new
+from hashlib import sha1 as hashlib_sha1
+
+# Invalid
+
+hashlib.new('md5')
+
+hashlib.new('md4', b'test')
+
+hashlib.new(name='md5', data=b'test')
+
+hashlib.new('MD4', data=b'test')
+
+hashlib.new('sha1')
+
+hashlib.new('sha1', data=b'test')
+
+hashlib.new('sha', data=b'test')
+
+hashlib.new(name='SHA', data=b'test')
+
+hashlib.sha(data=b'test')
+
+hashlib.md5()
+
+hashlib_new('sha1')
+
+hashlib_sha1('sha1')
+
+# usedforsecurity arg only available in Python 3.9+
+hashlib.new('sha1', usedforsecurity=True)
+
+# Valid
+
+hashlib.new('sha256')
+
+hashlib.new('SHA512')
+
+hashlib.sha256(data=b'test')
+
+# usedforsecurity arg only available in Python 3.9+
+hashlib_new(name='sha1', usedforsecurity=False)
+
+# usedforsecurity arg only available in Python 3.9+
+hashlib_sha1(name='sha1', usedforsecurity=False)
+
+# usedforsecurity arg only available in Python 3.9+
+hashlib.md4(usedforsecurity=False)
+
+# usedforsecurity arg only available in Python 3.9+
+hashlib.new(name='sha256', usedforsecurity=False)

ruff.schema.json

@@ -927,6 +927,9 @@
         "S106",
         "S107",
         "S108",
+        "S3",
+        "S32",
+        "S324",
         "SIM",
         "SIM1",
         "SIM10",

src/ast/helpers.rs

@@ -582,7 +582,7 @@ pub struct SimpleCallArgs<'a> {
 }
 
 impl<'a> SimpleCallArgs<'a> {
-    pub fn new(args: &'a Vec<Expr>, keywords: &'a Vec<Keyword>) -> Self {
+    pub fn new(args: &'a [Expr], keywords: &'a [Keyword]) -> Self {
         let mut result = SimpleCallArgs::default();
 
         for arg in args {

src/checkers/ast.rs

@@ -1901,6 +1901,17 @@ where
                         flake8_bandit::checks::hardcoded_password_func_arg(keywords).into_iter(),
                     );
                 }
+                if self.settings.enabled.contains(&CheckCode::S324) {
+                    if let Some(check) = flake8_bandit::checks::hashlib_insecure_hash_functions(
+                        func,
+                        args,
+                        keywords,
+                        &self.from_imports,
+                        &self.import_aliases,
+                    ) {
+                        self.add_check(check);
+                    }
+                }
 
                 // flake8-comprehensions
                 if self.settings.enabled.contains(&CheckCode::C400) {

src/flake8_bandit/checks/bad_file_permissions.rs

@@ -86,8 +86,8 @@ fn get_int_value(expr: &Expr) -> Option<u16> {
 /// S103
 pub fn bad_file_permissions(
     func: &Expr,
-    args: &Vec<Expr>,
-    keywords: &Vec<Keyword>,
+    args: &[Expr],
+    keywords: &[Keyword],
     from_imports: &FxHashMap<&str, FxHashSet<&str>>,
     import_aliases: &FxHashMap<&str, &str>,
 ) -> Option<Check> {

src/flake8_bandit/checks/hashlib_insecure_hash_functions.rs

@@ -0,0 +1,66 @@
+use rustc_hash::{FxHashMap, FxHashSet};
+use rustpython_ast::{Constant, Expr, ExprKind, Keyword};
+
+use crate::ast::helpers::{match_module_member, SimpleCallArgs};
+use crate::ast::types::Range;
+use crate::flake8_bandit::helpers::string_literal;
+use crate::registry::{Check, CheckKind};
+
+const WEAK_HASHES: [&str; 4] = ["md4", "md5", "sha", "sha1"];
+
+fn is_used_for_security(call_args: &SimpleCallArgs) -> bool {
+    match call_args.get_argument("usedforsecurity", None) {
+        Some(expr) => !matches!(
+            &expr.node,
+            ExprKind::Constant {
+                value: Constant::Bool(false),
+                ..
+            }
+        ),
+        _ => true,
+    }
+}
+
+/// S324
+pub fn hashlib_insecure_hash_functions(
+    func: &Expr,
+    args: &[Expr],
+    keywords: &[Keyword],
+    from_imports: &FxHashMap<&str, FxHashSet<&str>>,
+    import_aliases: &FxHashMap<&str, &str>,
+) -> Option<Check> {
+    if match_module_member(func, "hashlib", "new", from_imports, import_aliases) {
+        let call_args = SimpleCallArgs::new(args, keywords);
+
+        if !is_used_for_security(&call_args) {
+            return None;
+        }
+
+        if let Some(name_arg) = call_args.get_argument("name", Some(0)) {
+            let hash_func_name = string_literal(name_arg)?;
+
+            if WEAK_HASHES.contains(&hash_func_name.to_lowercase().as_str()) {
+                return Some(Check::new(
+                    CheckKind::HashlibInsecureHashFunction(hash_func_name.to_string()),
+                    Range::from_located(name_arg),
+                ));
+            }
+        }
+    } else {
+        for func_name in &WEAK_HASHES {
+            if match_module_member(func, "hashlib", func_name, from_imports, import_aliases) {
+                let call_args = SimpleCallArgs::new(args, keywords);
+
+                if !is_used_for_security(&call_args) {
+                    return None;
+                }
+
+                return Some(Check::new(
+                    CheckKind::HashlibInsecureHashFunction((*func_name).to_string()),
+                    Range::from_located(func),
+                ));
+            }
+        }
+    }
+    None
+}

src/flake8_bandit/checks/mod.rs

@@ -8,6 +8,7 @@ pub use hardcoded_password_string::{
     assign_hardcoded_password_string, compare_to_hardcoded_password_string,
 };
 pub use hardcoded_tmp_directory::hardcoded_tmp_directory;
+pub use hashlib_insecure_hash_functions::hashlib_insecure_hash_functions;
 
 mod assert_used;
 mod bad_file_permissions;
@@ -17,3 +18,4 @@ mod hardcoded_password_default;
 mod hardcoded_password_func_arg;
 mod hardcoded_password_string;
 mod hardcoded_tmp_directory;
+mod hashlib_insecure_hash_functions;

src/flake8_bandit/mod.rs

@@ -21,6 +21,7 @@ mod tests {
     #[test_case(CheckCode::S106, Path::new("S106.py"); "S106")]
     #[test_case(CheckCode::S107, Path::new("S107.py"); "S107")]
     #[test_case(CheckCode::S108, Path::new("S108.py"); "S108")]
+    #[test_case(CheckCode::S324, Path::new("S324.py"); "S324")]
     fn checks(check_code: CheckCode, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
         let checks = test_path(

src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S324_S324.py.snap

@@ -0,0 +1,135 @@
+---
+source: src/flake8_bandit/mod.rs
+expression: checks
+---
+- kind:
+    HashlibInsecureHashFunction: md5
+  location:
+    row: 7
+    column: 12
+  end_location:
+    row: 7
+    column: 17
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: md4
+  location:
+    row: 9
+    column: 12
+  end_location:
+    row: 9
+    column: 17
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: md5
+  location:
+    row: 11
+    column: 17
+  end_location:
+    row: 11
+    column: 22
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: MD4
+  location:
+    row: 13
+    column: 12
+  end_location:
+    row: 13
+    column: 17
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: sha1
+  location:
+    row: 15
+    column: 12
+  end_location:
+    row: 15
+    column: 18
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: sha1
+  location:
+    row: 17
+    column: 12
+  end_location:
+    row: 17
+    column: 18
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: sha
+  location:
+    row: 19
+    column: 12
+  end_location:
+    row: 19
+    column: 17
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: SHA
+  location:
+    row: 21
+    column: 17
+  end_location:
+    row: 21
+    column: 22
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: sha
+  location:
+    row: 23
+    column: 0
+  end_location:
+    row: 23
+    column: 11
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: md5
+  location:
+    row: 25
+    column: 0
+  end_location:
+    row: 25
+    column: 11
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: sha1
+  location:
+    row: 27
+    column: 12
+  end_location:
+    row: 27
+    column: 18
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: sha1
+  location:
+    row: 29
+    column: 0
+  end_location:
+    row: 29
+    column: 12
+  fix: ~
+  parent: ~
+- kind:
+    HashlibInsecureHashFunction: sha1
+  location:
+    row: 32
+    column: 12
+  end_location:
+    row: 32
+    column: 18
+  fix: ~
+  parent: ~
+

src/flake8_pytest_style/plugins/fail.rs

@@ -6,7 +6,7 @@ use crate::ast::types::Range;
 use crate::checkers::ast::Checker;
 use crate::registry::{Check, CheckKind};
 
-pub fn fail_call(checker: &mut Checker, call: &Expr, args: &Vec<Expr>, keywords: &Vec<Keyword>) {
+pub fn fail_call(checker: &mut Checker, call: &Expr, args: &[Expr], keywords: &[Keyword]) {
     if is_pytest_fail(call, checker) {
         let call_args = SimpleCallArgs::new(args, keywords);
         let msg = call_args.get_argument("msg", Some(0));

src/flake8_pytest_style/plugins/patch.rs

@@ -52,8 +52,8 @@ where
 
 fn check_patch_call(
     call: &Expr,
-    args: &Vec<Expr>,
-    keywords: &Vec<Keyword>,
+    args: &[Expr],
+    keywords: &[Keyword],
     new_arg_number: usize,
 ) -> Option<Check> {
     let simple_args = SimpleCallArgs::new(args, keywords);
@@ -81,7 +81,7 @@ fn check_patch_call(
     None
 }
 
-pub fn patch_with_lambda(call: &Expr, args: &Vec<Expr>, keywords: &Vec<Keyword>) -> Option<Check> {
+pub fn patch_with_lambda(call: &Expr, args: &[Expr], keywords: &[Keyword]) -> Option<Check> {
     if let Some(call_path) = compose_call_path(call) {
         if PATCH_NAMES.contains(&call_path.as_str()) {
             check_patch_call(call, args, keywords, 1)

src/registry.rs

@@ -334,6 +334,7 @@ pub enum CheckCode {
     S106,
     S107,
     S108,
+    S324,
     // flake8-boolean-trap
     FBT001,
     FBT002,
@@ -1073,6 +1074,7 @@ pub enum CheckKind {
     HardcodedPasswordFuncArg(String),
     HardcodedPasswordDefault(String),
     HardcodedTempFile(String),
+    HashlibInsecureHashFunction(String),
     // mccabe
     FunctionIsTooComplex(String, usize),
     // flake8-boolean-trap
@@ -1535,6 +1537,7 @@ impl CheckCode {
             CheckCode::S106 => CheckKind::HardcodedPasswordFuncArg("...".to_string()),
             CheckCode::S107 => CheckKind::HardcodedPasswordDefault("...".to_string()),
             CheckCode::S108 => CheckKind::HardcodedTempFile("...".to_string()),
+            CheckCode::S324 => CheckKind::HashlibInsecureHashFunction("...".to_string()),
             // mccabe
             CheckCode::C901 => CheckKind::FunctionIsTooComplex("...".to_string(), 10),
             // flake8-boolean-trap
@@ -1937,6 +1940,7 @@ impl CheckCode {
             CheckCode::S106 => CheckCategory::Flake8Bandit,
             CheckCode::S107 => CheckCategory::Flake8Bandit,
             CheckCode::S108 => CheckCategory::Flake8Bandit,
+            CheckCode::S324 => CheckCategory::Flake8Bandit,
             // flake8-simplify
             CheckCode::SIM102 => CheckCategory::Flake8Simplify,
             CheckCode::SIM105 => CheckCategory::Flake8Simplify,
@@ -2312,6 +2316,7 @@ impl CheckKind {
             CheckKind::HardcodedPasswordFuncArg(..) => &CheckCode::S106,
             CheckKind::HardcodedPasswordDefault(..) => &CheckCode::S107,
             CheckKind::HardcodedTempFile(..) => &CheckCode::S108,
+            CheckKind::HashlibInsecureHashFunction(..) => &CheckCode::S324,
             // mccabe
             CheckKind::FunctionIsTooComplex(..) => &CheckCode::C901,
             // flake8-boolean-trap
@@ -3266,6 +3271,12 @@ impl CheckKind {
                     string.escape_debug()
                 )
             }
+            CheckKind::HashlibInsecureHashFunction(string) => {
+                format!(
+                    "Probable use of insecure hash functions in hashlib: `\"{}\"`",
+                    string.escape_debug()
+                )
+            }
             // flake8-blind-except
             CheckKind::BlindExcept(name) => format!("Do not catch blind exception: `{name}`"),
             // mccabe