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 5ccb3d5
Show file tree
Hide file tree
Showing 3 changed files with 205 additions and 100 deletions.
39 changes: 18 additions & 21 deletions crates/ruff_linter/resources/test/fixtures/flake8_bandit/S324.py
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_false;
use ruff_python_ast::{self as ast, Arguments};
use ruff_text_size::Ranged;
use std::fmt::{Display, Formatter};

use crate::checkers::ast::Checker;

Expand Down Expand Up @@ -48,14 +49,15 @@ use super::super::helpers::string_literal;
/// - [Common Weakness Enumeration: CWE-916](https://cwe.mitre.org/data/definitions/916.html)
#[violation]
pub struct HashlibInsecureHashFunction {
string: String,
method: String,
library: HashLibrary,
}

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 { method, library } = self;
format!("Probable use of insecure hash functions in `{library}`: `{method}`")
}
}

Expand Down Expand Up @@ -88,7 +90,8 @@ pub(crate) fn hashlib_insecure_hash_functions(checker: &mut Checker, call: &ast:
) {
checker.diagnostics.push(Diagnostic::new(
HashlibInsecureHashFunction {
string: hash_func_name.to_string(),
method: hash_func_name.to_string(),
library: HashLibrary::Hashlib,
},
name_arg.range(),
));
Expand All @@ -99,13 +102,41 @@ pub(crate) fn hashlib_insecure_hash_functions(checker: &mut Checker, call: &ast:
HashlibCall::WeakHash(func_name) => {
checker.diagnostics.push(Diagnostic::new(
HashlibInsecureHashFunction {
string: (*func_name).to_string(),
method: (*func_name).to_string(),
library: HashLibrary::Hashlib,
},
call.func.range(),
));
}
}
}

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 {
method: qualified_name.to_string(),
library: HashLibrary::Crypt,
},
method.range(),
))
}
_ => (),
}
}
}
}
}

fn is_used_for_security(arguments: &Arguments) -> bool {
Expand All @@ -119,3 +150,18 @@ enum HashlibCall {
New,
WeakHash(&'static str),
}

#[derive(Debug, Eq, PartialEq)]
enum HashLibrary {
Crypt,
Hashlib,
}

impl Display for HashLibrary {
fn fmt(&self, fmt: &mut Formatter) -> std::fmt::Result {
match self {
HashLibrary::Crypt => fmt.write_str("crypt"),
HashLibrary::Hashlib => fmt.write_str("hashlib"),
}
}
}

0 comments on commit 5ccb3d5

Please sign in to comment.