Implement bandit's 'hardcoded-sql-expressions' S608 (#2698)

This is an attempt to implement `bandit` rule `B608` (renamed here `S608`).
- https://bandit.readthedocs.io/en/latest/plugins/b608_hardcoded_sql_expressions.html

The rule inspects strings constructed via `+`, `%`, `.format`, and `f""`.

- `+` and `%` via `BinOp`
- `.format` via `Call`
- `f""` via `JoinedString`

Any SQL-ish strings that use Python string formatting are flagged.

The expressions and targeted expression types for the rule come from here:
- 7104b336d3/bandit/plugins/injection_sql.py

> Related Issue: https://github.com/charliermarsh/ruff/issues/1646

crates/ruff/resources/test/fixtures/flake8_bandit/S608.py

@@ -0,0 +1,95 @@
+# single-line failures
+query1 = "SELECT %s FROM table" % (var,) # bad
+query2 = "SELECT var FROM " + table
+query3 = "SELECT " + val + " FROM " + table
+query4 = "SELECT {} FROM table;".format(var)
+query5 = f"SELECT * FROM table WHERE var = {var}"
+
+query6 = "DELETE FROM table WHERE var = %s" % (var,)
+query7 = "DELETE FROM table WHERE VAR = " + var
+query8 = "DELETE FROM " + table + "WHERE var = " + var
+query9 = "DELETE FROM table WHERE var = {}".format(var)
+query10 = f"DELETE FROM table WHERE var = {var}"
+
+query11 = "INSERT INTO table VALUES (%s)" % (var,)
+query12 = "INSERT INTO TABLE VALUES (" + var + ")"
+query13 = "INSERT INTO {} VALUES ({})".format(table, var)
+query14 = f"INSERT INTO {table} VALUES var = {var}"
+
+query15 = "UPDATE %s SET var = %s" % (table, var)
+query16 = "UPDATE " + table + " SET var = " + var
+query17 = "UPDATE {} SET var = {}".format(table, var)
+query18 = f"UPDATE {table} SET var = {var}"
+
+query19 = "select %s from table" % (var,)
+query20 = "select var from " + table
+query21 = "select " + val + " from " + table
+query22 = "select {} from table;".format(var)
+query23 = f"select * from table where var = {var}"
+
+query24 = "delete from table where var = %s" % (var,)
+query25 = "delete from table where var = " + var
+query26 = "delete from " + table + "where var = " + var
+query27 = "delete from table where var = {}".format(var)
+query28 = f"delete from table where var = {var}"
+
+query29 = "insert into table values (%s)" % (var,)
+query30 = "insert into table values (" + var + ")"
+query31 = "insert into {} values ({})".format(table, var)
+query32 = f"insert into {table} values var = {var}"
+
+query33 = "update %s set var = %s" % (table, var)
+query34 = "update " + table + " set var = " + var
+query35 = "update {} set var = {}".format(table, var)
+query36 = f"update {table} set var = {var}"
+
+# multi-line failures
+def query37():
+    return """
+    SELECT *
+    FROM table
+    WHERE var = %s
+    """ % var
+
+def query38():
+    return """
+    SELECT *
+    FROM TABLE
+    WHERE var =
+    """ + var
+
+def query39():
+    return """
+    SELECT *
+    FROM table
+    WHERE var = {}
+    """.format(var)
+
+def query40():
+    return f"""
+    SELECT *
+    FROM table
+    WHERE var = {var}
+    """
+
+def query41():
+    return (
+        "SELECT *"
+        "FROM table"
+        f"WHERE var = {var}"
+    )
+
+# # cursor-wrapped failures
+query42 = cursor.execute("SELECT * FROM table WHERE var = %s" % var)
+query43 = cursor.execute(f"SELECT * FROM table WHERE var = {var}")
+query44 = cursor.execute("SELECT * FROM table WHERE var = {}".format(var))
+query45 = cursor.executemany("SELECT * FROM table WHERE var = %s" % var, [])
+ 
+# # pass
+query = "SELECT * FROM table WHERE id = 1"
+query = "DELETE FROM table WHERE id = 1"
+query = "INSERT INTO table VALUES (1)"
+query = "UPDATE table SET id = 1"
+cursor.execute('SELECT * FROM table WHERE id = %s', var)
+cursor.execute('SELECT * FROM table WHERE id = 1')
+cursor.executemany('SELECT * FROM table WHERE id = %s', [var, var2])

crates/ruff/src/checkers/ast.rs

@@ -2402,6 +2402,9 @@ where
                     self.diagnostics
                         .extend(flake8_bandit::rules::hardcoded_password_func_arg(keywords));
                 }
+                if self.settings.rules.enabled(&Rule::HardcodedSQLExpression) {
+                    flake8_bandit::rules::hardcoded_sql_expression(self, expr);
+                }
                 if self
                     .settings
                     .rules
@@ -2809,6 +2812,9 @@ where
                 {
                     pyflakes::rules::f_string_missing_placeholders(expr, values, self);
                 }
+                if self.settings.rules.enabled(&Rule::HardcodedSQLExpression) {
+                    flake8_bandit::rules::hardcoded_sql_expression(self, expr);
+                }
             }
             ExprKind::BinOp {
                 left,
@@ -2970,6 +2976,9 @@ where
                     if self.settings.rules.enabled(&Rule::PrintfStringFormatting) {
                         pyupgrade::rules::printf_string_formatting(self, expr, left, right);
                     }
+                    if self.settings.rules.enabled(&Rule::HardcodedSQLExpression) {
+                        flake8_bandit::rules::hardcoded_sql_expression(self, expr);
+                    }
                 }
             }
             ExprKind::BinOp {
@@ -2991,6 +3000,9 @@ where
                 {
                     ruff::rules::unpack_instead_of_concatenating_to_collection_literal(self, expr);
                 }
+                if self.settings.rules.enabled(&Rule::HardcodedSQLExpression) {
+                    flake8_bandit::rules::hardcoded_sql_expression(self, expr);
+                }
             }
             ExprKind::UnaryOp { op, operand } => {
                 let check_not_in = self.settings.rules.enabled(&Rule::NotInTest);

crates/ruff/src/registry.rs

@@ -387,6 +387,7 @@ ruff_macros::define_rule_mapping!(
     S105 => rules::flake8_bandit::rules::HardcodedPasswordString,
     S106 => rules::flake8_bandit::rules::HardcodedPasswordFuncArg,
     S107 => rules::flake8_bandit::rules::HardcodedPasswordDefault,
+    S608 => rules::flake8_bandit::rules::HardcodedSQLExpression,
     S108 => rules::flake8_bandit::rules::HardcodedTempFile,
     S110 => rules::flake8_bandit::rules::TryExceptPass,
     S112 => rules::flake8_bandit::rules::TryExceptContinue,

crates/ruff/src/rules/flake8_bandit/mod.rs

@@ -22,6 +22,7 @@ mod tests {
     #[test_case(Rule::HardcodedPasswordString, Path::new("S105.py"); "S105")]
     #[test_case(Rule::HardcodedPasswordFuncArg, Path::new("S106.py"); "S106")]
     #[test_case(Rule::HardcodedPasswordDefault, Path::new("S107.py"); "S107")]
+    #[test_case(Rule::HardcodedSQLExpression, Path::new("S608.py"); "S608")]
     #[test_case(Rule::HardcodedTempFile, Path::new("S108.py"); "S108")]
     #[test_case(Rule::RequestWithoutTimeout, Path::new("S113.py"); "S113")]
     #[test_case(Rule::HashlibInsecureHashFunction, Path::new("S324.py"); "S324")]

crates/ruff/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs

@@ -0,0 +1,105 @@
+use once_cell::sync::Lazy;
+use regex::Regex;
+use ruff_macros::{define_violation, derive_message_formats};
+use rustpython_parser::ast::{Expr, ExprKind, Operator};
+
+use super::super::helpers::string_literal;
+use crate::ast::helpers::{any_over_expr, unparse_expr};
+use crate::ast::types::Range;
+use crate::checkers::ast::Checker;
+use crate::registry::Diagnostic;
+use crate::violation::Violation;
+
+static SQL_REGEX: Lazy<Regex> = Lazy::new(|| {
+    Regex::new(r"(?i)(select\s.*from\s|delete\s+from\s|insert\s+into\s.*values\s|update\s.*set\s)")
+        .unwrap()
+});
+
+define_violation!(
+    /// ### What it does
+    /// Checks for strings that resemble SQL statements involved in some form
+    /// string building operation.
+    ///
+    /// ### Why is this bad?
+    /// SQL injection is a common attack vector for web applications. Unless care
+    /// is taken to sanitize and control the input data when building such
+    /// SQL statement strings, an injection attack becomes possible.
+    ///
+    /// ### Example
+    /// ```python
+    /// query = "DELETE FROM foo WHERE id = '%s'" % identifier
+    /// ```
+    pub struct HardcodedSQLExpression {
+        pub string: String,
+    }
+);
+impl Violation for HardcodedSQLExpression {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let HardcodedSQLExpression { string } = self;
+        format!(
+            "Possible SQL injection vector through string-based query construction: \"{}\"",
+            string.escape_debug()
+        )
+    }
+}
+
+fn has_string_literal(expr: &Expr) -> bool {
+    string_literal(expr).is_some()
+}
+
+fn matches_sql_statement(string: &str) -> bool {
+    SQL_REGEX.is_match(string)
+}
+
+fn unparse_string_format_expression(checker: &mut Checker, expr: &Expr) -> Option<String> {
+    match &expr.node {
+        // "select * from table where val = " + "str" + ...
+        // "select * from table where val = %s" % ...
+        ExprKind::BinOp {
+            op: Operator::Add | Operator::Mod,
+            ..
+        } => {
+            let Some(parent) = checker.current_expr_parent() else {
+                if any_over_expr(expr, &has_string_literal) {
+                    return Some(unparse_expr(expr, checker.stylist));
+                }
+                return None;
+            };
+            // Only evaluate the full BinOp, not the nested components.
+            let ExprKind::BinOp { .. } = &parent.node else {
+                if any_over_expr(expr, &has_string_literal) {
+                    return Some(unparse_expr(expr, checker.stylist));
+                }
+                return None;
+            };
+            None
+        }
+        ExprKind::Call { func, .. } => {
+            let ExprKind::Attribute{ attr, value, .. } = &func.node else {
+                return None;
+            };
+            // "select * from table where val = {}".format(...)
+            if attr == "format" && string_literal(value).is_some() {
+                return Some(unparse_expr(expr, checker.stylist));
+            };
+            None
+        }
+        // f"select * from table where val = {val}"
+        ExprKind::JoinedStr { .. } => Some(unparse_expr(expr, checker.stylist)),
+        _ => None,
+    }
+}
+
+/// S608
+pub fn hardcoded_sql_expression(checker: &mut Checker, expr: &Expr) {
+    match unparse_string_format_expression(checker, expr) {
+        Some(string) if matches_sql_statement(&string) => {
+            checker.diagnostics.push(Diagnostic::new(
+                HardcodedSQLExpression { string },
+                Range::from_located(expr),
+            ));
+        }
+        _ => (),
+    }
+}

crates/ruff/src/rules/flake8_bandit/rules/mod.rs

@@ -9,6 +9,7 @@ pub use hardcoded_password_func_arg::{hardcoded_password_func_arg, HardcodedPass
 pub use hardcoded_password_string::{
     assign_hardcoded_password_string, compare_to_hardcoded_password_string, HardcodedPasswordString,
 };
+pub use hardcoded_sql_expression::{hardcoded_sql_expression, HardcodedSQLExpression};
 pub use hardcoded_tmp_directory::{hardcoded_tmp_directory, HardcodedTempFile};
 pub use hashlib_insecure_hash_functions::{
     hashlib_insecure_hash_functions, HashlibInsecureHashFunction,
@@ -34,6 +35,7 @@ mod hardcoded_bind_all_interfaces;
 mod hardcoded_password_default;
 mod hardcoded_password_func_arg;
 mod hardcoded_password_string;
+mod hardcoded_sql_expression;
 mod hardcoded_tmp_directory;
 mod hashlib_insecure_hash_functions;
 mod jinja2_autoescape_false;

crates/ruff/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S608_S608.py.snap

@@ -0,0 +1,500 @@
+---
+source: crates/ruff/src/rules/flake8_bandit/mod.rs
+expression: diagnostics
+---
+- kind:
+    HardcodedSQLExpression:
+      string: "\"SELECT %s FROM table\" % (var,)"
+  location:
+    row: 2
+    column: 9
+  end_location:
+    row: 2
+    column: 40
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"SELECT var FROM \" + table"
+  location:
+    row: 3
+    column: 9
+  end_location:
+    row: 3
+    column: 35
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"SELECT \" + val + \" FROM \" + table"
+  location:
+    row: 4
+    column: 9
+  end_location:
+    row: 4
+    column: 43
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"SELECT {} FROM table;\".format(var)"
+  location:
+    row: 5
+    column: 9
+  end_location:
+    row: 5
+    column: 44
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"SELECT * FROM table WHERE var = {var}\""
+  location:
+    row: 6
+    column: 9
+  end_location:
+    row: 6
+    column: 49
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"DELETE FROM table WHERE var = %s\" % (var,)"
+  location:
+    row: 8
+    column: 9
+  end_location:
+    row: 8
+    column: 52
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"DELETE FROM table WHERE VAR = \" + var"
+  location:
+    row: 9
+    column: 9
+  end_location:
+    row: 9
+    column: 47
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"DELETE FROM \" + table + \"WHERE var = \" + var"
+  location:
+    row: 10
+    column: 9
+  end_location:
+    row: 10
+    column: 54
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"DELETE FROM table WHERE var = {}\".format(var)"
+  location:
+    row: 11
+    column: 9
+  end_location:
+    row: 11
+    column: 55
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"DELETE FROM table WHERE var = {var}\""
+  location:
+    row: 12
+    column: 10
+  end_location:
+    row: 12
+    column: 48
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"INSERT INTO table VALUES (%s)\" % (var,)"
+  location:
+    row: 14
+    column: 10
+  end_location:
+    row: 14
+    column: 50
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"INSERT INTO TABLE VALUES (\" + var + \")\""
+  location:
+    row: 15
+    column: 10
+  end_location:
+    row: 15
+    column: 50
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"INSERT INTO {} VALUES ({})\".format(table, var)"
+  location:
+    row: 16
+    column: 10
+  end_location:
+    row: 16
+    column: 57
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"INSERT INTO {table} VALUES var = {var}\""
+  location:
+    row: 17
+    column: 10
+  end_location:
+    row: 17
+    column: 51
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"UPDATE %s SET var = %s\" % (table, var)"
+  location:
+    row: 19
+    column: 10
+  end_location:
+    row: 19
+    column: 49
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"UPDATE \" + table + \" SET var = \" + var"
+  location:
+    row: 20
+    column: 10
+  end_location:
+    row: 20
+    column: 49
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"UPDATE {} SET var = {}\".format(table, var)"
+  location:
+    row: 21
+    column: 10
+  end_location:
+    row: 21
+    column: 53
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"UPDATE {table} SET var = {var}\""
+  location:
+    row: 22
+    column: 10
+  end_location:
+    row: 22
+    column: 43
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"select %s from table\" % (var,)"
+  location:
+    row: 24
+    column: 10
+  end_location:
+    row: 24
+    column: 41
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"select var from \" + table"
+  location:
+    row: 25
+    column: 10
+  end_location:
+    row: 25
+    column: 36
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"select \" + val + \" from \" + table"
+  location:
+    row: 26
+    column: 10
+  end_location:
+    row: 26
+    column: 44
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"select {} from table;\".format(var)"
+  location:
+    row: 27
+    column: 10
+  end_location:
+    row: 27
+    column: 45
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"select * from table where var = {var}\""
+  location:
+    row: 28
+    column: 10
+  end_location:
+    row: 28
+    column: 50
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"delete from table where var = %s\" % (var,)"
+  location:
+    row: 30
+    column: 10
+  end_location:
+    row: 30
+    column: 53
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"delete from table where var = \" + var"
+  location:
+    row: 31
+    column: 10
+  end_location:
+    row: 31
+    column: 48
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"delete from \" + table + \"where var = \" + var"
+  location:
+    row: 32
+    column: 10
+  end_location:
+    row: 32
+    column: 55
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"delete from table where var = {}\".format(var)"
+  location:
+    row: 33
+    column: 10
+  end_location:
+    row: 33
+    column: 56
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"delete from table where var = {var}\""
+  location:
+    row: 34
+    column: 10
+  end_location:
+    row: 34
+    column: 48
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"insert into table values (%s)\" % (var,)"
+  location:
+    row: 36
+    column: 10
+  end_location:
+    row: 36
+    column: 50
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"insert into table values (\" + var + \")\""
+  location:
+    row: 37
+    column: 10
+  end_location:
+    row: 37
+    column: 50
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"insert into {} values ({})\".format(table, var)"
+  location:
+    row: 38
+    column: 10
+  end_location:
+    row: 38
+    column: 57
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"insert into {table} values var = {var}\""
+  location:
+    row: 39
+    column: 10
+  end_location:
+    row: 39
+    column: 51
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"update %s set var = %s\" % (table, var)"
+  location:
+    row: 41
+    column: 10
+  end_location:
+    row: 41
+    column: 49
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"update \" + table + \" set var = \" + var"
+  location:
+    row: 42
+    column: 10
+  end_location:
+    row: 42
+    column: 49
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"update {} set var = {}\".format(table, var)"
+  location:
+    row: 43
+    column: 10
+  end_location:
+    row: 43
+    column: 53
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"update {table} set var = {var}\""
+  location:
+    row: 44
+    column: 10
+  end_location:
+    row: 44
+    column: 43
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"\\n    SELECT *\\n    FROM table\\n    WHERE var = %s\\n    \" % var"
+  location:
+    row: 48
+    column: 11
+  end_location:
+    row: 52
+    column: 13
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"\\n    SELECT *\\n    FROM TABLE\\n    WHERE var =\\n    \" + var"
+  location:
+    row: 55
+    column: 11
+  end_location:
+    row: 59
+    column: 13
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"\\n    SELECT *\\n    FROM table\\n    WHERE var = {}\\n    \".format(var)"
+  location:
+    row: 62
+    column: 11
+  end_location:
+    row: 66
+    column: 19
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"\\n    SELECT *\\n    FROM table\\n    WHERE var = {var}\\n    \""
+  location:
+    row: 69
+    column: 11
+  end_location:
+    row: 73
+    column: 7
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"SELECT *FROM tableWHERE var = {var}\""
+  location:
+    row: 77
+    column: 8
+  end_location:
+    row: 79
+    column: 28
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"SELECT * FROM table WHERE var = %s\" % var"
+  location:
+    row: 83
+    column: 25
+  end_location:
+    row: 83
+    column: 67
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "f\"SELECT * FROM table WHERE var = {var}\""
+  location:
+    row: 84
+    column: 25
+  end_location:
+    row: 84
+    column: 65
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"SELECT * FROM table WHERE var = {}\".format(var)"
+  location:
+    row: 85
+    column: 25
+  end_location:
+    row: 85
+    column: 73
+  fix: ~
+  parent: ~
+- kind:
+    HardcodedSQLExpression:
+      string: "\"SELECT * FROM table WHERE var = %s\" % var"
+  location:
+    row: 86
+    column: 29
+  end_location:
+    row: 86
+    column: 71
+  fix: ~
+  parent: ~
+