From a87c22333f91c3efffcd8692b6bb4870b1002bc1 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 0d4fd8005..408152bda 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,
             },
         }