Skip to content

Commit

Permalink
[flake8-bandit] Add Rule for S324 (Insecure hash functions in `ha…
Browse files Browse the repository at this point in the history
…shlib`) (#1661)

ref: #1646
  • Loading branch information
saadmk11 committed Jan 5, 2023
1 parent 1991d61 commit 9f8ef17
Show file tree
Hide file tree
Showing 13 changed files with 289 additions and 7 deletions.
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

0 comments on commit 9f8ef17

Please sign in to comment.