From 2ab2876bdedadf9749cb11381a79646bb5df1f67 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 20 Nov 2023 12:09:49 +0000 Subject: [PATCH] Tweak logic --- crates/ruff_linter/src/codes.rs | 2 +- .../flake8_bandit/rules/django_raw_sql.rs | 26 +++++++++---------- ...s__flake8_bandit__tests__S611_S611.py.snap | 12 ++++----- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 7a71f6b5bbbc91..759ccc54ac49e5 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -632,7 +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::Stable, rules::flake8_bandit::rules::DjangoRawSql), + (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/rules/django_raw_sql.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/django_raw_sql.rs index 33bc08c0edc6b0..1491894d5b3eed 100644 --- 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 @@ -1,27 +1,27 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{self as ast}; +use ruff_python_ast::{self as ast, Expr}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; /// ## What it does -/// Checks for Django that use `RawSQL` function. +/// Checks for uses of Django's `RawSQL` function. /// /// ## Why is this bad? -/// Django `RawSQL` function can cause SQL injection attack. +/// 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=RawSQL("%secure" % "nos", [])) +/// User.objects.annotate(val=("%secure" % "nos", [])) /// ``` /// /// ## References -/// - [Django documentation: API](https://docs.djangoproject.com/en/dev/ref/models/expressions/#django.db.models.expressions.RawSQL) -/// - [Django documentation: sql injection protection](https://docs.djangoproject.com/en/dev/topics/security/#sql-injection-protection) +/// - [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; @@ -29,7 +29,7 @@ pub struct DjangoRawSql; impl Violation for DjangoRawSql { #[derive_message_formats] fn message(&self) -> String { - format!("Use of RawSQL potential SQL attack vector.") + format!("Use of `RawSQL` can lead to SQL injection vulnerabilities") } } @@ -45,13 +45,11 @@ pub(crate) fn django_raw_sql(checker: &mut Checker, call: &ast::ExprCall) { ) }) { - let sql = if let Some(arg) = call.arguments.find_argument("sql", 0) { - arg - } else { - &call.arguments.find_keyword("sql").unwrap().value - }; - - if !sql.is_string_literal_expr() { + 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/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 index 447a83a7536baa..026360e756ff12 100644 --- 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 @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S611.py:5:27: S611 Use of RawSQL potential SQL attack vector. +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', [])) @@ -10,7 +10,7 @@ S611.py:5:27: S611 Use of RawSQL potential SQL attack vector. 7 | raw = '"username") AS "val" FROM "auth_user" WHERE "username"="admin" --' | -S611.py:6:27: S611 Use of RawSQL potential SQL attack vector. +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', [])) @@ -20,7 +20,7 @@ S611.py:6:27: S611 Use of RawSQL potential SQL attack vector. 8 | User.objects.annotate(val=RawSQL(raw, [])) | -S611.py:8:27: S611 Use of RawSQL potential SQL attack vector. +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" --' @@ -30,7 +30,7 @@ S611.py:8:27: S611 Use of RawSQL potential SQL attack vector. 10 | ' WHERE "username"="admin" OR 1=%s --' | -S611.py:11:27: S611 Use of RawSQL potential SQL attack vector. +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 --' @@ -40,7 +40,7 @@ S611.py:11:27: S611 Use of RawSQL potential SQL attack vector. 13 | User.objects.annotate(val=RawSQL(params=[], sql='{}secure'.format('no'))) | -S611.py:12:27: S611 Use of RawSQL potential SQL attack vector. +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])) @@ -49,7 +49,7 @@ S611.py:12:27: S611 Use of RawSQL potential SQL attack vector. 13 | User.objects.annotate(val=RawSQL(params=[], sql='{}secure'.format('no'))) | -S611.py:13:27: S611 Use of RawSQL potential SQL attack vector. +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=[]))