Fix eval detection for suspicious-eval-usage (#5506)

Closes https://github.com/astral-sh/ruff/issues/5505.
This commit is contained in:
Charlie Marsh 2023-07-04 14:01:29 -04:00 committed by GitHub
parent 0b963ddcfa
commit 521e6de2c8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 52 additions and 15 deletions

View File

@ -0,0 +1,12 @@
import os
print(eval("1+1")) # S307
print(eval("os.getcwd()")) # S307
class Class(object):
def eval(self):
print("hi")
def foo(self):
self.eval() # OK

View File

@ -2584,9 +2584,7 @@ where
flake8_pie::rules::unnecessary_dict_kwargs(self, expr, keywords); flake8_pie::rules::unnecessary_dict_kwargs(self, expr, keywords);
} }
if self.enabled(Rule::ExecBuiltin) { if self.enabled(Rule::ExecBuiltin) {
if let Some(diagnostic) = flake8_bandit::rules::exec_used(expr, func) { flake8_bandit::rules::exec_used(self, func);
self.diagnostics.push(diagnostic);
}
} }
if self.enabled(Rule::BadFilePermissions) { if self.enabled(Rule::BadFilePermissions) {
flake8_bandit::rules::bad_file_permissions(self, func, args, keywords); flake8_bandit::rules::bad_file_permissions(self, func, args, keywords);

View File

@ -39,6 +39,7 @@ mod tests {
#[test_case(Rule::SubprocessPopenWithShellEqualsTrue, Path::new("S602.py"))] #[test_case(Rule::SubprocessPopenWithShellEqualsTrue, Path::new("S602.py"))]
#[test_case(Rule::SubprocessWithoutShellEqualsTrue, Path::new("S603.py"))] #[test_case(Rule::SubprocessWithoutShellEqualsTrue, Path::new("S603.py"))]
#[test_case(Rule::SuspiciousPickleUsage, Path::new("S301.py"))] #[test_case(Rule::SuspiciousPickleUsage, Path::new("S301.py"))]
#[test_case(Rule::SuspiciousEvalUsage, Path::new("S307.py"))]
#[test_case(Rule::SuspiciousTelnetUsage, Path::new("S312.py"))] #[test_case(Rule::SuspiciousTelnetUsage, Path::new("S312.py"))]
#[test_case(Rule::TryExceptContinue, Path::new("S112.py"))] #[test_case(Rule::TryExceptContinue, Path::new("S112.py"))]
#[test_case(Rule::TryExceptPass, Path::new("S110.py"))] #[test_case(Rule::TryExceptPass, Path::new("S110.py"))]

View File

@ -1,8 +1,10 @@
use rustpython_parser::ast::{self, Expr, Ranged}; use rustpython_parser::ast::{Expr, 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 crate::checkers::ast::Checker;
#[violation] #[violation]
pub struct ExecBuiltin; pub struct ExecBuiltin;
@ -14,12 +16,16 @@ impl Violation for ExecBuiltin {
} }
/// S102 /// S102
pub(crate) fn exec_used(expr: &Expr, func: &Expr) -> Option<Diagnostic> { pub(crate) fn exec_used(checker: &mut Checker, func: &Expr) {
let Expr::Name(ast::ExprName { id, .. }) = func else { if checker
return None; .semantic()
}; .resolve_call_path(func)
if id != "exec" { .map_or(false, |call_path| {
return None; matches!(call_path.as_slice(), ["" | "builtin", "exec"])
})
{
checker
.diagnostics
.push(Diagnostic::new(ExecBuiltin, func.range()));
} }
Some(Diagnostic::new(ExecBuiltin, expr.range()))
} }

View File

@ -219,7 +219,7 @@ impl Violation for SuspiciousFTPLibUsage {
} }
} }
/// S001 /// S301, S302, S303, S304, S305, S306, S307, S308, S310, S311, S312, S313, S314, S315, S316, S317, S318, S319, S320, S321, S323
pub(crate) fn suspicious_function_call(checker: &mut Checker, expr: &Expr) { pub(crate) fn suspicious_function_call(checker: &mut Checker, expr: &Expr) {
let Expr::Call(ast::ExprCall { func, .. }) = expr else { let Expr::Call(ast::ExprCall { func, .. }) = expr else {
return; return;
@ -246,7 +246,7 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, expr: &Expr) {
// Mktemp // Mktemp
["tempfile", "mktemp"] => Some(SuspiciousMktempUsage.into()), ["tempfile", "mktemp"] => Some(SuspiciousMktempUsage.into()),
// Eval // Eval
["eval"] => Some(SuspiciousEvalUsage.into()), ["" | "builtins", "eval"] => Some(SuspiciousEvalUsage.into()),
// MarkSafe // MarkSafe
["django", "utils", "safestring", "mark_safe"] => Some(SuspiciousMarkSafeUsage.into()), ["django", "utils", "safestring", "mark_safe"] => Some(SuspiciousMarkSafeUsage.into()),
// URLOpen // URLOpen

View File

@ -6,7 +6,7 @@ S102.py:3:5: S102 Use of `exec` detected
1 | def fn(): 1 | def fn():
2 | # Error 2 | # Error
3 | exec('x = 2') 3 | exec('x = 2')
| ^^^^^^^^^^^^^ S102 | ^^^^ S102
4 | 4 |
5 | exec('y = 3') 5 | exec('y = 3')
| |
@ -16,7 +16,7 @@ S102.py:5:1: S102 Use of `exec` detected
3 | exec('x = 2') 3 | exec('x = 2')
4 | 4 |
5 | exec('y = 3') 5 | exec('y = 3')
| ^^^^^^^^^^^^^ S102 | ^^^^ S102
| |

View File

@ -0,0 +1,20 @@
---
source: crates/ruff/src/rules/flake8_bandit/mod.rs
---
S307.py:3:7: S307 Use of possibly insecure function; consider using `ast.literal_eval`
|
1 | import os
2 |
3 | print(eval("1+1")) # S307
| ^^^^^^^^^^^ S307
4 | print(eval("os.getcwd()")) # S307
|
S307.py:4:7: S307 Use of possibly insecure function; consider using `ast.literal_eval`
|
3 | print(eval("1+1")) # S307
4 | print(eval("os.getcwd()")) # S307
| ^^^^^^^^^^^^^^^^^^^ S307
|