From 5dfc2d1e784741f99fb1d5fe4e90c9c233cf0c3f Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Fri, 8 Mar 2024 22:38:45 +0100 Subject: [PATCH 1/7] feat(rules): cover more `S605` cases Ref: https://github.com/PyCQA/bandit/pull/1116 --- .../test/fixtures/flake8_bandit/S605.py | 3 + .../flake8_bandit/rules/shell_injection.rs | 1 + ...s__flake8_bandit__tests__S605_S605.py.snap | 192 ++++++++++-------- 3 files changed, 109 insertions(+), 87 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S605.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S605.py index de9499ec54dc2..548b20a13a61e 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S605.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S605.py @@ -1,4 +1,5 @@ import os +import subprocess import commands import popen2 @@ -16,6 +17,8 @@ popen2.Popen4("true") commands.getoutput("true") commands.getstatusoutput("true") +subprocess.getoutput("true") +subprocess.getstatusoutput("true") # Check command argument looks unsafe. diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/shell_injection.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/shell_injection.rs index 952c8b11f418e..4d0df15f387e4 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/shell_injection.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/shell_injection.rs @@ -433,6 +433,7 @@ fn get_call_kind(func: &Expr, semantic: &SemanticModel) -> Option { "Popen" | "call" | "check_call" | "check_output" | "run" => { Some(CallKind::Subprocess) } + "getoutput" | "getstatusoutput" => Some(CallKind::Shell), _ => None, }, "popen2" => match submodule { diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S605_S605.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S605_S605.py.snap index 49b3823a03fe3..6ea0e7c7fde70 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S605_S605.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S605_S605.py.snap @@ -1,147 +1,165 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S605.py:7:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` - | -6 | # Check all shell functions. -7 | os.system("true") - | ^^^^^^ S605 -8 | os.popen("true") -9 | os.popen2("true") - | - -S605.py:8:10: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` - | - 6 | # Check all shell functions. - 7 | os.system("true") - 8 | os.popen("true") - | ^^^^^^ S605 - 9 | os.popen2("true") -10 | os.popen3("true") +S605.py:8:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | + 7 | # Check all shell functions. + 8 | os.system("true") + | ^^^^^^ S605 + 9 | os.popen("true") +10 | os.popen2("true") | -S605.py:9:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` +S605.py:9:10: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` | - 7 | os.system("true") - 8 | os.popen("true") - 9 | os.popen2("true") - | ^^^^^^ S605 -10 | os.popen3("true") -11 | os.popen4("true") + 7 | # Check all shell functions. + 8 | os.system("true") + 9 | os.popen("true") + | ^^^^^^ S605 +10 | os.popen2("true") +11 | os.popen3("true") | S605.py:10:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` | - 8 | os.popen("true") - 9 | os.popen2("true") -10 | os.popen3("true") + 8 | os.system("true") + 9 | os.popen("true") +10 | os.popen2("true") | ^^^^^^ S605 -11 | os.popen4("true") -12 | popen2.popen2("true") +11 | os.popen3("true") +12 | os.popen4("true") | S605.py:11:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` | - 9 | os.popen2("true") -10 | os.popen3("true") -11 | os.popen4("true") + 9 | os.popen("true") +10 | os.popen2("true") +11 | os.popen3("true") | ^^^^^^ S605 -12 | popen2.popen2("true") -13 | popen2.popen3("true") +12 | os.popen4("true") +13 | popen2.popen2("true") | -S605.py:12:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` +S605.py:12:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` | -10 | os.popen3("true") -11 | os.popen4("true") -12 | popen2.popen2("true") - | ^^^^^^ S605 -13 | popen2.popen3("true") -14 | popen2.popen4("true") +10 | os.popen2("true") +11 | os.popen3("true") +12 | os.popen4("true") + | ^^^^^^ S605 +13 | popen2.popen2("true") +14 | popen2.popen3("true") | S605.py:13:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` | -11 | os.popen4("true") -12 | popen2.popen2("true") -13 | popen2.popen3("true") +11 | os.popen3("true") +12 | os.popen4("true") +13 | popen2.popen2("true") | ^^^^^^ S605 -14 | popen2.popen4("true") -15 | popen2.Popen3("true") +14 | popen2.popen3("true") +15 | popen2.popen4("true") | S605.py:14:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` | -12 | popen2.popen2("true") -13 | popen2.popen3("true") -14 | popen2.popen4("true") +12 | os.popen4("true") +13 | popen2.popen2("true") +14 | popen2.popen3("true") | ^^^^^^ S605 -15 | popen2.Popen3("true") -16 | popen2.Popen4("true") +15 | popen2.popen4("true") +16 | popen2.Popen3("true") | S605.py:15:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` | -13 | popen2.popen3("true") -14 | popen2.popen4("true") -15 | popen2.Popen3("true") +13 | popen2.popen2("true") +14 | popen2.popen3("true") +15 | popen2.popen4("true") | ^^^^^^ S605 -16 | popen2.Popen4("true") -17 | commands.getoutput("true") +16 | popen2.Popen3("true") +17 | popen2.Popen4("true") | S605.py:16:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` | -14 | popen2.popen4("true") -15 | popen2.Popen3("true") -16 | popen2.Popen4("true") +14 | popen2.popen3("true") +15 | popen2.popen4("true") +16 | popen2.Popen3("true") | ^^^^^^ S605 -17 | commands.getoutput("true") -18 | commands.getstatusoutput("true") +17 | popen2.Popen4("true") +18 | commands.getoutput("true") | -S605.py:17:20: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` +S605.py:17:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` | -15 | popen2.Popen3("true") -16 | popen2.Popen4("true") -17 | commands.getoutput("true") +15 | popen2.popen4("true") +16 | popen2.Popen3("true") +17 | popen2.Popen4("true") + | ^^^^^^ S605 +18 | commands.getoutput("true") +19 | commands.getstatusoutput("true") + | + +S605.py:18:20: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +16 | popen2.Popen3("true") +17 | popen2.Popen4("true") +18 | commands.getoutput("true") | ^^^^^^ S605 -18 | commands.getstatusoutput("true") +19 | commands.getstatusoutput("true") +20 | subprocess.getoutput("true") | -S605.py:18:26: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` +S605.py:19:26: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` | -16 | popen2.Popen4("true") -17 | commands.getoutput("true") -18 | commands.getstatusoutput("true") +17 | popen2.Popen4("true") +18 | commands.getoutput("true") +19 | commands.getstatusoutput("true") | ^^^^^^ S605 +20 | subprocess.getoutput("true") +21 | subprocess.getstatusoutput("true") | -S605.py:23:11: S605 Starting a process with a shell, possible injection detected +S605.py:20:22: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +18 | commands.getoutput("true") +19 | commands.getstatusoutput("true") +20 | subprocess.getoutput("true") + | ^^^^^^ S605 +21 | subprocess.getstatusoutput("true") + | + +S605.py:21:28: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell` + | +19 | commands.getstatusoutput("true") +20 | subprocess.getoutput("true") +21 | subprocess.getstatusoutput("true") + | ^^^^^^ S605 | -21 | # Check command argument looks unsafe. -22 | var_string = "true" -23 | os.system(var_string) + +S605.py:26:11: S605 Starting a process with a shell, possible injection detected + | +24 | # Check command argument looks unsafe. +25 | var_string = "true" +26 | os.system(var_string) | ^^^^^^^^^^ S605 -24 | os.system([var_string]) -25 | os.system([var_string, ""]) +27 | os.system([var_string]) +28 | os.system([var_string, ""]) | -S605.py:24:11: S605 Starting a process with a shell, possible injection detected +S605.py:27:11: S605 Starting a process with a shell, possible injection detected | -22 | var_string = "true" -23 | os.system(var_string) -24 | os.system([var_string]) +25 | var_string = "true" +26 | os.system(var_string) +27 | os.system([var_string]) | ^^^^^^^^^^^^ S605 -25 | os.system([var_string, ""]) +28 | os.system([var_string, ""]) | -S605.py:25:11: S605 Starting a process with a shell, possible injection detected +S605.py:28:11: S605 Starting a process with a shell, possible injection detected | -23 | os.system(var_string) -24 | os.system([var_string]) -25 | os.system([var_string, ""]) +26 | os.system(var_string) +27 | os.system([var_string]) +28 | os.system([var_string, ""]) | ^^^^^^^^^^^^^^^^ S605 | - - From f08fa2648024b75f83919a0f7d810385d6ca1712 Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Fri, 8 Mar 2024 23:36:41 +0100 Subject: [PATCH 2/7] test(rules): add missing tests for `S311` --- .../test/fixtures/flake8_bandit/S311.py | 21 ++++++ .../src/rules/flake8_bandit/mod.rs | 1 + ...s__flake8_bandit__tests__S311_S311.py.snap | 71 +++++++++++++++++++ 3 files changed, 93 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_bandit/S311.py create mode 100644 crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S311_S311.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S311.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S311.py new file mode 100644 index 0000000000000..043158fbaebd9 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S311.py @@ -0,0 +1,21 @@ +import os +import random + +import a_lib + +# OK +random.SystemRandom() + +# Errors +random.Random() +random.random() +random.randrange() +random.randint() +random.choice() +random.choices() +random.uniform() +random.triangular() + +# Unrelated +os.urandom() +a_lib.random() diff --git a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs index ec2a462aafbbd..cb4b1a670be52 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs @@ -48,6 +48,7 @@ mod tests { #[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"))] #[test_case(Rule::SuspiciousTelnetlibImport, Path::new("S401.py"))] #[test_case(Rule::SuspiciousFtplibImport, Path::new("S402.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S311_S311.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S311_S311.py.snap new file mode 100644 index 0000000000000..81e321f53f5cf --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S311_S311.py.snap @@ -0,0 +1,71 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs +--- +S311.py:11:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes + | + 9 | # Errors +10 | random.Random() +11 | random.random() + | ^^^^^^^^^^^^^^^ S311 +12 | random.randrange() +13 | random.randint() + | + +S311.py:12:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes + | +10 | random.Random() +11 | random.random() +12 | random.randrange() + | ^^^^^^^^^^^^^^^^^^ S311 +13 | random.randint() +14 | random.choice() + | + +S311.py:13:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes + | +11 | random.random() +12 | random.randrange() +13 | random.randint() + | ^^^^^^^^^^^^^^^^ S311 +14 | random.choice() +15 | random.choices() + | + +S311.py:14:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes + | +12 | random.randrange() +13 | random.randint() +14 | random.choice() + | ^^^^^^^^^^^^^^^ S311 +15 | random.choices() +16 | random.uniform() + | + +S311.py:15:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes + | +13 | random.randint() +14 | random.choice() +15 | random.choices() + | ^^^^^^^^^^^^^^^^ S311 +16 | random.uniform() +17 | random.triangular() + | + +S311.py:16:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes + | +14 | random.choice() +15 | random.choices() +16 | random.uniform() + | ^^^^^^^^^^^^^^^^ S311 +17 | random.triangular() + | + +S311.py:17:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes + | +15 | random.choices() +16 | random.uniform() +17 | random.triangular() + | ^^^^^^^^^^^^^^^^^^^ S311 +18 | +19 | # Unrelated + | From d0634f542a6651f87f344c63344c52214df0dd44 Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Fri, 8 Mar 2024 23:42:17 +0100 Subject: [PATCH 3/7] feat(rules): flag `Random` and `randbytes` in `S311` Ref: https://github.com/PyCQA/bandit/pull/940 and https://github.com/PyCQA/bandit/pull/1096 --- .../test/fixtures/flake8_bandit/S311.py | 1 + .../rules/suspicious_function_call.rs | 2 +- ...s__flake8_bandit__tests__S311_S311.py.snap | 23 +++++++++++++++++-- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S311.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S311.py index 043158fbaebd9..5c0c0e6b57d0b 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S311.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S311.py @@ -15,6 +15,7 @@ random.choices() random.uniform() random.triangular() +random.randbytes() # Unrelated os.urandom() diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs index bcd7d305fe87d..4b90142a19536 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs @@ -867,7 +867,7 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) { ["urllib", "request", "URLopener" | "FancyURLopener"] | ["six", "moves", "urllib", "request", "URLopener" | "FancyURLopener"] => Some(SuspiciousURLOpenUsage.into()), // NonCryptographicRandom - ["random", "random" | "randrange" | "randint" | "choice" | "choices" | "uniform" | "triangular"] => Some(SuspiciousNonCryptographicRandomUsage.into()), + ["random", "Random" | "random" | "randrange" | "randint" | "choice" | "choices" | "uniform" | "triangular" | "randbytes"] => Some(SuspiciousNonCryptographicRandomUsage.into()), // UnverifiedContext ["ssl", "_create_unverified_context"] => Some(SuspiciousUnverifiedContextUsage.into()), // XMLCElementTree diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S311_S311.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S311_S311.py.snap index 81e321f53f5cf..3c395057aaf74 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S311_S311.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S311_S311.py.snap @@ -1,6 +1,15 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- +S311.py:10:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes + | + 9 | # Errors +10 | random.Random() + | ^^^^^^^^^^^^^^^ S311 +11 | random.random() +12 | random.randrange() + | + S311.py:11:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes | 9 | # Errors @@ -58,6 +67,7 @@ S311.py:16:1: S311 Standard pseudo-random generators are not suitable for crypto 16 | random.uniform() | ^^^^^^^^^^^^^^^^ S311 17 | random.triangular() +18 | random.randbytes() | S311.py:17:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes @@ -66,6 +76,15 @@ S311.py:17:1: S311 Standard pseudo-random generators are not suitable for crypto 16 | random.uniform() 17 | random.triangular() | ^^^^^^^^^^^^^^^^^^^ S311 -18 | -19 | # Unrelated +18 | random.randbytes() + | + +S311.py:18:1: S311 Standard pseudo-random generators are not suitable for cryptographic purposes + | +16 | random.uniform() +17 | random.triangular() +18 | random.randbytes() + | ^^^^^^^^^^^^^^^^^^ S311 +19 | +20 | # Unrelated | From c7d412d14c4f4d689ec38a13f21fd265d5d4d35f Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Sat, 9 Mar 2024 12:25:44 +0100 Subject: [PATCH 4/7] test(rules): cleanup `S324` tests Remove unnecessary empty lines, and use `OK`/`Errors` to be more consistent with other tests. --- .../test/fixtures/flake8_bandit/S324.py | 27 +-- ...s__flake8_bandit__tests__S324_S324.py.snap | 162 +++++++++--------- 2 files changed, 82 insertions(+), 107 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 bea55067ea77e..8cb3f70d1f301 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S324.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S324.py @@ -2,51 +2,28 @@ from hashlib import new as hashlib_new from hashlib import sha1 as hashlib_sha1 -# Invalid - +# Errors 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 - +# OK 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/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 8cd080b375c84..4e8fd2450007d 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,133 +1,131 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S324.py:7:13: S324 Probable use of insecure hash functions in `hashlib`: `md5` +S324.py:6:13: S324 Probable use of insecure hash functions in `hashlib`: `md5` | -5 | # Invalid -6 | -7 | hashlib.new('md5') +5 | # Errors +6 | hashlib.new('md5') | ^^^^^ S324 -8 | -9 | hashlib.new('md4', b'test') +7 | hashlib.new('md4', b'test') +8 | hashlib.new(name='md5', data=b'test') | -S324.py:9:13: S324 Probable use of insecure hash functions in `hashlib`: `md4` - | - 7 | hashlib.new('md5') - 8 | - 9 | hashlib.new('md4', b'test') - | ^^^^^ S324 -10 | -11 | 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:11:18: S324 Probable use of insecure hash functions in `hashlib`: `md5` +S324.py:8:18: S324 Probable use of insecure hash functions in `hashlib`: `md5` | - 9 | hashlib.new('md4', b'test') -10 | -11 | hashlib.new(name='md5', data=b'test') + 6 | hashlib.new('md5') + 7 | hashlib.new('md4', b'test') + 8 | hashlib.new(name='md5', data=b'test') | ^^^^^ S324 -12 | -13 | hashlib.new('MD4', data=b'test') + 9 | hashlib.new('MD4', data=b'test') +10 | hashlib.new('sha1') | -S324.py:13:13: S324 Probable use of insecure hash functions in `hashlib`: `MD4` +S324.py:9:13: S324 Probable use of insecure hash functions in `hashlib`: `MD4` | -11 | hashlib.new(name='md5', data=b'test') -12 | -13 | hashlib.new('MD4', data=b'test') + 7 | hashlib.new('md4', b'test') + 8 | hashlib.new(name='md5', data=b'test') + 9 | hashlib.new('MD4', data=b'test') | ^^^^^ S324 -14 | -15 | hashlib.new('sha1') +10 | hashlib.new('sha1') +11 | hashlib.new('sha1', data=b'test') | -S324.py:15:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` +S324.py:10:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` | -13 | hashlib.new('MD4', data=b'test') -14 | -15 | hashlib.new('sha1') + 8 | hashlib.new(name='md5', data=b'test') + 9 | hashlib.new('MD4', data=b'test') +10 | hashlib.new('sha1') | ^^^^^^ S324 -16 | -17 | hashlib.new('sha1', data=b'test') +11 | hashlib.new('sha1', data=b'test') +12 | hashlib.new('sha', data=b'test') | -S324.py:17: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` | -15 | hashlib.new('sha1') -16 | -17 | hashlib.new('sha1', data=b'test') + 9 | hashlib.new('MD4', data=b'test') +10 | hashlib.new('sha1') +11 | hashlib.new('sha1', data=b'test') | ^^^^^^ S324 -18 | -19 | hashlib.new('sha', data=b'test') +12 | hashlib.new('sha', data=b'test') +13 | hashlib.new(name='SHA', data=b'test') | -S324.py:19:13: S324 Probable use of insecure hash functions in `hashlib`: `sha` +S324.py:12:13: S324 Probable use of insecure hash functions in `hashlib`: `sha` | -17 | hashlib.new('sha1', data=b'test') -18 | -19 | hashlib.new('sha', data=b'test') +10 | hashlib.new('sha1') +11 | hashlib.new('sha1', data=b'test') +12 | hashlib.new('sha', data=b'test') | ^^^^^ S324 -20 | -21 | hashlib.new(name='SHA', data=b'test') +13 | hashlib.new(name='SHA', data=b'test') +14 | hashlib.sha(data=b'test') | -S324.py:21:18: S324 Probable use of insecure hash functions in `hashlib`: `SHA` +S324.py:13:18: S324 Probable use of insecure hash functions in `hashlib`: `SHA` | -19 | hashlib.new('sha', data=b'test') -20 | -21 | hashlib.new(name='SHA', data=b'test') +11 | hashlib.new('sha1', data=b'test') +12 | hashlib.new('sha', data=b'test') +13 | hashlib.new(name='SHA', data=b'test') | ^^^^^ S324 -22 | -23 | hashlib.sha(data=b'test') +14 | hashlib.sha(data=b'test') +15 | hashlib.md5() | -S324.py:23:1: S324 Probable use of insecure hash functions in `hashlib`: `sha` +S324.py:14:1: S324 Probable use of insecure hash functions in `hashlib`: `sha` | -21 | hashlib.new(name='SHA', data=b'test') -22 | -23 | hashlib.sha(data=b'test') +12 | hashlib.new('sha', data=b'test') +13 | hashlib.new(name='SHA', data=b'test') +14 | hashlib.sha(data=b'test') | ^^^^^^^^^^^ S324 -24 | -25 | hashlib.md5() +15 | hashlib.md5() +16 | hashlib_new('sha1') | -S324.py:25:1: S324 Probable use of insecure hash functions in `hashlib`: `md5` +S324.py:15:1: S324 Probable use of insecure hash functions in `hashlib`: `md5` | -23 | hashlib.sha(data=b'test') -24 | -25 | hashlib.md5() +13 | hashlib.new(name='SHA', data=b'test') +14 | hashlib.sha(data=b'test') +15 | hashlib.md5() | ^^^^^^^^^^^ S324 -26 | -27 | hashlib_new('sha1') +16 | hashlib_new('sha1') +17 | hashlib_sha1('sha1') | -S324.py:27:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` +S324.py:16:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` | -25 | hashlib.md5() -26 | -27 | hashlib_new('sha1') +14 | hashlib.sha(data=b'test') +15 | hashlib.md5() +16 | hashlib_new('sha1') | ^^^^^^ S324 -28 | -29 | hashlib_sha1('sha1') +17 | hashlib_sha1('sha1') +18 | # usedforsecurity arg only available in Python 3.9+ | -S324.py:29:1: S324 Probable use of insecure hash functions in `hashlib`: `sha1` +S324.py:17:1: S324 Probable use of insecure hash functions in `hashlib`: `sha1` | -27 | hashlib_new('sha1') -28 | -29 | hashlib_sha1('sha1') +15 | hashlib.md5() +16 | hashlib_new('sha1') +17 | hashlib_sha1('sha1') | ^^^^^^^^^^^^ S324 -30 | -31 | # usedforsecurity arg only available in Python 3.9+ +18 | # usedforsecurity arg only available in Python 3.9+ +19 | hashlib.new('sha1', usedforsecurity=True) | -S324.py:32:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` +S324.py:19:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1` | -31 | # usedforsecurity arg only available in Python 3.9+ -32 | hashlib.new('sha1', usedforsecurity=True) +17 | hashlib_sha1('sha1') +18 | # usedforsecurity arg only available in Python 3.9+ +19 | hashlib.new('sha1', usedforsecurity=True) | ^^^^^^ S324 -33 | -34 | # Valid +20 | +21 | # OK | - - From 798e9a7469bbdf806fbbfd1253afab962d797970 Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Sat, 9 Mar 2024 12:39:35 +0100 Subject: [PATCH 5/7] refactor(rules): prepare for flagging `crypt` in `S324` --- .../rules/hashlib_insecure_hash_functions.rs | 72 +++++++++++-------- 1 file changed, 42 insertions(+), 30 deletions(-) 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 38520e804bd74..d38832f6034a9 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 @@ -48,14 +48,15 @@ use super::super::helpers::string_literal; /// - [Common Weakness Enumeration: CWE-916](https://cwe.mitre.org/data/definitions/916.html) #[violation] pub struct HashlibInsecureHashFunction { + library: String, string: String, } impl Violation for HashlibInsecureHashFunction { #[derive_message_formats] fn message(&self) -> String { - let HashlibInsecureHashFunction { string } = self; - format!("Probable use of insecure hash functions in `hashlib`: `{string}`") + let HashlibInsecureHashFunction { library, string } = self; + format!("Probable use of insecure hash functions in `{library}`: `{string}`") } } @@ -73,37 +74,48 @@ pub(crate) fn hashlib_insecure_hash_functions(checker: &mut Checker, call: &ast: _ => None, }) { - if !is_used_for_security(&call.arguments) { - return; - } - match hashlib_call { - HashlibCall::New => { - if let Some(name_arg) = call.arguments.find_argument("name", 0) { - if let Some(hash_func_name) = string_literal(name_arg) { - // `hashlib.new` accepts both lowercase and uppercase names for hash - // functions. - if matches!( - hash_func_name, - "md4" | "md5" | "sha" | "sha1" | "MD4" | "MD5" | "SHA" | "SHA1" - ) { - checker.diagnostics.push(Diagnostic::new( - HashlibInsecureHashFunction { - string: hash_func_name.to_string(), - }, - name_arg.range(), - )); - } + detect_insecure_hashlib_calls(checker, call, &hashlib_call); + } +} + +fn detect_insecure_hashlib_calls( + checker: &mut Checker, + call: &ast::ExprCall, + hashlib_call: &HashlibCall, +) { + if !is_used_for_security(&call.arguments) { + return; + } + + match hashlib_call { + HashlibCall::New => { + if let Some(name_arg) = call.arguments.find_argument("name", 0) { + if let Some(hash_func_name) = string_literal(name_arg) { + // `hashlib.new` accepts both lowercase and uppercase names for hash + // functions. + if matches!( + hash_func_name, + "md4" | "md5" | "sha" | "sha1" | "MD4" | "MD5" | "SHA" | "SHA1" + ) { + checker.diagnostics.push(Diagnostic::new( + HashlibInsecureHashFunction { + library: "hashlib".to_string(), + string: hash_func_name.to_string(), + }, + name_arg.range(), + )); } } } - HashlibCall::WeakHash(func_name) => { - checker.diagnostics.push(Diagnostic::new( - HashlibInsecureHashFunction { - string: (*func_name).to_string(), - }, - call.func.range(), - )); - } + } + HashlibCall::WeakHash(func_name) => { + checker.diagnostics.push(Diagnostic::new( + HashlibInsecureHashFunction { + library: "hashlib".to_string(), + string: (*func_name).to_string(), + }, + call.func.range(), + )); } } } From eeb7a041e2d559686c48713764bb0c93ae969187 Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Sat, 9 Mar 2024 12:53:45 +0100 Subject: [PATCH 6/7] 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 | 110 +++++++-- ...s__flake8_bandit__tests__S324_S324.py.snap | 226 +++++++++++------- 3 files changed, 249 insertions(+), 105 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 8cb3f70d1f301..c3cb83e4d386d 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 d38832f6034a9..f28b0668c14a6 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), + } } } @@ -89,23 +107,26 @@ fn detect_insecure_hashlib_calls( match hashlib_call { HashlibCall::New => { - if let Some(name_arg) = call.arguments.find_argument("name", 0) { - if let Some(hash_func_name) = string_literal(name_arg) { - // `hashlib.new` accepts both lowercase and uppercase names for hash - // functions. - if matches!( - hash_func_name, - "md4" | "md5" | "sha" | "sha1" | "MD4" | "MD5" | "SHA" | "SHA1" - ) { - checker.diagnostics.push(Diagnostic::new( - HashlibInsecureHashFunction { - library: "hashlib".to_string(), - string: hash_func_name.to_string(), - }, - name_arg.range(), - )); - } - } + let Some(name_arg) = call.arguments.find_argument("name", 0) else { + return; + }; + let Some(hash_func_name) = string_literal(name_arg) else { + return; + }; + + // `hashlib.new` accepts both lowercase and uppercase names for hash + // functions. + if matches!( + hash_func_name, + "md4" | "md5" | "sha" | "sha1" | "MD4" | "MD5" | "SHA" | "SHA1" + ) { + checker.diagnostics.push(Diagnostic::new( + HashlibInsecureHashFunction { + library: "hashlib".to_string(), + string: hash_func_name.to_string(), + }, + name_arg.range(), + )); } } HashlibCall::WeakHash(func_name) => { @@ -120,12 +141,51 @@ fn detect_insecure_hashlib_calls( } } +fn detect_insecure_crypt_calls(checker: &mut Checker, call: &ast::ExprCall) { + let Some(method) = 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, + }) + .and_then(|(argument_name, position)| { + call.arguments.find_argument(argument_name, position) + }) + else { + return; + }; + + let Some(qualified_name) = checker.semantic().resolve_qualified_name(method) else { + return; + }; + + if matches!( + qualified_name.segments(), + ["crypt", "METHOD_CRYPT" | "METHOD_MD5" | "METHOD_BLOWFISH"] + ) { + 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 4e8fd2450007d..1a521d8ce44aa 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 | From 1b2edd43eb705db921cd52b612c992017048a2ba Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 11 Mar 2024 16:55:06 -0400 Subject: [PATCH 7/7] Make enums copy --- .../flake8_bandit/rules/hashlib_insecure_hash_functions.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 f28b0668c14a6..e19d6eb848db1 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 @@ -89,7 +89,7 @@ pub(crate) fn hashlib_insecure_hash_functions(checker: &mut Checker, call: &ast: { match weak_hash_call { WeakHashCall::Hashlib { call: hashlib_call } => { - detect_insecure_hashlib_calls(checker, call, &hashlib_call); + detect_insecure_hashlib_calls(checker, call, hashlib_call); } WeakHashCall::Crypt => detect_insecure_crypt_calls(checker, call), } @@ -99,7 +99,7 @@ pub(crate) fn hashlib_insecure_hash_functions(checker: &mut Checker, call: &ast: fn detect_insecure_hashlib_calls( checker: &mut Checker, call: &ast::ExprCall, - hashlib_call: &HashlibCall, + hashlib_call: HashlibCall, ) { if !is_used_for_security(&call.arguments) { return; @@ -181,12 +181,13 @@ fn is_used_for_security(arguments: &Arguments) -> bool { .map_or(true, |keyword| !is_const_false(&keyword.value)) } +#[derive(Debug, Copy, Clone)] enum WeakHashCall { Hashlib { call: HashlibCall }, Crypt, } -#[derive(Debug)] +#[derive(Debug, Copy, Clone)] enum HashlibCall { New, WeakHash(&'static str),