Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from Jan 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -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)

Expand Down
52 changes: 52 additions & 0 deletions 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)
3 changes: 3 additions & 0 deletions ruff.schema.json
Expand Up @@ -927,6 +927,9 @@
"S106",
"S107",
"S108",
"S3",
"S32",
"S324",
"SIM",
"SIM1",
"SIM10",
Expand Down
2 changes: 1 addition & 1 deletion src/ast/helpers.rs
Expand Up @@ -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 {
Expand Down
11 changes: 11 additions & 0 deletions src/checkers/ast.rs
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/flake8_bandit/checks/bad_file_permissions.rs
Expand Up @@ -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> {
Expand Down
66 changes: 66 additions & 0 deletions 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
}
2 changes: 2 additions & 0 deletions src/flake8_bandit/checks/mod.rs
Expand Up @@ -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;
Expand All @@ -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;
1 change: 1 addition & 0 deletions src/flake8_bandit/mod.rs
Expand Up @@ -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(
Expand Down
@@ -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: ~

2 changes: 1 addition & 1 deletion src/flake8_pytest_style/plugins/fail.rs
Expand Up @@ -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));
Expand Down
6 changes: 3 additions & 3 deletions src/flake8_pytest_style/plugins/patch.rs
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down