Java: fix @Pattern sanitization for java/path-injection

java/ql/lib/change-notes/2026-07-03-fix-pattern-sanitizer-tainted-path.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Regular expression checks via annotation with `@javax.validation.constraints.Pattern` are now recognized as sanitizers for `java/path-injection`.

java/ql/lib/semmle/code/java/Concepts.qll

@@ -25,6 +25,13 @@ private module Frameworks {
  *
  * These are either method calls, which return `true` when there is a match, or
  * annotations, which are considered to match if they are present.
+ *
+ * To use a `RegexMatch` as a barrier or sanitizer, use the `RegexMatchBarrier`
+ * module rather than instantiating `DataFlow::BarrierGuard` directly. An
+ * annotation match does not dominate the expression it constrains, so it
+ * cannot act as a control-flow barrier guard; `RegexMatchBarrier` handles
+ * method-call matches as barrier guards and annotation matches as direct
+ * barriers, so callers need not distinguish the two.
  */
 class RegexMatch extends Expr instanceof RegexMatch::Range {
   /** Gets the expression for the regex being executed by this node. */
@@ -33,7 +40,13 @@ class RegexMatch extends Expr instanceof RegexMatch::Range {
   /** Gets an expression for the string to be searched or matched against. */
   Expr getString() { result = super.getString() }
 
-  /** Gets an expression to be sanitized. */
+  /**
+   * Gets an expression to be sanitized.
+   *
+   * To turn these into barrier nodes, use the `RegexMatchBarrier` module (in
+   * `semmle.code.java.security.Sanitizers`), which correctly handles both
+   * method-call and annotation matches.
+   */
   Expr getASanitizedExpr() { result = [this.getString(), super.getAdditionalSanitizedExpr()] }
 
   /**

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

@@ -40,7 +40,7 @@ private class LineBreaksLogInjectionSanitizer extends LogInjectionSanitizer {
   LineBreaksLogInjectionSanitizer() {
     logInjectionSanitizer(this.asExpr())
     or
-    this = DataFlow::BarrierGuard<logInjectionGuard/3>::getABarrierNode()
+    this = RegexMatchBarrier<logInjectionGuard/3>::getABarrierNode()
   }
 }
 
@@ -90,13 +90,6 @@ private predicate logInjectionSanitizer(Expr e) {
       target.getStringValue() = ["\n", "\r", "\\n", "\\r", "\\R"]
     )
   )
-  or
-  exists(RegexMatch rm, CompileTimeConstantExpr target |
-    rm instanceof Annotation and
-    e = rm.getASanitizedExpr() and
-    target = rm.getRegex() and
-    regexPreventsLogInjection(target.getStringValue(), true)
-  )
 }
 
 /**
@@ -113,7 +106,6 @@ private predicate logInjectionGuard(Guard g, Expr e, boolean branch) {
   or
   exists(RegexMatch rm, CompileTimeConstantExpr target |
     rm = g and
-    not rm instanceof Annotation and
     target = rm.getRegex() and
     e = rm.getASanitizedExpr()
   |

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

@@ -10,6 +10,7 @@ private import semmle.code.java.dataflow.SSA
 private import semmle.code.java.frameworks.kotlin.IO
 private import semmle.code.java.frameworks.kotlin.Text
 private import semmle.code.java.dataflow.Nullness
+private import semmle.code.java.security.Sanitizers
 
 /** A sanitizer that protects against path injection vulnerabilities. */
 abstract class PathInjectionSanitizer extends DataFlow::Node { }
@@ -493,7 +494,8 @@ private predicate directoryCharactersGuard(Guard g, Expr e, boolean branch) {
  */
 private class DirectoryCharactersSanitizer extends PathInjectionSanitizer {
   DirectoryCharactersSanitizer() {
-    this.asExpr() instanceof ReplaceDirectoryCharactersSanitizer or
-    this = DataFlow::BarrierGuard<directoryCharactersGuard/3>::getABarrierNode()
+    this.asExpr() instanceof ReplaceDirectoryCharactersSanitizer
+    or
+    this = RegexMatchBarrier<directoryCharactersGuard/3>::getABarrierNode()
   }
 }

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

@@ -37,31 +37,61 @@ class SimpleTypeSanitizer extends DataFlow::Node {
  *
  * This is overapproximate: we do not attempt to reason about the correctness of the regexp.
  *
- * Use this if you want to define a derived `DataFlow::BarrierGuard` without
- * make the type recursive. Otherwise use `RegexpCheckBarrier`.
+ * This holds for both method-call and annotation regular-expression matches.
+ * Used directly with `DataFlow::BarrierGuard`, only method-call matches yield
+ * barrier nodes, since an annotation does not dominate the expression it
+ * constrains. Use `RegexMatchBarrier` (as `RegexpCheckBarrier` does) to also
+ * treat annotation matches as barriers.
  */
 predicate regexpMatchGuardChecks(Guard guard, Expr e, boolean branch) {
-  exists(RegexMatch rm | not rm instanceof Annotation |
+  exists(RegexMatch rm |
     guard = rm and
     e = rm.getASanitizedExpr() and
     branch = true
   )
 }
 
+/**
+ * Holds if the guard `g`, which may be a `RegexMatch`, sanitizes `e` when it
+ * evaluates to `branch`. This is the guard-check signature used by
+ * `RegexMatchBarrier`.
+ */
+signature predicate regexMatchGuardChecksSig(Guard g, Expr e, boolean branch);
+
+/**
+ * Given a guard-check predicate that includes regular-expression matches,
+ * this module provides the corresponding barrier nodes, transparently handling
+ * the two kinds of `RegexMatch`:
+ *  - method calls, which act as ordinary control-flow barrier guards, and
+ *  - annotations, which don't dominate the expressions they constrain and so
+ *    are instead treated as direct barriers.
+ *
+ * Callers do not need to distinguish between the two: include annotation
+ * matches in `guardChecks` (with `branch = true`, since an annotation is
+ * always present) and use `getABarrierNode` in place of a direct
+ * `DataFlow::BarrierGuard` instantiation.
+ */
+module RegexMatchBarrier<regexMatchGuardChecksSig/3 guardChecks> {
+  /** Gets a node that is sanitized by a regular-expression match. */
+  DataFlow::Node getABarrierNode() {
+    result = DataFlow::BarrierGuard<guardChecks/3>::getABarrierNode()
+    or
+    // An annotation doesn't dominate the expression it constrains, so the
+    // barrier-guard machinery above never yields a node for it; treat it as a
+    // direct barrier instead.
+    exists(Guard g, Expr e |
+      g instanceof Annotation and guardChecks(g, e, true) and result.asExpr() = e
+    )
+  }
+}
+
 /**
  * A check against a regular expression, considered as a barrier guard.
  *
  * This is overapproximate: we do not attempt to reason about the correctness of the regexp.
  */
 class RegexpCheckBarrier extends DataFlow::Node {
-  RegexpCheckBarrier() {
-    this = DataFlow::BarrierGuard<regexpMatchGuardChecks/3>::getABarrierNode()
-    or
-    // Annotations don't fit into the model of barrier guards because the
-    // annotation doesn't dominate the sanitized expression, so we instead
-    // treat them as barriers directly.
-    exists(RegexMatch rm | rm instanceof Annotation | this.asExpr() = rm.getString())
-  }
+  RegexpCheckBarrier() { this = RegexMatchBarrier<regexpMatchGuardChecks/3>::getABarrierNode() }
 }
 
 /**

java/ql/test/query-tests/security/CWE-022/semmle/tests/SanitizationTests2.java

@@ -0,0 +1,30 @@
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+@RestController
+public class SanitizationTests2 {
+
+    @GetMapping("/product/vuln/path-injection")
+    public ResponseEntity<String> vulnerablePathInjection(@RequestParam("path") String path) throws IOException { // $ Source[java/path-injection]
+        // Intentionally vulnerable for SAST testing: user-controlled path reaches file read sink.
+        try (FileInputStream stream = new FileInputStream(path)) { // $ Alert[java/path-injection]
+            byte[] fileContents = stream.readNBytes(1024);
+            return ResponseEntity.ok(new String(fileContents, StandardCharsets.UTF_8));
+        }
+    }
+
+    @GetMapping("/product/vuln/path-injection-fix")
+    public ResponseEntity<String> vulnerablePathInjectionFix(@RequestParam("path")
+                                                                 @javax.validation.constraints.Pattern(regexp = "[a-zA-Z0-9]*") String path) throws IOException {
+        try (FileInputStream stream = new FileInputStream(path)) {
+            byte[] fileContents = stream.readNBytes(1024);
+            return ResponseEntity.ok(new String(fileContents, StandardCharsets.UTF_8));
+        }
+    }
+}

java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected

@@ -1,4 +1,5 @@
 #select
+| SanitizationTests2.java:16:59:16:62 | path | SanitizationTests2.java:14:59:14:91 | path : String | SanitizationTests2.java:16:59:16:62 | path | This path depends on a $@. | SanitizationTests2.java:14:59:14:91 | path | user-provided value |
 | TaintedPath.java:16:71:16:78 | filename | TaintedPath.java:13:58:13:78 | getInputStream(...) : InputStream | TaintedPath.java:16:71:16:78 | filename | This path depends on a $@. | TaintedPath.java:13:58:13:78 | getInputStream(...) | user-provided value |
 | Test.java:37:52:37:68 | (...)... | Test.java:32:16:32:45 | getParameter(...) : String | Test.java:37:52:37:68 | (...)... | This path depends on a $@. | Test.java:32:16:32:45 | getParameter(...) | user-provided value |
 | Test.java:39:32:39:48 | (...)... | Test.java:32:16:32:45 | getParameter(...) : String | Test.java:39:32:39:48 | (...)... | This path depends on a $@. | Test.java:32:16:32:45 | getParameter(...) | user-provided value |
@@ -77,6 +78,7 @@
 | Test.java:199:19:199:33 | (...)... | Test.java:32:16:32:45 | getParameter(...) : String | Test.java:199:19:199:33 | (...)... | This path depends on a $@. | Test.java:32:16:32:45 | getParameter(...) | user-provided value |
 | Test.java:204:20:204:36 | (...)... | Test.java:32:16:32:45 | getParameter(...) : String | Test.java:204:20:204:36 | (...)... | This path depends on a $@. | Test.java:32:16:32:45 | getParameter(...) | user-provided value |
 edges
+| SanitizationTests2.java:14:59:14:91 | path : String | SanitizationTests2.java:16:59:16:62 | path | provenance | Sink:MaD:23 |
 | TaintedPath.java:13:17:13:89 | new BufferedReader(...) : BufferedReader | TaintedPath.java:14:27:14:40 | filenameReader : BufferedReader | provenance |  |
 | TaintedPath.java:13:36:13:88 | new InputStreamReader(...) : InputStreamReader | TaintedPath.java:13:17:13:89 | new BufferedReader(...) : BufferedReader | provenance | MaD:74 |
 | TaintedPath.java:13:58:13:78 | getInputStream(...) : InputStream | TaintedPath.java:13:36:13:88 | new InputStreamReader(...) : InputStreamReader | provenance | Src:MaD:72 MaD:76 |
@@ -312,6 +314,8 @@ models
 | 75 | Summary: java.io; BufferedReader; true; readLine; ; ; Argument[this]; ReturnValue; taint; manual |
 | 76 | Summary: java.io; InputStreamReader; false; InputStreamReader; ; ; Argument[0]; Argument[this]; taint; manual |
 nodes
+| SanitizationTests2.java:14:59:14:91 | path : String | semmle.label | path : String |
+| SanitizationTests2.java:16:59:16:62 | path | semmle.label | path |
 | TaintedPath.java:13:17:13:89 | new BufferedReader(...) : BufferedReader | semmle.label | new BufferedReader(...) : BufferedReader |
 | TaintedPath.java:13:36:13:88 | new InputStreamReader(...) : InputStreamReader | semmle.label | new InputStreamReader(...) : InputStreamReader |
 | TaintedPath.java:13:58:13:78 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |

java/ql/test/query-tests/security/CWE-022/semmle/tests/options

@@ -1 +1 @@
-// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/servlet-api-2.4:${testdir}/../../../../../stubs/apache-commons-io-2.6:${testdir}/../../../../../stubs/cargo:${testdir}/../../../../../stubs/apache-ant-1.10.13:${testdir}/../../../../../stubs/stapler-1.263:${testdir}/../../../../../stubs/javax-servlet-2.5:${testdir}/../../../../../stubs/apache-commons-jelly-1.0.1:${testdir}/../../../../../stubs/apache-commons-fileupload-1.4:${testdir}/../../../../../stubs/saxon-xqj-9.x:${testdir}/../../../../../stubs/apache-commons-beanutils:${testdir}/../../../../../stubs/dom4j-2.1.1:${testdir}/../../../../../stubs/apache-commons-lang:${testdir}/../../../../../stubs/jaxen-1.2.0:${testdir}/../../../../../stubs/jmh-1.3.6:${testdir}/../../../../../stubs/springframework-5.8.x:${testdir}/../../../../../stubs/jaxws-api-2.0:${testdir}/../../../../../stubs/apache-cxf
+// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/javax-validation-constraints:${testdir}/../../../../../stubs/servlet-api-2.4:${testdir}/../../../../../stubs/apache-commons-io-2.6:${testdir}/../../../../../stubs/cargo:${testdir}/../../../../../stubs/apache-ant-1.10.13:${testdir}/../../../../../stubs/stapler-1.263:${testdir}/../../../../../stubs/javax-servlet-2.5:${testdir}/../../../../../stubs/apache-commons-jelly-1.0.1:${testdir}/../../../../../stubs/apache-commons-fileupload-1.4:${testdir}/../../../../../stubs/saxon-xqj-9.x:${testdir}/../../../../../stubs/apache-commons-beanutils:${testdir}/../../../../../stubs/dom4j-2.1.1:${testdir}/../../../../../stubs/apache-commons-lang:${testdir}/../../../../../stubs/jaxen-1.2.0:${testdir}/../../../../../stubs/jmh-1.3.6:${testdir}/../../../../../stubs/springframework-5.8.x:${testdir}/../../../../../stubs/jaxws-api-2.0:${testdir}/../../../../../stubs/apache-cxf