diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S611.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S611.py new file mode 100644 index 0000000000000..ee4230273582f --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S611.py @@ -0,0 +1,13 @@ +from django.db.models.expressions import RawSQL +from django.contrib.auth.models import User + +User.objects.annotate(val=RawSQL('secure', [])) +User.objects.annotate(val=RawSQL('%secure' % 'nos', [])) +User.objects.annotate(val=RawSQL('{}secure'.format('no'), [])) +raw = '"username") AS "val" FROM "auth_user" WHERE "username"="admin" --' +User.objects.annotate(val=RawSQL(raw, [])) +raw = '"username") AS "val" FROM "auth_user"' \ + ' WHERE "username"="admin" OR 1=%s --' +User.objects.annotate(val=RawSQL(raw, [0])) +User.objects.annotate(val=RawSQL(sql='{}secure'.format('no'), params=[])) +User.objects.annotate(val=RawSQL(params=[], sql='{}secure'.format('no'))) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 8ce687e0f6a73..313218cca2ded 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -612,6 +612,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { ]) { flake8_bandit::rules::shell_injection(checker, call); } + if checker.enabled(Rule::DjangoRawSql) { + flake8_bandit::rules::django_raw_sql(checker, call); + } if checker.enabled(Rule::UnnecessaryGeneratorList) { flake8_comprehensions::rules::unnecessary_generator_list( checker, expr, func, args, keywords, diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 5e23a2e13887e..759ccc54ac49e 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -632,6 +632,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Bandit, "607") => (RuleGroup::Stable, rules::flake8_bandit::rules::StartProcessWithPartialPath), (Flake8Bandit, "608") => (RuleGroup::Stable, rules::flake8_bandit::rules::HardcodedSQLExpression), (Flake8Bandit, "609") => (RuleGroup::Stable, rules::flake8_bandit::rules::UnixCommandWildcardInjection), + (Flake8Bandit, "611") => (RuleGroup::Preview, rules::flake8_bandit::rules::DjangoRawSql), (Flake8Bandit, "612") => (RuleGroup::Stable, rules::flake8_bandit::rules::LoggingConfigInsecureListen), (Flake8Bandit, "701") => (RuleGroup::Stable, rules::flake8_bandit::rules::Jinja2AutoescapeFalse), (Flake8Bandit, "702") => (RuleGroup::Preview, rules::flake8_bandit::rules::MakoTemplates), diff --git a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs index 3ad7a8e421b41..feb2dcb507651 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs @@ -50,6 +50,7 @@ mod tests { #[test_case(Rule::UnixCommandWildcardInjection, Path::new("S609.py"))] #[test_case(Rule::UnsafeYAMLLoad, Path::new("S506.py"))] #[test_case(Rule::WeakCryptographicKey, Path::new("S505.py"))] + #[test_case(Rule::DjangoRawSql, Path::new("S611.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/django_raw_sql.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/django_raw_sql.rs new file mode 100644 index 0000000000000..1491894d5b3ee --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/django_raw_sql.rs @@ -0,0 +1,58 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Expr}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for uses of Django's `RawSQL` function. +/// +/// ## Why is this bad? +/// Django's `RawSQL` function can be used to execute arbitrary SQL queries, +/// which can in turn lead to SQL injection vulnerabilities. +/// +/// ## Example +/// ```python +/// from django.db.models.expressions import RawSQL +/// from django.contrib.auth.models import User +/// +/// User.objects.annotate(val=("%secure" % "nos", [])) +/// ``` +/// +/// ## References +/// - [Django documentation: SQL injection protection](https://docs.djangoproject.com/en/dev/topics/security/#sql-injection-protection) +/// - [Common Weakness Enumeration: CWE-89](https://cwe.mitre.org/data/definitions/89.html) +#[violation] +pub struct DjangoRawSql; + +impl Violation for DjangoRawSql { + #[derive_message_formats] + fn message(&self) -> String { + format!("Use of `RawSQL` can lead to SQL injection vulnerabilities") + } +} + +/// S611 +pub(crate) fn django_raw_sql(checker: &mut Checker, call: &ast::ExprCall) { + if checker + .semantic() + .resolve_call_path(&call.func) + .is_some_and(|call_path| { + matches!( + call_path.as_slice(), + ["django", "db", "models", "expressions", "RawSQL"] + ) + }) + { + if !call + .arguments + .find_argument("sql", 0) + .is_some_and(Expr::is_string_literal_expr) + { + checker + .diagnostics + .push(Diagnostic::new(DjangoRawSql, call.func.range())); + } + } +} diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs index b1ebad53e41ca..b7a655366876f 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs @@ -1,5 +1,6 @@ pub(crate) use assert_used::*; pub(crate) use bad_file_permissions::*; +pub(crate) use django_raw_sql::*; pub(crate) use exec_used::*; pub(crate) use flask_debug_true::*; pub(crate) use hardcoded_bind_all_interfaces::*; @@ -27,6 +28,7 @@ pub(crate) use weak_cryptographic_key::*; mod assert_used; mod bad_file_permissions; +mod django_raw_sql; mod exec_used; mod flask_debug_true; mod hardcoded_bind_all_interfaces; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S611_S611.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S611_S611.py.snap new file mode 100644 index 0000000000000..026360e756ff1 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S611_S611.py.snap @@ -0,0 +1,60 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs +--- +S611.py:5:27: S611 Use of `RawSQL` can lead to SQL injection vulnerabilities + | +4 | User.objects.annotate(val=RawSQL('secure', [])) +5 | User.objects.annotate(val=RawSQL('%secure' % 'nos', [])) + | ^^^^^^ S611 +6 | User.objects.annotate(val=RawSQL('{}secure'.format('no'), [])) +7 | raw = '"username") AS "val" FROM "auth_user" WHERE "username"="admin" --' + | + +S611.py:6:27: S611 Use of `RawSQL` can lead to SQL injection vulnerabilities + | +4 | User.objects.annotate(val=RawSQL('secure', [])) +5 | User.objects.annotate(val=RawSQL('%secure' % 'nos', [])) +6 | User.objects.annotate(val=RawSQL('{}secure'.format('no'), [])) + | ^^^^^^ S611 +7 | raw = '"username") AS "val" FROM "auth_user" WHERE "username"="admin" --' +8 | User.objects.annotate(val=RawSQL(raw, [])) + | + +S611.py:8:27: S611 Use of `RawSQL` can lead to SQL injection vulnerabilities + | + 6 | User.objects.annotate(val=RawSQL('{}secure'.format('no'), [])) + 7 | raw = '"username") AS "val" FROM "auth_user" WHERE "username"="admin" --' + 8 | User.objects.annotate(val=RawSQL(raw, [])) + | ^^^^^^ S611 + 9 | raw = '"username") AS "val" FROM "auth_user"' \ +10 | ' WHERE "username"="admin" OR 1=%s --' + | + +S611.py:11:27: S611 Use of `RawSQL` can lead to SQL injection vulnerabilities + | + 9 | raw = '"username") AS "val" FROM "auth_user"' \ +10 | ' WHERE "username"="admin" OR 1=%s --' +11 | User.objects.annotate(val=RawSQL(raw, [0])) + | ^^^^^^ S611 +12 | User.objects.annotate(val=RawSQL(sql='{}secure'.format('no'), params=[])) +13 | User.objects.annotate(val=RawSQL(params=[], sql='{}secure'.format('no'))) + | + +S611.py:12:27: S611 Use of `RawSQL` can lead to SQL injection vulnerabilities + | +10 | ' WHERE "username"="admin" OR 1=%s --' +11 | User.objects.annotate(val=RawSQL(raw, [0])) +12 | User.objects.annotate(val=RawSQL(sql='{}secure'.format('no'), params=[])) + | ^^^^^^ S611 +13 | User.objects.annotate(val=RawSQL(params=[], sql='{}secure'.format('no'))) + | + +S611.py:13:27: S611 Use of `RawSQL` can lead to SQL injection vulnerabilities + | +11 | User.objects.annotate(val=RawSQL(raw, [0])) +12 | User.objects.annotate(val=RawSQL(sql='{}secure'.format('no'), params=[])) +13 | User.objects.annotate(val=RawSQL(params=[], sql='{}secure'.format('no'))) + | ^^^^^^ S611 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index a3c30890c6f16..8ac638a4c11d1 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3396,6 +3396,7 @@ "S608", "S609", "S61", + "S611", "S612", "S7", "S70",