diff --git a/README.md b/README.md index 94f59937c21f8..2365d9aca091f 100644 --- a/README.md +++ b/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) diff --git a/resources/test/fixtures/flake8_bandit/S324.py b/resources/test/fixtures/flake8_bandit/S324.py new file mode 100644 index 0000000000000..bea55067ea77e --- /dev/null +++ b/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) diff --git a/ruff.schema.json b/ruff.schema.json index 2b97da8dc75bb..f3de63daa7dce 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -927,6 +927,9 @@ "S106", "S107", "S108", + "S3", + "S32", + "S324", "SIM", "SIM1", "SIM10", diff --git a/src/ast/helpers.rs b/src/ast/helpers.rs index 78a0d88edf2d8..0c16f4bccadc4 100644 --- a/src/ast/helpers.rs +++ b/src/ast/helpers.rs @@ -582,7 +582,7 @@ pub struct SimpleCallArgs<'a> { } impl<'a> SimpleCallArgs<'a> { - pub fn new(args: &'a Vec, keywords: &'a Vec) -> Self { + pub fn new(args: &'a [Expr], keywords: &'a [Keyword]) -> Self { let mut result = SimpleCallArgs::default(); for arg in args { diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 4eae376a12671..e32bbe32160f7 100644 --- a/src/checkers/ast.rs +++ b/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) { diff --git a/src/flake8_bandit/checks/bad_file_permissions.rs b/src/flake8_bandit/checks/bad_file_permissions.rs index 0efb572bd29fb..ce6a4d4f91228 100644 --- a/src/flake8_bandit/checks/bad_file_permissions.rs +++ b/src/flake8_bandit/checks/bad_file_permissions.rs @@ -86,8 +86,8 @@ fn get_int_value(expr: &Expr) -> Option { /// S103 pub fn bad_file_permissions( func: &Expr, - args: &Vec, - keywords: &Vec, + args: &[Expr], + keywords: &[Keyword], from_imports: &FxHashMap<&str, FxHashSet<&str>>, import_aliases: &FxHashMap<&str, &str>, ) -> Option { diff --git a/src/flake8_bandit/checks/hashlib_insecure_hash_functions.rs b/src/flake8_bandit/checks/hashlib_insecure_hash_functions.rs new file mode 100644 index 0000000000000..5d27c0fdc4d10 --- /dev/null +++ b/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 { + 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 +} diff --git a/src/flake8_bandit/checks/mod.rs b/src/flake8_bandit/checks/mod.rs index 6a1bb66f2256f..785ee5cd69070 100644 --- a/src/flake8_bandit/checks/mod.rs +++ b/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; diff --git a/src/flake8_bandit/mod.rs b/src/flake8_bandit/mod.rs index f7dc93c6ae209..117d78f42eff7 100644 --- a/src/flake8_bandit/mod.rs +++ b/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( diff --git a/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S324_S324.py.snap b/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S324_S324.py.snap new file mode 100644 index 0000000000000..913c3486c6004 --- /dev/null +++ b/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: ~ + diff --git a/src/flake8_pytest_style/plugins/fail.rs b/src/flake8_pytest_style/plugins/fail.rs index 39e4c24b77d6d..3164501fbf6de 100644 --- a/src/flake8_pytest_style/plugins/fail.rs +++ b/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, keywords: &Vec) { +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)); diff --git a/src/flake8_pytest_style/plugins/patch.rs b/src/flake8_pytest_style/plugins/patch.rs index 5ec7731109fbb..98c6a48f8d55a 100644 --- a/src/flake8_pytest_style/plugins/patch.rs +++ b/src/flake8_pytest_style/plugins/patch.rs @@ -52,8 +52,8 @@ where fn check_patch_call( call: &Expr, - args: &Vec, - keywords: &Vec, + args: &[Expr], + keywords: &[Keyword], new_arg_number: usize, ) -> Option { 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, keywords: &Vec) -> Option { +pub fn patch_with_lambda(call: &Expr, args: &[Expr], keywords: &[Keyword]) -> Option { if let Some(call_path) = compose_call_path(call) { if PATCH_NAMES.contains(&call_path.as_str()) { check_patch_call(call, args, keywords, 1) diff --git a/src/registry.rs b/src/registry.rs index 39c963d60ca0c..0cd8de155f87d 100644 --- a/src/registry.rs +++ b/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