Improve detecting SQL injections in f-strings

This commit fixes detecting SQL injection
in statements like:

f"SELECT {column_name} FROM foo WHERE id = 1"
f"INSERT INTO {table_name} VALUES (1)"
f"UPDATE {table_name} SET id = 1"

Before this change, the bandit was analyzing statements
by parts, especially, in the case of:
"SELECT {column_name} FROM foo WHERE id = 1"
it was firstly checking "SELECT " for being
an SQL statement and then " FROM foo WHERE id = 1".
Neither of these parts match to defined
regular expressions:

    r"(select\s.*from\s|"
    r"delete\s+from\s|"
    r"insert\s+into\s.*values\s|"
    r"update\s.*set\s)",

Thus SQL injection was not detected.

This commit makes bandit checking the whole SQL
statement for matching the above regexps.
However, since it uses ast.unparse it works
only for python >= 3.9.

Resolves: #916

bandit/plugins/injection_sql.py

@@ -92,8 +92,25 @@ def _evaluate_ast(node):
     elif hasattr(ast, "JoinedStr") and isinstance(
         node._bandit_parent, ast.JoinedStr
     ):
-        statement = node.s
-        wrapper = node._bandit_parent._bandit_parent
+        if hasattr(ast, 'unparse'):
+            # JoinedStr consists of list of Constant and FormattedValue
+            # instances. Let's perform one test for the whole string
+            # and abandon all parts except the first one to raise one
+            # failed test instead of many for the same SQL statement.
+            if node == next(
+                filter(
+                    lambda n: isinstance(n, ast.Constant),
+                    node._bandit_parent.values
+                ),
+                None,
+            ):
+                statement = ast.unparse(node._bandit_parent)
+                wrapper = node._bandit_parent._bandit_parent
+            else:
+                pass
+        else:
+            statement = node.s
+            wrapper = node._bandit_parent._bandit_parent
 
     if isinstance(wrapper, ast.Call):  # wrapped in "execute" call?
         names = ["execute", "executemany"]

examples/sql_statements.py

@@ -20,6 +20,11 @@
 cur.execute("SELECT * FROM foo WHERE id = '" + identifier + "'")
 cur.execute("SELECT * FROM foo WHERE id = '{}'".format(identifier))
 
+# bad f-strings, they are detected in python >= 3.9
+cur.execute(f"SELECT {column_name} FROM foo WHERE id = 1")
+cur.execute(f"INSERT INTO {table_name} VALUES (1)")
+cur.execute(f"UPDATE {table_name} SET id = 1")
+
 # good
 cur.execute("SELECT * FROM foo WHERE id = '%s'", identifier)
 cur.execute("INSERT INTO foo VALUES ('a', 'b', '%s')", value)

tests/functional/test_functional.py

@@ -2,6 +2,7 @@
 # Copyright 2014 Hewlett-Packard Development Company, L.P.
 #
 # SPDX-License-Identifier: Apache-2.0
+import ast
 import os
 import sys
 
@@ -419,17 +420,26 @@ def test_ignore_skip(self):
 
     def test_sql_statements(self):
         """Test for SQL injection through string building."""
+        expected_sev_medium = 17
+        expected_conf_medium = 9
+        tests_for_joined_str = 3
+        # bandit does not detect sql injection in
+        # "SELECT {column_name} FROM foo WHERE id = 1"
+        # statements for python < 3.9
+        if not hasattr(ast, 'unparse'):
+            expected_conf_medium = expected_conf_medium - tests_for_joined_str
+            expected_sev_medium = expected_sev_medium - tests_for_joined_str
         expect = {
             "SEVERITY": {
                 "UNDEFINED": 0,
                 "LOW": 0,
-                "MEDIUM": 14,
+                "MEDIUM": expected_sev_medium,
                 "HIGH": 0,
             },
             "CONFIDENCE": {
                 "UNDEFINED": 0,
                 "LOW": 8,
-                "MEDIUM": 6,
+                "MEDIUM": expected_conf_medium,
                 "HIGH": 0,
             },
         }