From 4b48d08a127388f13b22db93d96955adc4cc39c1 Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Sat, 9 Mar 2024 12:53:45 +0100 Subject: [PATCH] feat(rules): flag weak hashes in `crypt` for `S324` Ref: https://github.com/PyCQA/bandit/pull/1018 --- .../test/fixtures/flake8_bandit/S324.py | 18 ++ .../rules/hashlib_insecure_hash_functions.rs | 67 +++++- ...s__flake8_bandit__tests__S324_S324.py.snap | 226 +++++++++++------- 3 files changed, 223 insertions(+), 88 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S324.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S324.py index 8cb3f70d1f301b..c3cb83e4d386df 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S324.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S324.py @@ -1,3 +1,4 @@ +import crypt import hashlib from hashlib import new as hashlib_new from hashlib import sha1 as hashlib_sha1 @@ -18,6 +19,15 @@ # usedforsecurity arg only available in Python 3.9+ hashlib.new('sha1', usedforsecurity=True) +crypt.crypt("test", salt=crypt.METHOD_CRYPT) +crypt.crypt("test", salt=crypt.METHOD_MD5) +crypt.crypt("test", salt=crypt.METHOD_BLOWFISH) +crypt.crypt("test", crypt.METHOD_BLOWFISH) + +crypt.mksalt(crypt.METHOD_CRYPT) +crypt.mksalt(crypt.METHOD_MD5) +crypt.mksalt(crypt.METHOD_BLOWFISH) + # OK hashlib.new('sha256') hashlib.new('SHA512') @@ -27,3 +37,11 @@ hashlib_sha1(name='sha1', usedforsecurity=False) hashlib.md4(usedforsecurity=False) hashlib.new(name='sha256', usedforsecurity=False) + +crypt.crypt("test") +crypt.crypt("test", salt=crypt.METHOD_SHA256) +crypt.crypt("test", salt=crypt.METHOD_SHA512) + +crypt.mksalt() +crypt.mksalt(crypt.METHOD_SHA256) +crypt.mksalt(crypt.METHOD_SHA512) diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs index d38832f6034a92..436d59dd2f371c 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs @@ -9,7 +9,8 @@ use crate::checkers::ast::Checker; use super::super::helpers::string_literal; /// ## What it does -/// Checks for uses of weak or broken cryptographic hash functions. +/// Checks for uses of weak or broken cryptographic hash functions in +/// `hashlib` and `crypt` libraries. /// /// ## Why is this bad? /// Weak or broken cryptographic hash functions may be susceptible to @@ -43,6 +44,7 @@ use super::super::helpers::string_literal; /// /// ## References /// - [Python documentation: `hashlib` — Secure hashes and message digests](https://docs.python.org/3/library/hashlib.html) +/// - [Python documentation: `crypt` — Function to check Unix passwords](https://docs.python.org/3/library/crypt.html) /// - [Common Weakness Enumeration: CWE-327](https://cwe.mitre.org/data/definitions/327.html) /// - [Common Weakness Enumeration: CWE-328](https://cwe.mitre.org/data/definitions/328.html) /// - [Common Weakness Enumeration: CWE-916](https://cwe.mitre.org/data/definitions/916.html) @@ -62,19 +64,35 @@ impl Violation for HashlibInsecureHashFunction { /// S324 pub(crate) fn hashlib_insecure_hash_functions(checker: &mut Checker, call: &ast::ExprCall) { - if let Some(hashlib_call) = checker + if let Some(weak_hash_call) = checker .semantic() .resolve_qualified_name(&call.func) .and_then(|qualified_name| match qualified_name.segments() { - ["hashlib", "new"] => Some(HashlibCall::New), - ["hashlib", "md4"] => Some(HashlibCall::WeakHash("md4")), - ["hashlib", "md5"] => Some(HashlibCall::WeakHash("md5")), - ["hashlib", "sha"] => Some(HashlibCall::WeakHash("sha")), - ["hashlib", "sha1"] => Some(HashlibCall::WeakHash("sha1")), + ["hashlib", "new"] => Some(WeakHashCall::Hashlib { + call: HashlibCall::New, + }), + ["hashlib", "md4"] => Some(WeakHashCall::Hashlib { + call: HashlibCall::WeakHash("md4"), + }), + ["hashlib", "md5"] => Some(WeakHashCall::Hashlib { + call: HashlibCall::WeakHash("md5"), + }), + ["hashlib", "sha"] => Some(WeakHashCall::Hashlib { + call: HashlibCall::WeakHash("sha"), + }), + ["hashlib", "sha1"] => Some(WeakHashCall::Hashlib { + call: HashlibCall::WeakHash("sha1"), + }), + ["crypt", "crypt" | "mksalt"] => Some(WeakHashCall::Crypt), _ => None, }) { - detect_insecure_hashlib_calls(checker, call, &hashlib_call); + match weak_hash_call { + WeakHashCall::Hashlib { call: hashlib_call } => { + detect_insecure_hashlib_calls(checker, call, &hashlib_call); + } + WeakHashCall::Crypt => detect_insecure_crypt_calls(checker, call), + } } } @@ -120,12 +138,45 @@ fn detect_insecure_hashlib_calls( } } +fn detect_insecure_crypt_calls(checker: &mut Checker, call: &ast::ExprCall) { + if let Some((argument_name, position)) = checker + .semantic() + .resolve_qualified_name(&call.func) + .and_then(|qualified_name| match qualified_name.segments() { + ["crypt", "crypt"] => Some(("salt", 1)), + ["crypt", "mksalt"] => Some(("method", 0)), + _ => None, + }) + { + if let Some(method) = call.arguments.find_argument(argument_name, position) { + if let Some(qualified_name) = checker.semantic().resolve_qualified_name(method) { + if let ["crypt", "METHOD_CRYPT" | "METHOD_MD5" | "METHOD_BLOWFISH"] = + qualified_name.segments() + { + checker.diagnostics.push(Diagnostic::new( + HashlibInsecureHashFunction { + library: "crypt".to_string(), + string: qualified_name.to_string(), + }, + method.range(), + )); + } + } + } + } +} + fn is_used_for_security(arguments: &Arguments) -> bool { arguments .find_keyword("usedforsecurity") .map_or(true, |keyword| !is_const_false(&keyword.value)) } +enum WeakHashCall { + Hashlib { call: HashlibCall }, + Crypt, +} + #[derive(Debug)] enum HashlibCall { New, diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S324_S324.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S324_S324.py.snap index 4e8fd2450007d8..1a521d8ce44aa4 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S324_S324.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S324_S324.py.snap @@ -1,131 +1,197 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S324.py:6:13: S324 Probable use of insecure hash functions in `hashlib`: `md5` +S324.py:7:13: S324 Probable use of insecure hash functions in `hashlib`: `md5` | -5 | # Errors -6 | hashlib.new('md5') +6 | # Errors +7 | hashlib.new('md5') | ^^^^^ S324 -7 | hashlib.new('md4', b'test') -8 | hashlib.new(name='md5', data=b'test') +8 | hashlib.new('md4', b'test') +9 | hashlib.new(name='md5', data=b'test') | -S324.py:7:13: S324 Probable use of insecure hash functions in `hashlib`: `md4` - | -5 | # Errors -6 | hashlib.new('md5') -7 | hashlib.new('md4', b'test') - | ^^^^^ S324 -8 | hashlib.new(name='md5', data=b'test') -9 | hashlib.new('MD4', data=b'test') - | +S324.py:8:13: S324 Probable use of insecure hash functions in `hashlib`: `md4` + | + 6 | # Errors + 7 | hashlib.new('md5') + 8 | hashlib.new('md4', b'test') + | ^^^^^ S324 + 9 | hashlib.new(name='md5', data=b'test') +10 | hashlib.new('MD4', data=b'test') + | -S324.py:8:18: S324 Probable use of insecure hash functions in `hashlib`: `md5` +S324.py:9:18: S324 Probable use of insecure hash functions in `hashlib`: `md5` | - 6 | hashlib.new('md5') - 7 | hashlib.new('md4', b'test') - 8 | hashlib.new(name='md5', data=b'test') + 7 | hashlib.new('md5') + 8 | hashlib.new('md4', b'test') + 9 | hashlib.new(name='md5', data=b'test') | ^^^^^ S324 - 9 | hashlib.new('MD4', data=b'test') -10 | hashlib.new('sha1') +10 | hashlib.new('MD4', data=b'test') +11 | hashlib.new('sha1') | -S324.py:9:13: S324 Probable use of insecure hash functions in `hashlib`: `MD4` +S324.py:10:13: S324 Probable use of insecure hash functions in `hashlib`: `MD4` | - 7 | hashlib.new('md4', b'test') - 8 | hashlib.new(name='md5', data=b'test') - 9 | hashlib.new('MD4', data=b'test') + 8 | hashlib.new('md4', b'test') + 9 | hashlib.new(name='md5', data=b'test') +10 | hashlib.new('MD4', data=b'test') | ^^^^^ S324 -10 | hashlib.new('sha1') -11 | hashlib.new('sha1', data=b'test') +11 | hashlib.new('sha1') +12 | hashlib.new('sha1', data=b'test') | -S324.py:10:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` +S324.py:11:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` | - 8 | hashlib.new(name='md5', data=b'test') - 9 | hashlib.new('MD4', data=b'test') -10 | hashlib.new('sha1') + 9 | hashlib.new(name='md5', data=b'test') +10 | hashlib.new('MD4', data=b'test') +11 | hashlib.new('sha1') | ^^^^^^ S324 -11 | hashlib.new('sha1', data=b'test') -12 | hashlib.new('sha', data=b'test') +12 | hashlib.new('sha1', data=b'test') +13 | hashlib.new('sha', data=b'test') | -S324.py:11:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` +S324.py:12:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` | - 9 | hashlib.new('MD4', data=b'test') -10 | hashlib.new('sha1') -11 | hashlib.new('sha1', data=b'test') +10 | hashlib.new('MD4', data=b'test') +11 | hashlib.new('sha1') +12 | hashlib.new('sha1', data=b'test') | ^^^^^^ S324 -12 | hashlib.new('sha', data=b'test') -13 | hashlib.new(name='SHA', data=b'test') +13 | hashlib.new('sha', data=b'test') +14 | hashlib.new(name='SHA', data=b'test') | -S324.py:12:13: S324 Probable use of insecure hash functions in `hashlib`: `sha` +S324.py:13:13: S324 Probable use of insecure hash functions in `hashlib`: `sha` | -10 | hashlib.new('sha1') -11 | hashlib.new('sha1', data=b'test') -12 | hashlib.new('sha', data=b'test') +11 | hashlib.new('sha1') +12 | hashlib.new('sha1', data=b'test') +13 | hashlib.new('sha', data=b'test') | ^^^^^ S324 -13 | hashlib.new(name='SHA', data=b'test') -14 | hashlib.sha(data=b'test') +14 | hashlib.new(name='SHA', data=b'test') +15 | hashlib.sha(data=b'test') | -S324.py:13:18: S324 Probable use of insecure hash functions in `hashlib`: `SHA` +S324.py:14:18: S324 Probable use of insecure hash functions in `hashlib`: `SHA` | -11 | hashlib.new('sha1', data=b'test') -12 | hashlib.new('sha', data=b'test') -13 | hashlib.new(name='SHA', data=b'test') +12 | hashlib.new('sha1', data=b'test') +13 | hashlib.new('sha', data=b'test') +14 | hashlib.new(name='SHA', data=b'test') | ^^^^^ S324 -14 | hashlib.sha(data=b'test') -15 | hashlib.md5() +15 | hashlib.sha(data=b'test') +16 | hashlib.md5() | -S324.py:14:1: S324 Probable use of insecure hash functions in `hashlib`: `sha` +S324.py:15:1: S324 Probable use of insecure hash functions in `hashlib`: `sha` | -12 | hashlib.new('sha', data=b'test') -13 | hashlib.new(name='SHA', data=b'test') -14 | hashlib.sha(data=b'test') +13 | hashlib.new('sha', data=b'test') +14 | hashlib.new(name='SHA', data=b'test') +15 | hashlib.sha(data=b'test') | ^^^^^^^^^^^ S324 -15 | hashlib.md5() -16 | hashlib_new('sha1') +16 | hashlib.md5() +17 | hashlib_new('sha1') | -S324.py:15:1: S324 Probable use of insecure hash functions in `hashlib`: `md5` +S324.py:16:1: S324 Probable use of insecure hash functions in `hashlib`: `md5` | -13 | hashlib.new(name='SHA', data=b'test') -14 | hashlib.sha(data=b'test') -15 | hashlib.md5() +14 | hashlib.new(name='SHA', data=b'test') +15 | hashlib.sha(data=b'test') +16 | hashlib.md5() | ^^^^^^^^^^^ S324 -16 | hashlib_new('sha1') -17 | hashlib_sha1('sha1') +17 | hashlib_new('sha1') +18 | hashlib_sha1('sha1') | -S324.py:16:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` +S324.py:17:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` | -14 | hashlib.sha(data=b'test') -15 | hashlib.md5() -16 | hashlib_new('sha1') +15 | hashlib.sha(data=b'test') +16 | hashlib.md5() +17 | hashlib_new('sha1') | ^^^^^^ S324 -17 | hashlib_sha1('sha1') -18 | # usedforsecurity arg only available in Python 3.9+ +18 | hashlib_sha1('sha1') +19 | # usedforsecurity arg only available in Python 3.9+ | -S324.py:17:1: S324 Probable use of insecure hash functions in `hashlib`: `sha1` +S324.py:18:1: S324 Probable use of insecure hash functions in `hashlib`: `sha1` | -15 | hashlib.md5() -16 | hashlib_new('sha1') -17 | hashlib_sha1('sha1') +16 | hashlib.md5() +17 | hashlib_new('sha1') +18 | hashlib_sha1('sha1') | ^^^^^^^^^^^^ S324 -18 | # usedforsecurity arg only available in Python 3.9+ -19 | hashlib.new('sha1', usedforsecurity=True) +19 | # usedforsecurity arg only available in Python 3.9+ +20 | hashlib.new('sha1', usedforsecurity=True) | -S324.py:19:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` +S324.py:20:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` | -17 | hashlib_sha1('sha1') -18 | # usedforsecurity arg only available in Python 3.9+ -19 | hashlib.new('sha1', usedforsecurity=True) +18 | hashlib_sha1('sha1') +19 | # usedforsecurity arg only available in Python 3.9+ +20 | hashlib.new('sha1', usedforsecurity=True) | ^^^^^^ S324 -20 | -21 | # OK +21 | +22 | crypt.crypt("test", salt=crypt.METHOD_CRYPT) + | + +S324.py:22:26: S324 Probable use of insecure hash functions in `crypt`: `crypt.METHOD_CRYPT` + | +20 | hashlib.new('sha1', usedforsecurity=True) +21 | +22 | crypt.crypt("test", salt=crypt.METHOD_CRYPT) + | ^^^^^^^^^^^^^^^^^^ S324 +23 | crypt.crypt("test", salt=crypt.METHOD_MD5) +24 | crypt.crypt("test", salt=crypt.METHOD_BLOWFISH) + | + +S324.py:23:26: S324 Probable use of insecure hash functions in `crypt`: `crypt.METHOD_MD5` + | +22 | crypt.crypt("test", salt=crypt.METHOD_CRYPT) +23 | crypt.crypt("test", salt=crypt.METHOD_MD5) + | ^^^^^^^^^^^^^^^^ S324 +24 | crypt.crypt("test", salt=crypt.METHOD_BLOWFISH) +25 | crypt.crypt("test", crypt.METHOD_BLOWFISH) + | + +S324.py:24:26: S324 Probable use of insecure hash functions in `crypt`: `crypt.METHOD_BLOWFISH` + | +22 | crypt.crypt("test", salt=crypt.METHOD_CRYPT) +23 | crypt.crypt("test", salt=crypt.METHOD_MD5) +24 | crypt.crypt("test", salt=crypt.METHOD_BLOWFISH) + | ^^^^^^^^^^^^^^^^^^^^^ S324 +25 | crypt.crypt("test", crypt.METHOD_BLOWFISH) + | + +S324.py:25:21: S324 Probable use of insecure hash functions in `crypt`: `crypt.METHOD_BLOWFISH` + | +23 | crypt.crypt("test", salt=crypt.METHOD_MD5) +24 | crypt.crypt("test", salt=crypt.METHOD_BLOWFISH) +25 | crypt.crypt("test", crypt.METHOD_BLOWFISH) + | ^^^^^^^^^^^^^^^^^^^^^ S324 +26 | +27 | crypt.mksalt(crypt.METHOD_CRYPT) + | + +S324.py:27:14: S324 Probable use of insecure hash functions in `crypt`: `crypt.METHOD_CRYPT` + | +25 | crypt.crypt("test", crypt.METHOD_BLOWFISH) +26 | +27 | crypt.mksalt(crypt.METHOD_CRYPT) + | ^^^^^^^^^^^^^^^^^^ S324 +28 | crypt.mksalt(crypt.METHOD_MD5) +29 | crypt.mksalt(crypt.METHOD_BLOWFISH) + | + +S324.py:28:14: S324 Probable use of insecure hash functions in `crypt`: `crypt.METHOD_MD5` + | +27 | crypt.mksalt(crypt.METHOD_CRYPT) +28 | crypt.mksalt(crypt.METHOD_MD5) + | ^^^^^^^^^^^^^^^^ S324 +29 | crypt.mksalt(crypt.METHOD_BLOWFISH) + | + +S324.py:29:14: S324 Probable use of insecure hash functions in `crypt`: `crypt.METHOD_BLOWFISH` + | +27 | crypt.mksalt(crypt.METHOD_CRYPT) +28 | crypt.mksalt(crypt.METHOD_MD5) +29 | crypt.mksalt(crypt.METHOD_BLOWFISH) + | ^^^^^^^^^^^^^^^^^^^^^ S324 +30 | +31 | # OK |