SONARJAVA-4884 S4507: Support detection of enabled Debug Features in …

…Spring programmatically (#4739)
SonarSource · Mar 28, 2024 · 373182d · 373182d
1 parent 5b514a4
commit 373182d
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 5 deletions.
diff --git a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json
@@ -1970,7 +1970,7 @@
   {
     "ruleKey": "S4507",
     "hasTruePositives": true,
-    "falseNegatives": 0,
+    "falseNegatives": 2,
     "falsePositives": 0
   },
   {

diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S4507.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S4507.json
@@ -1,6 +1,6 @@
 {
   "ruleKey": "S4507",
   "hasTruePositives": true,
-  "falseNegatives": 0,
+  "falseNegatives": 2,
   "falsePositives": 0
-}
+}
diff --git a/java-checks-test-sources/default/pom.xml b/java-checks-test-sources/default/pom.xml
@@ -306,7 +306,7 @@
     <dependency>
       <groupId>org.springframework.security</groupId>
       <artifactId>spring-security-config</artifactId>
-      <version>5.0.5.RELEASE</version>
+      <version>5.4.0</version>
       <scope>provided</scope>
     </dependency>
     <dependency>

diff --git a/java-checks-test-sources/default/src/main/java/checks/security/DebugFeatureEnabledCheck.java b/java-checks-test-sources/default/src/main/java/checks/security/DebugFeatureEnabledCheck.java
@@ -3,6 +3,8 @@
 import android.webkit.WebView;
 import android.webkit.WebViewFactoryProvider;
 import java.lang.reflect.InvocationTargetException;
+import org.springframework.security.config.annotation.web.builders.WebSecurity;
+import org.springframework.security.config.annotation.web.configuration.WebSecurityCustomizer;
 
 // Tests for printStackTrace.
 // Tests for @EnableWebSecurity are in files/non-compiling/checks/security/DebugFeatureEnabledCheck.java
@@ -60,4 +62,19 @@ void fun() {
       ex.printStackTrace();
     }
   }
+
+  void foo(WebSecurity web, boolean cond){
+    web.debug(true);   // Noncompliant [sc=9;ec=14] {{Make sure this debug feature is deactivated before delivering the code in production.}}
+    web.debug(false);
+    web.debug(cond);
+  }
+
+  public WebSecurityCustomizer debugCustomizer() {
+    return (web) -> web.debug(true); // Noncompliant [sc=25;ec=30] {{Make sure this debug feature is deactivated before delivering the code in production.}}
+  }
+
+  public WebSecurityCustomizer nonDebugCustomizer() {
+    return (web) -> web.debug(false);
+  }
+
 }
diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/DebugFeatureEnabledCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/DebugFeatureEnabledCheck.java
@@ -51,6 +51,10 @@ public class DebugFeatureEnabledCheck extends IssuableSubscriptionVisitor {
       .ofSubTypes("android.webkit.WebView", "android.webkit.WebViewFactoryProvider$Statics")
       .names("setWebContentsDebuggingEnabled").addParametersMatcher("boolean").build();
 
+  private static final MethodMatchers DEBUG_MATCHER = MethodMatchers.create()
+    .ofSubTypes("org.springframework.security.config.annotation.web.builders.WebSecurity")
+    .names("debug").addParametersMatcher("boolean").build();
+
   private final Deque<Symbol.TypeSymbol> enclosingClass = new LinkedList<>();
 
   @Override
@@ -82,7 +86,7 @@ public void leaveNode(Tree tree) {
   }
 
   private void checkMethodInvocation(MethodInvocationTree mit) {
-    if (isPrintStackTraceIllegalUsage(mit) || isSetWebContentsDebuggingEnabled(mit)) {
+    if (isPrintStackTraceIllegalUsage(mit) || isSetWebContentsDebuggingEnabled(mit) || isDebugWithTrueArgument(mit)) {
       reportIssue(ExpressionUtils.methodName(mit), MESSAGE);
     }
   }
@@ -96,6 +100,14 @@ private static boolean isSetWebContentsDebuggingEnabled(MethodInvocationTree mit
       Boolean.TRUE.equals(ExpressionUtils.resolveAsConstant(mit.arguments().get(0)));
   }
 
+  private static boolean isDebugWithTrueArgument(MethodInvocationTree mit){
+    if (!DEBUG_MATCHER.matches(mit.methodSymbol())){
+      return false;
+    }
+    var cstArg = mit.arguments().get(0).asConstant();
+    return cstArg.isPresent() && cstArg.get().equals(true);
+  }
+
   private void checkAnnotation(AnnotationTree annotation) {
     if (annotation.symbolType().is("org.springframework.security.config.annotation.web.configuration.EnableWebSecurity")) {
       annotation.arguments().stream()