[flake8-bandit] Detect patterns from multi line SQL statements (`S6…

…08`) (#13574) Co-authored-by: Santhosh Solomon <santhosh@advarisk.com> Co-authored-by: Micha Reiser <micha@reiser.io>
astral-sh · Oct 17, 2024 · 4ea4bbb · 4ea4bbb
1 parent ed4a0b3
commit 4ea4bbb
Show file tree

Hide file tree

Showing 3 changed files with 154 additions and 29 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S608.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S608.py
@@ -95,18 +95,62 @@ def query41():
 cursor.executemany('SELECT * FROM table WHERE id = %s', [var, var2])
 
 # # INSERT without INTO (e.g. MySQL and derivatives)
-query = "INSERT table VALUES (%s)" % (var,)
+query46 = "INSERT table VALUES (%s)" % (var,)
 
 # # REPLACE (e.g. MySQL and derivatives, SQLite)
-query = "REPLACE INTO table VALUES (%s)" % (var,)
-query = "REPLACE table VALUES (%s)" % (var,)
+query47 = "REPLACE INTO table VALUES (%s)" % (var,)
+query48 = "REPLACE table VALUES (%s)" % (var,)
 
-query = "Deselect something that is not SQL even though it has a ' from ' somewhere in %s." % "there"
+query49 = "Deselect something that is not SQL even though it has a ' from ' somewhere in %s." % "there"
 
 # # pass
 ["select colA from tableA"] + ["select colB from tableB"]
 "SELECT * FROM " + (["table1"] if x > 0 else ["table2"])
 
 # # errors
-"SELECT * FROM " + ("table1" if x > 0 else "table2")
-"SELECT * FROM " + ("table1" if x > 0 else ["table2"])
+"SELECT * FROM " + ("table1" if x > 0 else "table2") # query50
+"SELECT * FROM " + ("table1" if x > 0 else ["table2"]) # query51
+
+# test cases from #12044
+
+def query52():
+    return f"""
+SELECT {var}
+    FROM bar
+    """
+
+def query53():
+    return f"""
+    SELECT
+        {var}
+    FROM bar
+    """
+
+def query54():
+    return f"""
+    SELECT {var}
+    FROM
+        bar
+    """
+
+query55 = f"""SELECT * FROM
+{var}.table
+"""
+
+query56 = f"""SELECT *
+FROM {var}.table
+"""
+
+query57 = f"""
+SELECT *
+FROM {var}.table
+"""
+
+query57 = f"""
+PRESELECT *
+FROM {var}.table
+"""
+
+# to be handled seperately
+# query58 = f"SELECT\
+#  * FROM {var}.table"
diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs
@@ -1,18 +1,17 @@
 use once_cell::sync::Lazy;
 use regex::Regex;
-use ruff_python_ast::{self as ast, Expr, Operator};
-
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::str::raw_contents;
+use ruff_python_ast::{self as ast, Expr, Operator};
 use ruff_source_file::Locator;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
 
 static SQL_REGEX: Lazy<Regex> = Lazy::new(|| {
-    Regex::new(r"(?i)\b(select\s.+\sfrom\s|delete\s+from\s|(insert|replace)\s.+\svalues\s|update\s.+\sset\s)")
-        .unwrap()
+    Regex::new(r"(?i)\b(select\s+.*\s+from\s|delete\s+from\s|(insert|replace)\s+.*\s+values\s|update\s+.*\s+set\s)")
+    .unwrap()
 });
 
 /// ## What it does
@@ -88,6 +87,7 @@ pub(crate) fn hardcoded_sql_expression(checker: &mut Checker, expr: &Expr) {
             };
             string.value.to_str().escape_default().to_string()
         }
+
         // f"select * from table where val = {val}"
         Expr::FString(f_string) => concatenated_f_string(f_string, checker.locator()),
         _ => return,
@@ -113,9 +113,7 @@ pub(crate) fn hardcoded_sql_expression(checker: &mut Checker, expr: &Expr) {
 fn concatenated_f_string(expr: &ast::ExprFString, locator: &Locator) -> String {
     expr.value
         .iter()
-        .filter_map(|part| {
-            raw_contents(locator.slice(part)).map(|s| s.escape_default().to_string())
-        })
+        .filter_map(|part| raw_contents(locator.slice(part)))
         .collect()
 }
 

diff --git a/...rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S608_S608.py.snap b/...rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S608_S608.py.snap
@@ -452,45 +452,128 @@ S608.py:86:30: S608 Possible SQL injection vector through string-based query con
 88 | # # pass
    |
 
-S608.py:98:9: S608 Possible SQL injection vector through string-based query construction
+S608.py:98:11: S608 Possible SQL injection vector through string-based query construction
     |
  97 | # # INSERT without INTO (e.g. MySQL and derivatives)
- 98 | query = "INSERT table VALUES (%s)" % (var,)
-    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
+ 98 | query46 = "INSERT table VALUES (%s)" % (var,)
+    |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
  99 | 
 100 | # # REPLACE (e.g. MySQL and derivatives, SQLite)
     |
 
-S608.py:101:9: S608 Possible SQL injection vector through string-based query construction
+S608.py:101:11: S608 Possible SQL injection vector through string-based query construction
     |
 100 | # # REPLACE (e.g. MySQL and derivatives, SQLite)
-101 | query = "REPLACE INTO table VALUES (%s)" % (var,)
-    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
-102 | query = "REPLACE table VALUES (%s)" % (var,)
+101 | query47 = "REPLACE INTO table VALUES (%s)" % (var,)
+    |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
+102 | query48 = "REPLACE table VALUES (%s)" % (var,)
     |
 
-S608.py:102:9: S608 Possible SQL injection vector through string-based query construction
+S608.py:102:11: S608 Possible SQL injection vector through string-based query construction
     |
 100 | # # REPLACE (e.g. MySQL and derivatives, SQLite)
-101 | query = "REPLACE INTO table VALUES (%s)" % (var,)
-102 | query = "REPLACE table VALUES (%s)" % (var,)
-    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
+101 | query47 = "REPLACE INTO table VALUES (%s)" % (var,)
+102 | query48 = "REPLACE table VALUES (%s)" % (var,)
+    |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
 103 | 
-104 | query = "Deselect something that is not SQL even though it has a ' from ' somewhere in %s." % "there"
+104 | query49 = "Deselect something that is not SQL even though it has a ' from ' somewhere in %s." % "there"
     |
 
 S608.py:111:1: S608 Possible SQL injection vector through string-based query construction
     |
 110 | # # errors
-111 | "SELECT * FROM " + ("table1" if x > 0 else "table2")
+111 | "SELECT * FROM " + ("table1" if x > 0 else "table2") # query50
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
-112 | "SELECT * FROM " + ("table1" if x > 0 else ["table2"])
+112 | "SELECT * FROM " + ("table1" if x > 0 else ["table2"]) # query51
     |
 
 S608.py:112:1: S608 Possible SQL injection vector through string-based query construction
     |
 110 | # # errors
-111 | "SELECT * FROM " + ("table1" if x > 0 else "table2")
-112 | "SELECT * FROM " + ("table1" if x > 0 else ["table2"])
+111 | "SELECT * FROM " + ("table1" if x > 0 else "table2") # query50
+112 | "SELECT * FROM " + ("table1" if x > 0 else ["table2"]) # query51
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
+113 | 
+114 | # test cases from #12044
+    |
+
+S608.py:117:12: S608 Possible SQL injection vector through string-based query construction
+    |
+116 |   def query52():
+117 |       return f"""
+    |  ____________^
+118 | | SELECT {var}
+119 | |     FROM bar
+120 | |     """
+    | |_______^ S608
+121 |   
+122 |   def query53():
+    |
+
+S608.py:123:12: S608 Possible SQL injection vector through string-based query construction
+    |
+122 |   def query53():
+123 |       return f"""
+    |  ____________^
+124 | |     SELECT
+125 | |         {var}
+126 | |     FROM bar
+127 | |     """
+    | |_______^ S608
+128 |   
+129 |   def query54():
+    |
+
+S608.py:130:12: S608 Possible SQL injection vector through string-based query construction
+    |
+129 |   def query54():
+130 |       return f"""
+    |  ____________^
+131 | |     SELECT {var}
+132 | |     FROM
+133 | |         bar
+134 | |     """
+    | |_______^ S608
+135 |   
+136 |   query55 = f"""SELECT * FROM
+    |
+
+S608.py:136:11: S608 Possible SQL injection vector through string-based query construction
+    |
+134 |       """
+135 |   
+136 |   query55 = f"""SELECT * FROM
+    |  ___________^
+137 | | {var}.table
+138 | | """
+    | |___^ S608
+139 |   
+140 |   query56 = f"""SELECT *
+    |
+
+S608.py:140:11: S608 Possible SQL injection vector through string-based query construction
+    |
+138 |   """
+139 |   
+140 |   query56 = f"""SELECT *
+    |  ___________^
+141 | | FROM {var}.table
+142 | | """
+    | |___^ S608
+143 |   
+144 |   query57 = f"""
+    |
+
+S608.py:144:11: S608 Possible SQL injection vector through string-based query construction
+    |
+142 |   """
+143 |   
+144 |   query57 = f"""
+    |  ___________^
+145 | | SELECT *
+146 | | FROM {var}.table
+147 | | """
+    | |___^ S608
+148 |   
+149 |   query57 = f"""
     |