From 8879d9bcb09b909da9d17b8f5686bb928b0d9070 Mon Sep 17 00:00:00 2001
From: Costa Paraskevopoulos <costa.paras1@gmail.com>
Date: Sun, 27 Aug 2023 12:44:47 +1000
Subject: [PATCH] Reduce str.replace to LOW confidence in all cases

Since the rate of false positives may be higher for
str.replace over other string constructions like str.format,
we should reduce to LOW confidence to compensate for this.
---
 bandit/plugins/injection_sql.py     | 22 +++++++++++++++++-----
 tests/functional/test_functional.py |  4 ++--
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/bandit/plugins/injection_sql.py b/bandit/plugins/injection_sql.py
index 4e2eda6df..83295855a 100644
--- a/bandit/plugins/injection_sql.py
+++ b/bandit/plugins/injection_sql.py
@@ -27,6 +27,13 @@
 
  - cursor.execute("SELECT %s FROM derp;" % var)
 
+Use of str.replace in the string construction can also be dangerous.
+For example:
+
+- "SELECT * FROM foo WHERE id = '[VALUE]'".replace("[VALUE]", identifier)
+
+However, such cases are always reported with LOW confidence to compensate
+for false positives, since valid ues of str.replace can be common.
 
 :Example:
 
@@ -80,6 +87,7 @@ def _check_string(data):
 def _evaluate_ast(node):
     wrapper = None
     statement = ""
+    str_replace = False
 
     if isinstance(node._bandit_parent, ast.BinOp):
         out = utils.concat_string(node, node._bandit_parent)
@@ -91,6 +99,8 @@ def _evaluate_ast(node):
         statement = node.s
         # Hierarchy for "".format() is Wrapper -> Call -> Attribute -> Str
         wrapper = node._bandit_parent._bandit_parent._bandit_parent
+        if node._bandit_parent.attr == "replace":
+            str_replace = True
     elif hasattr(ast, "JoinedStr") and isinstance(
         node._bandit_parent, ast.JoinedStr
     ):
@@ -110,19 +120,21 @@ def _evaluate_ast(node):
     if isinstance(wrapper, ast.Call):  # wrapped in "execute" call?
         names = ["execute", "executemany"]
         name = utils.get_called_name(wrapper)
-        return (name in names, statement)
+        return (name in names, statement, str_replace)
     else:
-        return (False, statement)
+        return (False, statement, str_replace)
 
 
 @test.checks("Str")
 @test.test_id("B608")
 def hardcoded_sql_expressions(context):
-    val = _evaluate_ast(context.node)
-    if _check_string(val[1]):
+    execute_call, statement, str_replace = _evaluate_ast(context.node)
+    if _check_string(statement):
         return bandit.Issue(
             severity=bandit.MEDIUM,
-            confidence=bandit.MEDIUM if val[0] else bandit.LOW,
+            confidence=bandit.MEDIUM
+            if execute_call and not str_replace
+            else bandit.LOW,
             cwe=issue.Cwe.SQL_INJECTION,
             text="Possible SQL injection vector through string-based "
             "query construction.",
diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py
index 3f82f764c..846994379 100644
--- a/tests/functional/test_functional.py
+++ b/tests/functional/test_functional.py
@@ -444,8 +444,8 @@ def test_sql_statements(self):
             },
             "CONFIDENCE": {
                 "UNDEFINED": 0,
-                "LOW": 9,
-                "MEDIUM": 11,
+                "LOW": 10,
+                "MEDIUM": 10,
                 "HIGH": 0,
             },
         }