[flake8-bandit] Implement django-raw-sql (S611) (#8651)

See: https://github.com/astral-sh/ruff/issues/1646.
This commit is contained in:
Chaojie 2023-11-20 20:21:12 +08:00 committed by GitHub
parent 04f0625d23
commit 653e51ae97
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 139 additions and 0 deletions

View file

@ -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')))

View file

@ -612,6 +612,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
]) { ]) {
flake8_bandit::rules::shell_injection(checker, call); 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) { if checker.enabled(Rule::UnnecessaryGeneratorList) {
flake8_comprehensions::rules::unnecessary_generator_list( flake8_comprehensions::rules::unnecessary_generator_list(
checker, expr, func, args, keywords, checker, expr, func, args, keywords,

View file

@ -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, "607") => (RuleGroup::Stable, rules::flake8_bandit::rules::StartProcessWithPartialPath),
(Flake8Bandit, "608") => (RuleGroup::Stable, rules::flake8_bandit::rules::HardcodedSQLExpression), (Flake8Bandit, "608") => (RuleGroup::Stable, rules::flake8_bandit::rules::HardcodedSQLExpression),
(Flake8Bandit, "609") => (RuleGroup::Stable, rules::flake8_bandit::rules::UnixCommandWildcardInjection), (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, "612") => (RuleGroup::Stable, rules::flake8_bandit::rules::LoggingConfigInsecureListen),
(Flake8Bandit, "701") => (RuleGroup::Stable, rules::flake8_bandit::rules::Jinja2AutoescapeFalse), (Flake8Bandit, "701") => (RuleGroup::Stable, rules::flake8_bandit::rules::Jinja2AutoescapeFalse),
(Flake8Bandit, "702") => (RuleGroup::Preview, rules::flake8_bandit::rules::MakoTemplates), (Flake8Bandit, "702") => (RuleGroup::Preview, rules::flake8_bandit::rules::MakoTemplates),

View file

@ -50,6 +50,7 @@ mod tests {
#[test_case(Rule::UnixCommandWildcardInjection, Path::new("S609.py"))] #[test_case(Rule::UnixCommandWildcardInjection, Path::new("S609.py"))]
#[test_case(Rule::UnsafeYAMLLoad, Path::new("S506.py"))] #[test_case(Rule::UnsafeYAMLLoad, Path::new("S506.py"))]
#[test_case(Rule::WeakCryptographicKey, Path::new("S505.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<()> { fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path( let diagnostics = test_path(

View file

@ -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()));
}
}
}

View file

@ -1,5 +1,6 @@
pub(crate) use assert_used::*; pub(crate) use assert_used::*;
pub(crate) use bad_file_permissions::*; pub(crate) use bad_file_permissions::*;
pub(crate) use django_raw_sql::*;
pub(crate) use exec_used::*; pub(crate) use exec_used::*;
pub(crate) use flask_debug_true::*; pub(crate) use flask_debug_true::*;
pub(crate) use hardcoded_bind_all_interfaces::*; pub(crate) use hardcoded_bind_all_interfaces::*;
@ -27,6 +28,7 @@ pub(crate) use weak_cryptographic_key::*;
mod assert_used; mod assert_used;
mod bad_file_permissions; mod bad_file_permissions;
mod django_raw_sql;
mod exec_used; mod exec_used;
mod flask_debug_true; mod flask_debug_true;
mod hardcoded_bind_all_interfaces; mod hardcoded_bind_all_interfaces;

View file

@ -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
|

1
ruff.schema.json generated
View file

@ -3396,6 +3396,7 @@
"S608", "S608",
"S609", "S609",
"S61", "S61",
"S611",
"S612", "S612",
"S7", "S7",
"S70", "S70",