SONARPY-2422 Revert checking for FQN instead of a heuristic (#2225)

Author: Seppli11

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

@@ -20,10 +20,13 @@
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
 import org.sonar.check.Rule;
 import org.sonar.plugins.python.api.PythonSubscriptionCheck;
 import org.sonar.plugins.python.api.SubscriptionContext;
@@ -114,14 +117,16 @@ private static Predicate<Expression> isListAnyMatch(Predicate<Expression> pred)
     "django.views.decorators.csrf.csrf_exempt",
     "flask_wtf.csrf.CSRFProtect.exempt"));
 
-  private static boolean isDangerousDecorator(Decorator expression) {
-    return DANGEROUS_DECORATORS.stream().anyMatch(dangerousFqn -> TreeUtils.isDecoratorWithFQN(expression, dangerousFqn));
-  }
-
   /** Raises issue whenever a decorator with something about "CSRF" and "exempt" in the combined name is found. */
   private static void decoratorCsrfExemptCheck(SubscriptionContext subscriptionContext) {
     Decorator decorator = (Decorator) subscriptionContext.syntaxNode();
-    if(isDangerousDecorator(decorator)) {
+    List<String> names = Stream.of(TreeUtils.decoratorNameFromExpression(decorator.expression()))
+      .filter(Objects::nonNull)
+      .flatMap(s -> Arrays.stream(s.split("\\.")))
+      .toList();
+    boolean isDangerous = names.stream().anyMatch(s -> s.toLowerCase(Locale.US).contains("csrf")) &&
+      names.stream().anyMatch(s -> s.toLowerCase(Locale.US).contains("exempt"));
+    if (isDangerous) {
       subscriptionContext.addIssue(decorator.lastToken(), MESSAGE);
     }
   }

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

@@ -19,6 +19,7 @@
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import org.sonar.check.Rule;
@@ -30,14 +31,17 @@
 import org.sonar.plugins.python.api.tree.CallExpression;
 import org.sonar.plugins.python.api.tree.Decorator;
 import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.FileInput;
 import org.sonar.plugins.python.api.tree.FunctionDef;
 import org.sonar.plugins.python.api.tree.ListLiteral;
 import org.sonar.plugins.python.api.tree.RegularArgument;
 import org.sonar.plugins.python.api.tree.StringLiteral;
 import org.sonar.python.semantic.FunctionSymbolImpl;
 import org.sonar.python.tree.FunctionDefImpl;
+import org.sonar.python.tree.TreeUtils;
 
 import static org.sonar.plugins.python.api.tree.Tree.Kind.CALL_EXPR;
+import static org.sonar.plugins.python.api.tree.Tree.Kind.FILE_INPUT;
 import static org.sonar.plugins.python.api.tree.Tree.Kind.FUNCDEF;
 import static org.sonar.plugins.python.api.tree.Tree.Kind.LIST_LITERAL;
 import static org.sonar.plugins.python.api.tree.Tree.Kind.REGULAR_ARGUMENT;
@@ -133,13 +137,25 @@ private static Optional<CallExpression> getFlaskViewDecorator(FunctionDef functi
 
   private static boolean isFlaskRouteDecorator(CallExpression callExpression) {
     Symbol calleeSymbol = callExpression.calleeSymbol();
-    return calleeSymbol != null && "flask.scaffold.Scaffold.route".equals(calleeSymbol.fullyQualifiedName());
+    if (calleeSymbol == null) {
+      return false;
+    }
+    return "route".equals(calleeSymbol.name());
   }
 
   private static void checkFlaskView(CallExpression callExpression, SubscriptionContext ctx) {
     RegularArgument methodsArg = argumentByKeyword("methods", callExpression.arguments());
-    if (methodsArg != null && hasBothUnsafeAndSafeHttpMethods(methodsArg)) {
+    if (methodsArg != null && hasBothUnsafeAndSafeHttpMethods(methodsArg) && isFlaskImported(callExpression)) {
       ctx.addIssue(callExpression, MESSAGE);
     }
   }
+
+  private static boolean isFlaskImported(CallExpression callExpression) {
+    return Optional.ofNullable(TreeUtils.firstAncestorOfKind(callExpression, FILE_INPUT))
+      .filter(fileInput -> ((FileInput) fileInput).globalVariables().stream()
+          .map(Symbol::fullyQualifiedName)
+          .filter(Objects::nonNull)
+          .anyMatch(fqn -> fqn.contains("flask")))
+      .isPresent();
+  }
 }

python-checks/src/test/java/org/sonar/python/checks/hotspots/CsrfDisabledCheckTest.java

@@ -16,14 +16,18 @@
  */
 package org.sonar.python.checks.hotspots;
 
+import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.sonar.python.checks.utils.PythonCheckVerifier;
 
 class CsrfDisabledCheckTest {
-
   private static void testFile(String relPath) {
-    String path = "src/test/resources/checks/hotspots/csrfDisabledCheck/" + relPath;
-    PythonCheckVerifier.verify(path, new CsrfDisabledCheck());
+    testFile(List.of(relPath));
+  }
+
+  private static void testFile(List<String> relPaths) {
+    List<String> absolutePaths = relPaths.stream().map(path -> "src/test/resources/checks/hotspots/csrfDisabledCheck/" + path).toList();
+    PythonCheckVerifier.verify(absolutePaths, new CsrfDisabledCheck());
   }
 
   @Test
@@ -53,7 +57,7 @@ void testGloballyMissingCSRFProtect(){
 
   @Test
   void testExemptDecorators() {
-    testFile("flask/flaskExempt.py");
+    testFile(List.of("flask/flaskExempt.py", "flask/exportedCsrfProtect.py"));
     testFile("django/djangoExempt.py");
   }
 

python-checks/src/test/resources/checks/hotspots/csrfDisabledCheck/flask/exportedCsrfProtect.py

@@ -0,0 +1,6 @@
+from flask import Flask
+from flask_wtf.csrf import CSRFProtect
+
+app = Flask(__name__)
+csrfProtect = CSRFProtect()
+csrfProtect.init_app(app) # Compliant

python-checks/src/test/resources/checks/hotspots/csrfDisabledCheck/flask/flaskExempt.py

@@ -1,6 +1,7 @@
 def exemptExample():
   from flask import Flask
   from flask_wtf.csrf import CSRFProtect
+  from exportedCsrfProtect import csrfProtect as exported_csrf_protect
 
   app = Flask(__name__)
   csrfProtect = CSRFProtect()
@@ -9,5 +10,15 @@ def exemptExample():
   @app.route('/csrftest1/', methods=['POST'])
   @csrfProtect.exempt # Noncompliant {{Make sure disabling CSRF protection is safe here.}}
   #            ^^^^^^
-  def csrftestpost():
+  def csrftestpost1():
+      pass
+
+  @app.route('/csrftest1/', methods=['POST'])
+  @exported_csrf_protect.exempt # Noncompliant
+  def csrftestpost2():
+      pass
+
+  @app.route('/csrftest1/', methods=['POST'])
+  @exported_csrf_protect.something_else
+  def csrftestpost3():
       pass

python-checks/src/test/resources/checks/hotspots/unsafeHttpMethods/flask/exportedBlueprint.py

@@ -0,0 +1,4 @@
+from flask import Blueprint, request
+from flask import Response
+
+routes = Blueprint('methods', __name__)

python-checks/src/test/resources/checks/hotspots/unsafeHttpMethods/flask/views.py

@@ -1,5 +1,6 @@
 from flask import Blueprint, request
 from flask import Response
+from exportedBlueprint import routes as other_routes
 
 methods = Blueprint('methods', __name__)
 
@@ -8,6 +9,16 @@
 def sensitive1():
     return Response("Method: " + request.method, 200)
 
+# 127.0.0.1:5000/methods/sensitive1
+@other_routes.route('/sensitive1', methods=['GET', 'POST'])  # Noncompliant
+def sensitive1_other_routes():
+    return Response("Method: " + request.method, 200)
+
+# 127.0.0.1:5000/methods/compliant1
+@other_routes.route('/compliant1')  # Compliant
+def compliant1_other_routes():
+    return Response("Method: " + request.method, 200)
+
 # 127.0.0.1:5000/methods/compliant1
 @methods.route('/compliant1')  # Compliant
 def compliant1():