Implement bandit's 'hardcoded-sql-expressions' S608

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.

README.md

@@ -933,6 +933,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/) on PyPI.
 | S506 | unsafe-yaml-load | Probable use of unsafe loader `{name}` with `yaml.load`. Allows instantiation of arbitrary objects. Consider `yaml.safe_load`. |  |
 | S508 | snmp-insecure-version | The use of SNMPv1 and SNMPv2 is insecure. Use SNMPv3 if able. |  |
 | S509 | snmp-weak-cryptography | You should not use SNMPv3 without encryption. `noAuthNoPriv` & `authNoPriv` is insecure. |  |
+| S608 | hardcoded-sql-expression | Possible SQL injection vector through string-based query construction: "{}" |  |
 | S612 | logging-config-insecure-listen | Use of insecure `logging.config.listen` detected |  |
 | S701 | jinja2-autoescape-false | Using jinja2 templates with `autoescape=False` is dangerous and can lead to XSS. Ensure `autoescape=True` or use the `select_autoescape` function. |  |
 

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

@@ -2396,6 +2396,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
@@ -2803,6 +2806,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,
@@ -2964,6 +2970,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 {
@@ -2985,6 +2994,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,92 @@
+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!(
+    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;