Use matches! for insecure hash rule (#5141)

This commit is contained in:
Charlie Marsh 2023-06-16 00:18:32 -04:00 committed by GitHub
parent 13813dc1b1
commit fab2a4adf7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 39 additions and 32 deletions

View file

@ -1,8 +1,8 @@
use rustpython_parser::ast::{self, Constant, Expr, Keyword, Ranged}; use rustpython_parser::ast::{Expr, Keyword, Ranged};
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::SimpleCallArgs; use ruff_python_ast::helpers::{is_const_false, SimpleCallArgs};
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -21,26 +21,6 @@ impl Violation for HashlibInsecureHashFunction {
} }
} }
const WEAK_HASHES: [&str; 4] = ["md4", "md5", "sha", "sha1"];
fn is_used_for_security(call_args: &SimpleCallArgs) -> bool {
match call_args.keyword_argument("usedforsecurity") {
Some(expr) => !matches!(
expr,
Expr::Constant(ast::ExprConstant {
value: Constant::Bool(false),
..
})
),
_ => true,
}
}
enum HashlibCall {
New,
WeakHash(&'static str),
}
/// S324 /// S324
pub(crate) fn hashlib_insecure_hash_functions( pub(crate) fn hashlib_insecure_hash_functions(
checker: &mut Checker, checker: &mut Checker,
@ -51,15 +31,13 @@ pub(crate) fn hashlib_insecure_hash_functions(
if let Some(hashlib_call) = checker if let Some(hashlib_call) = checker
.semantic() .semantic()
.resolve_call_path(func) .resolve_call_path(func)
.and_then(|call_path| { .and_then(|call_path| match call_path.as_slice() {
if matches!(call_path.as_slice(), ["hashlib", "new"]) { ["hashlib", "new"] => Some(HashlibCall::New),
Some(HashlibCall::New) ["hashlib", "md4"] => Some(HashlibCall::WeakHash("md4")),
} else { ["hashlib", "md5"] => Some(HashlibCall::WeakHash("md5")),
WEAK_HASHES ["hashlib", "sha"] => Some(HashlibCall::WeakHash("sha")),
.iter() ["hashlib", "sha1"] => Some(HashlibCall::WeakHash("sha1")),
.find(|hash| call_path.as_slice() == ["hashlib", hash]) _ => None,
.map(|hash| HashlibCall::WeakHash(hash))
}
}) })
{ {
match hashlib_call { match hashlib_call {
@ -72,7 +50,12 @@ pub(crate) fn hashlib_insecure_hash_functions(
if let Some(name_arg) = call_args.argument("name", 0) { if let Some(name_arg) = call_args.argument("name", 0) {
if let Some(hash_func_name) = string_literal(name_arg) { if let Some(hash_func_name) = string_literal(name_arg) {
if WEAK_HASHES.contains(&hash_func_name.to_lowercase().as_str()) { // `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( checker.diagnostics.push(Diagnostic::new(
HashlibInsecureHashFunction { HashlibInsecureHashFunction {
string: hash_func_name.to_string(), string: hash_func_name.to_string(),
@ -100,3 +83,15 @@ pub(crate) fn hashlib_insecure_hash_functions(
} }
} }
} }
fn is_used_for_security(call_args: &SimpleCallArgs) -> bool {
match call_args.keyword_argument("usedforsecurity") {
Some(expr) => !is_const_false(expr),
_ => true,
}
}
enum HashlibCall {
New,
WeakHash(&'static str),
}

View file

@ -634,6 +634,18 @@ pub const fn is_const_true(expr: &Expr) -> bool {
) )
} }
/// Return `true` if an [`Expr`] is `False`.
pub const fn is_const_false(expr: &Expr) -> bool {
matches!(
expr,
Expr::Constant(ast::ExprConstant {
value: Constant::Bool(false),
kind: None,
..
}),
)
}
/// Return `true` if a keyword argument is present with a non-`None` value. /// Return `true` if a keyword argument is present with a non-`None` value.
pub fn has_non_none_keyword(keywords: &[Keyword], keyword: &str) -> bool { pub fn has_non_none_keyword(keywords: &[Keyword], keyword: &str) -> bool {
find_keyword(keywords, keyword).map_or(false, |keyword| { find_keyword(keywords, keyword).map_or(false, |keyword| {