Merge pull request #22826 from owen-mc/java/constant-comparison-sanitizer

Java: Recognize constant string comparisons as taint sanitizers

Author: owen-mc

java/ql/lib/change-notes/2026-10-14-constant-value-guards.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Java taint-tracking queries now recognize equality and case-insensitive equality checks against fixed string constants as sanitizers. The queries `java/ssrf` and `java/unvalidated-url-redirection` also recognize checks of `URI.getHost()` against fixed collections of strings, including when the host is normalized to upper or lower case.

java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll

@@ -30,7 +30,7 @@ private module Frameworks {
   private import semmle.code.java.frameworks.ThreadLocal
   private import semmle.code.java.frameworks.ratpack.RatpackExec
   private import semmle.code.java.frameworks.stapler.Stapler
-  private import semmle.code.java.security.ListOfConstantsSanitizer
+  private import semmle.code.java.security.ConstantValueSanitizer
   private import semmle.code.java.security.PathSanitizer
 }
 

…va/security/ListOfConstantsSanitizer.qll …java/security/ConstantValueSanitizer.qlljava/ql/lib/semmle/code/java/security/ListOfConstantsSanitizer.qll renamed to java/ql/lib/semmle/code/java/security/ConstantValueSanitizer.qll java/ql/lib/semmle/code/java/security/ListOfConstantsSanitizer.qll renamed to java/ql/lib/semmle/code/java/security/ConstantValueSanitizer.qll

@@ -1,7 +1,4 @@
-/**
- * Provides a default taint sanitizer identifying comparisons against lists of
- * compile-time constants.
- */
+/** Provides a default taint sanitizer identifying checks against compile-time constants. */
 overlay[local?]
 module;
 
@@ -10,6 +7,7 @@ private import codeql.typeflow.UniversalFlow as UniversalFlow
 private import semmle.code.java.Collections
 private import semmle.code.java.controlflow.Guards
 private import semmle.code.java.dataflow.internal.BaseSSA as Base
+private import semmle.code.java.dataflow.NullGuards
 private import semmle.code.java.dataflow.TaintTracking
 private import semmle.code.java.dataflow.TypeFlow
 private import semmle.code.java.dispatch.VirtualDispatch
@@ -157,6 +155,29 @@ private predicate collectionWithPossibleMutation(FlowNode n) {
   )
 }
 
+/** Holds if `n` is a collection constructed by an API that rejects null elements. */
+private predicate nullRejectingCollectionCreation(FlowNode n) {
+  exists(Call c, Callable tgt |
+    n.asExpr() = c and
+    c.getCallee().getSourceDeclaration() = tgt
+  |
+    tgt.hasQualifiedName("java.util", ["List", "Set"], ["copyOf", "of"])
+    or
+    tgt.hasQualifiedName("com.google.common.collect", ["ImmutableList", "ImmutableSet"],
+      ["copyOf", "of"])
+  )
+}
+
+private module NullRejectingCollectionProp implements BaseUniversalFlow::NullaryPropertySig {
+  predicate hasPropertyBase(FlowNode n) { nullRejectingCollectionCreation(n) }
+
+  predicate barrier = collectionWithPossibleMutation/1;
+}
+
+/** Holds if the given collection cannot contain null. */
+private predicate nullRejectingCollection =
+  BaseUniversalFlow::FlowNullary<NullRejectingCollectionProp>::hasProperty/1;
+
 /**
  * A collection constructor that constructs an empty mutable collection.
  */
@@ -241,27 +262,89 @@ private MethodCall normalizeCaseCall(Expr arg) {
  * Holds if the guard `g` ensures that the expression `e` is one of a set of
  * known constants upon evaluating to `branch`.
  */
-private predicate constantCollectionContainsCheck(Guard g, Expr e, boolean branch) {
-  exists(MethodCall mc, Method m, FlowNode n, Expr checked |
-    g = mc and
-    mc.getMethod().getSourceDeclaration().overridesOrInstantiates*(m) and
+private predicate constantCollectionContainsCall(
+  MethodCall contains, FlowNode collection, Expr checked
+) {
+  exists(Method m, FlowNode n |
+    contains.getMethod().getSourceDeclaration().overridesOrInstantiates*(m) and
     m.hasQualifiedName("java.util", "Collection", "contains") and
-    n.asExpr() = mc.getQualifier() and
+    n.asExpr() = contains.getQualifier() and
     constantCollection(n) and
-    checked = mc.getAnArgument() and
+    checked = contains.getAnArgument() and
+    collection = n
+  )
+}
+
+private predicate constantCollectionContainsCheck(Guard g, Expr e, boolean branch) {
+  exists(Expr checked | checked = e or checked = normalizeCaseCall(e) |
+    constantCollectionContainsCall(g, _, checked) and
+    branch = true
+  )
+}
+
+/**
+ * Holds if the guard `g` ensures that the string expression `e` equals a compile-time constant
+ * upon evaluating to `branch`.
+ */
+private predicate constantStringEqualityCheck(Guard g, Expr e, boolean branch) {
+  exists(MethodCall comparison, Expr constant |
+    g = comparison and
+    comparison.getMethod().getDeclaringType() instanceof TypeString and
+    comparison.getMethod().hasName(["equals", "equalsIgnoreCase"]) and
+    branch = true and
+    (
+      e = comparison.getQualifier() and constant = comparison.getArgument(0)
+      or
+      e = comparison.getArgument(0) and constant = comparison.getQualifier()
+    ) and
+    exists(constant.(CompileTimeConstantExpr).getStringValue())
+  )
+}
+
+/**
+ * Holds if the guard `g` ensures that the expression `e` is one of a set of compile-time constants
+ * upon evaluating to `branch`.
+ */
+predicate constantValueGuardChecks(Guard g, Expr e, boolean branch) {
+  constantStringEqualityCheck(g, e, branch)
+  or
+  constantCollectionContainsCheck(g, e, branch)
+}
+
+/**
+ * Holds if `g` ensures that `e` is a non-null member of a set of compile-time constants upon
+ * evaluating to `branch`.
+ */
+predicate nonNullConstantValueGuardChecks(Guard g, Expr e, boolean branch) {
+  constantStringEqualityCheck(g, e, branch)
+  or
+  exists(FlowNode collection, Expr checked |
+    constantCollectionContainsCall(g, collection, checked) and
     branch = true
   |
-    checked = e or
-    checked = normalizeCaseCall(e)
+    checked = e and
+    (
+      nullRejectingCollection(collection)
+      or
+      e = clearlyNotNullExpr()
+    )
+    or
+    checked = normalizeCaseCall(e) and
+    (
+      nullRejectingCollection(collection)
+      or
+      checked.(MethodCall).getQualifier() = e
+      or
+      e = clearlyNotNullExpr()
+    )
   )
 }
 
 /**
- * A comparison against a list of compile-time constants, sanitizing taint by
- * restricting to a set of known values.
+ * A check against compile-time constants, sanitizing taint by restricting a value to a known set.
  */
-private class ListOfConstantsComparisonSanitizerGuard extends TaintTracking::DefaultTaintSanitizer {
-  ListOfConstantsComparisonSanitizerGuard() {
-    this = DataFlow::BarrierGuard<constantCollectionContainsCheck/3>::getABarrierNode()
+private class ConstantValueSanitizerGuard extends TaintTracking::DefaultTaintSanitizer {
+  ConstantValueSanitizerGuard() {
+    this = DataFlow::BarrierGuard<constantValueGuardChecks/3>::getABarrierNode()
   }
 }

java/ql/lib/semmle/code/java/security/RequestForgery.qll

@@ -14,6 +14,7 @@ private import semmle.code.java.controlflow.Guards
 private import semmle.code.java.dataflow.StringPrefixes
 private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.SSA
+private import semmle.code.java.security.ConstantValueSanitizer
 private import semmle.code.java.security.Sanitizers
 
 /**
@@ -149,18 +150,8 @@ private predicate isHostComparisonSanitizer(Guard guard, Expr e, boolean branch)
       )
     )
   or
-  exists(MethodCall comparison, Expr checked, Expr allowed |
-    guard = comparison and
-    comparison.getMethod().getDeclaringType() instanceof TypeString and
-    comparison.getMethod().hasName(["equals", "equalsIgnoreCase"]) and
-    branch = true and
-    (
-      checked = comparison.getQualifier() and allowed = comparison.getArgument(0)
-      or
-      checked = comparison.getArgument(0) and allowed = comparison.getQualifier()
-    ) and
-    // New cases require a fixed allowlist entry, not another untrusted value.
-    exists(allowed.(CompileTimeConstantExpr).getStringValue()) and
+  exists(Expr checked |
+    nonNullConstantValueGuardChecks(guard, checked, branch) and
     e = getCheckedHost(checked).getQualifier()
   )
 }

java/ql/test/library-tests/default-taint-sanitizer/CONSISTENCY/typeParametersInScope.expected

@@ -0,0 +1 @@
+| Type ConstantValue uses out-of-scope type variable E. Note the Java extractor is known to sometimes do this; the Kotlin extractor should not. |

…est/library-tests/listofconstants/A.java …fault-taint-sanitizer/ConstantValue.javajava/ql/test/library-tests/listofconstants/A.java renamed to java/ql/test/library-tests/default-taint-sanitizer/ConstantValue.java java/ql/test/library-tests/listofconstants/A.java renamed to java/ql/test/library-tests/default-taint-sanitizer/ConstantValue.java

@@ -1,6 +1,6 @@
 import java.util.*;
 
-public class A {
+public class ConstantValue {
   private static final Set<String> SEPARATORS =
       Collections.unmodifiableSet(
           new HashSet<>(Arrays.asList("\t", "\n", ";")));
@@ -32,4 +32,28 @@ private void testAdd(String inp) {
       sink(inp);
     }
   }
+
+  private void checkEquals(String input) {
+    if ("AA".equals(input)) {
+      sink(input);
+    }
+  }
+
+  private void checkEqualsIgnoreCase(String input) {
+    if (input.equalsIgnoreCase("AA")) {
+      sink(input);
+    }
+  }
+
+  private void checkWrongBranch(String input) {
+    if (!"AA".equals(input)) {
+      sink(input);
+    }
+  }
+
+  private void checkUntrustedValue(String input, String allowed) {
+    if (allowed.equals(input)) {
+      sink(input);
+    }
+  }
 }

java/ql/test/library-tests/default-taint-sanitizer/test.expected

@@ -0,0 +1,5 @@
+| ConstantValue.java:12:12:12:20 | separator |
+| ConstantValue.java:23:12:23:17 | schema |
+| ConstantValue.java:32:12:32:14 | inp |
+| ConstantValue.java:38:12:38:16 | input |
+| ConstantValue.java:44:12:44:16 | input |