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 144e32c commit 08738a0
Show file tree
Hide file tree
Showing 3 changed files with 255 additions and 131 deletions.
39 changes: 18 additions & 21 deletions crates/ruff_linter/resources/test/fixtures/flake8_bandit/S324.py
@@ -1,52 +1,49 @@
import crypt
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
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)

hashlib.new('sha256')
crypt.mksalt(crypt.METHOD_CRYPT)
crypt.mksalt(crypt.METHOD_MD5)
crypt.mksalt(crypt.METHOD_BLOWFISH)

# 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)

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,66 +44,124 @@ 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)
#[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}`")
}
}

/// 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 {
method: HashlibMethod::New,
}),
["hashlib", "md4"] => Some(WeakHashCall::Hashlib {
method: HashlibMethod::WeakHash("md4"),
}),
["hashlib", "md5"] => Some(WeakHashCall::Hashlib {
method: HashlibMethod::WeakHash("md5"),
}),
["hashlib", "sha"] => Some(WeakHashCall::Hashlib {
method: HashlibMethod::WeakHash("sha"),
}),
["hashlib", "sha1"] => Some(WeakHashCall::Hashlib {
method: HashlibMethod::WeakHash("sha1"),
}),
["crypt", "crypt" | "mksalt"] => Some(WeakHashCall::Crypt),
_ => None,
})
{
if !is_used_for_security(&call.arguments) {
return;
match weak_hash_call {
WeakHashCall::Hashlib { method } => {
detect_insecure_hashlib_calls(checker, call, method)
}
WeakHashCall::Crypt => detect_insecure_crypt_calls(checker, call),
}
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(),
));
}
}
}

fn detect_insecure_hashlib_calls(
checker: &mut Checker,
call: &ast::ExprCall,
method: HashlibMethod,
) {
if !is_used_for_security(&call.arguments) {
return;
}

match method {
HashlibMethod::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(),
));
}
HashlibMethod::WeakHash(func_name) => {
checker.diagnostics.push(Diagnostic::new(
HashlibInsecureHashFunction {
library: "hashlib".to_string(),
string: (*func_name).to_string(),
},
call.func.range(),
));
}
}
}

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) {
match 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(),
))
}
_ => (),
}
}
}
}
Expand All @@ -115,7 +174,13 @@ fn is_used_for_security(arguments: &Arguments) -> bool {
}

#[derive(Debug)]
enum HashlibCall {
enum WeakHashCall {
Hashlib { method: HashlibMethod },
Crypt,
}

#[derive(Debug)]
enum HashlibMethod {
New,
WeakHash(&'static str),
}

0 comments on commit 08738a0

Please sign in to comment.