Skip to content

Commit

Permalink
feat(rules): flag weak hashes in crypt for S324
Browse files Browse the repository at this point in the history
  • Loading branch information
mkniewallner committed Mar 9, 2024
1 parent 798e9a7 commit 4b48d08
Show file tree
Hide file tree
Showing 3 changed files with 223 additions and 88 deletions.
18 changes: 18 additions & 0 deletions 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
Expand All @@ -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')
Expand All @@ -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)
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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),
}
}
}

Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 4b48d08

Please sign in to comment.