SONARPY-3915 Improve SQLQueriesCheck with reaching definitions analysis (#960)

GitOrigin-RevId: 693473abdbaa10b0ee7648ee4e8f830159bd5480

Author: marc-jasper-sonarsource

python-checks/src/main/java/org/sonar/python/checks/hotspots/SQLQueriesCheck.java

@@ -111,8 +111,8 @@ private void checkCallExpression(SubscriptionContext context) {
   }
 
   private static void addIssue(SubscriptionContext context, CallExpression callExpression) {
-    Optional<Tree> secondary = sensitiveArgumentValue(callExpression);
-    secondary.ifPresent(tree ->  context.addIssue(callExpression, MESSAGE).secondary(tree, null));
+    Optional<Tree> secondary = sensitiveArgumentValue(callExpression, context);
+    secondary.ifPresent(tree -> context.addIssue(callExpression, MESSAGE).secondary(tree, null));
   }
 
   private static boolean isException(CallExpression callExpression, String functionName) {
@@ -123,7 +123,7 @@ private static boolean isException(CallExpression callExpression, String functio
     return argListNode.isEmpty();
   }
 
-  private static Optional<Tree> sensitiveArgumentValue(CallExpression callExpression) {
+  private static Optional<Tree> sensitiveArgumentValue(CallExpression callExpression, SubscriptionContext ctx) {
     List<Argument> argListNode = callExpression.arguments();
     if (argListNode.isEmpty()) {
       return Optional.empty();
@@ -134,14 +134,27 @@ private static Optional<Tree> sensitiveArgumentValue(CallExpression callExpressi
     }
     Expression expression = getExpression(((RegularArgument) arg).expression());
     if (expression.is(Tree.Kind.NAME)) {
-      expression = Expressions.singleAssignedValue((Name) expression);
+      return findFormattedValue((Name) expression, ctx);
     }
-    if (expression != null && isFormatted(expression)) {
+    if (isFormatted(expression)) {
       return Optional.of(expression);
     }
     return Optional.empty();
   }
 
+  private static Optional<Tree> findFormattedValue(Name name, SubscriptionContext ctx) {
+    Set<Expression> values = ctx.valuesAtLocation(name);
+    if (!values.isEmpty()) {
+      return values.stream()
+        .filter(SQLQueriesCheck::isFormatted)
+        .findFirst()
+        .map(Tree.class::cast);
+    }
+    return Optional.ofNullable(Expressions.singleAssignedValue(name))
+      .filter(SQLQueriesCheck::isFormatted)
+      .map(Tree.class::cast);
+  }
+
   private static boolean isFormatted(Expression tree) {
     FormattedStringVisitor visitor = new FormattedStringVisitor();
     tree.accept(visitor);

python-checks/src/test/resources/checks/hotspots/sqlQuery.py

@@ -30,7 +30,7 @@ def query_my_user(request, params):
         MyUser.objects.raw(formatted_request3)  # Noncompliant
         MyUser.objects.raw(y := formatted_request3)  # Noncompliant
         MyUser.objects.raw((y := formatted_request3))  # Noncompliant
-        MyUser.objects.raw(formatted_request4)  # FN, multiple assignments
+        MyUser.objects.raw(formatted_request4)  # Noncompliant
         MyUser.objects.raw(formatted_request5)  # OK
         MyUser.objects.raw(*formatted_request5)  # OK
 
@@ -58,5 +58,21 @@ def query_my_user(request, params):
         MyUser.objects.extra({ 'mycol': "select col from sometable here mycol = %s and othercol = " + value}) # Noncompliant
         MyUser.objects.extra({ 'mycol': "select col from sometable here mycol = %s and othercol = " + ""}) # Noncompliant
 
+    def test_reaching_definitions(self, request, value):
+        query = "SELECT 1"
+        if request:
+            query = 'SELECT * FROM mytable WHERE name = "%s"' % value
+        MyUser.objects.raw(query)  # Noncompliant
+
+        safe_query = "SELECT 1"
+        if request:
+            safe_query = "SELECT 2"
+        MyUser.objects.raw(safe_query)  # OK
+
+        all_formatted = 'SELECT * FROM mytable WHERE name = "%s"' % value
+        if request:
+            all_formatted = f'SELECT * FROM mytable WHERE name = "{value}"'
+        MyUser.objects.raw(all_formatted)  # Noncompliant
+
      def fun():
         pass