From 077b531e4109e9b14110e8f9a65012190bbae6da Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 3 Jul 2026 11:04:47 +0100 Subject: [PATCH 1/5] Add failing TaintedPath test for @Pattern sanitizer --- .../semmle/tests/SanitizationTests2.java | 30 +++++++++++++++++++ .../CWE-022/semmle/tests/TaintedPath.expected | 8 +++++ .../security/CWE-022/semmle/tests/options | 2 +- 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 java/ql/test/query-tests/security/CWE-022/semmle/tests/SanitizationTests2.java diff --git a/java/ql/test/query-tests/security/CWE-022/semmle/tests/SanitizationTests2.java b/java/ql/test/query-tests/security/CWE-022/semmle/tests/SanitizationTests2.java new file mode 100644 index 000000000000..3dcdfff8cf6f --- /dev/null +++ b/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 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 vulnerablePathInjectionFix(@RequestParam("path") + @javax.validation.constraints.Pattern(regexp = "[a-zA-Z0-9]*") String path) throws IOException { // $ SPURIOUS: Source + try (FileInputStream stream = new FileInputStream(path)) { // $ SPURIOUS: Alert + byte[] fileContents = stream.readNBytes(1024); + return ResponseEntity.ok(new String(fileContents, StandardCharsets.UTF_8)); + } + } +} diff --git a/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected b/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected index 06ab1d6340d8..297187e8ece8 100644 --- a/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected +++ b/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected @@ -1,4 +1,6 @@ #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 | +| SanitizationTests2.java:25:59:25:62 | path | SanitizationTests2.java:23:62:24:139 | path : String | SanitizationTests2.java:25:59:25:62 | path | This path depends on a $@. | SanitizationTests2.java:23:62:24:139 | 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 +79,8 @@ | 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 | +| SanitizationTests2.java:23:62:24:139 | path : String | SanitizationTests2.java:25:59:25: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 +316,10 @@ 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 | +| SanitizationTests2.java:23:62:24:139 | path : String | semmle.label | path : String | +| SanitizationTests2.java:25:59:25: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 | diff --git a/java/ql/test/query-tests/security/CWE-022/semmle/tests/options b/java/ql/test/query-tests/security/CWE-022/semmle/tests/options index 547355c8e106..4d285fc6e927 100644 --- a/java/ql/test/query-tests/security/CWE-022/semmle/tests/options +++ b/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 From 76b4f4f22343747ebf934aafbf23d6f92c69c268 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 3 Jul 2026 11:08:54 +0100 Subject: [PATCH 2/5] Fix @Pattern sanitizer for TaintedPath --- .../semmle/code/java/security/PathSanitizer.qll | 17 +++++++++++++++-- .../semmle/tests/SanitizationTests2.java | 4 ++-- .../CWE-022/semmle/tests/TaintedPath.expected | 4 ---- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll index 1288569fa5bb..1b833b7d9160 100644 --- a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll +++ b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll @@ -470,7 +470,12 @@ private class DirectoryCharactersGuard extends PathGuard { Expr checkedExpr; boolean branch; - DirectoryCharactersGuard() { isMatchesCall(this, checkedExpr, branch) } + DirectoryCharactersGuard() { + // Annotations are handled directly as barriers in `DirectoryCharactersSanitizer`, + // since they don't dominate the sanitized expression and so can't act as barrier guards. + not this instanceof Annotation and + isMatchesCall(this, checkedExpr, branch) + } override Expr getCheckedExpr() { result = checkedExpr } @@ -493,7 +498,15 @@ private predicate directoryCharactersGuard(Guard g, Expr e, boolean branch) { */ private class DirectoryCharactersSanitizer extends PathInjectionSanitizer { DirectoryCharactersSanitizer() { - this.asExpr() instanceof ReplaceDirectoryCharactersSanitizer or + this.asExpr() instanceof ReplaceDirectoryCharactersSanitizer + or this = DataFlow::BarrierGuard::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 and isMatchesCall(rm, _, true) | + this.asExpr() = rm.getString() + ) } } diff --git a/java/ql/test/query-tests/security/CWE-022/semmle/tests/SanitizationTests2.java b/java/ql/test/query-tests/security/CWE-022/semmle/tests/SanitizationTests2.java index 3dcdfff8cf6f..19f4996f0cbe 100644 --- a/java/ql/test/query-tests/security/CWE-022/semmle/tests/SanitizationTests2.java +++ b/java/ql/test/query-tests/security/CWE-022/semmle/tests/SanitizationTests2.java @@ -21,8 +21,8 @@ public ResponseEntity vulnerablePathInjection(@RequestParam("path") Stri @GetMapping("/product/vuln/path-injection-fix") public ResponseEntity vulnerablePathInjectionFix(@RequestParam("path") - @javax.validation.constraints.Pattern(regexp = "[a-zA-Z0-9]*") String path) throws IOException { // $ SPURIOUS: Source - try (FileInputStream stream = new FileInputStream(path)) { // $ SPURIOUS: Alert + @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)); } diff --git a/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected b/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected index 297187e8ece8..bfeb4d730c25 100644 --- a/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected +++ b/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected @@ -1,6 +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 | -| SanitizationTests2.java:25:59:25:62 | path | SanitizationTests2.java:23:62:24:139 | path : String | SanitizationTests2.java:25:59:25:62 | path | This path depends on a $@. | SanitizationTests2.java:23:62:24:139 | 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 | @@ -80,7 +79,6 @@ | 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 | -| SanitizationTests2.java:23:62:24:139 | path : String | SanitizationTests2.java:25:59:25: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 | @@ -318,8 +316,6 @@ models nodes | SanitizationTests2.java:14:59:14:91 | path : String | semmle.label | path : String | | SanitizationTests2.java:16:59:16:62 | path | semmle.label | path | -| SanitizationTests2.java:23:62:24:139 | path : String | semmle.label | path : String | -| SanitizationTests2.java:25:59:25: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 | From e5bd62dbd3ff27d1feec5b05db8050d81f57a3ae Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 3 Jul 2026 11:13:58 +0100 Subject: [PATCH 3/5] Add change note --- .../2026-07-03-fix-pattern-sanitizer-tainted-path.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 java/ql/lib/change-notes/2026-07-03-fix-pattern-sanitizer-tainted-path.md diff --git a/java/ql/lib/change-notes/2026-07-03-fix-pattern-sanitizer-tainted-path.md b/java/ql/lib/change-notes/2026-07-03-fix-pattern-sanitizer-tainted-path.md new file mode 100644 index 000000000000..b04c65944459 --- /dev/null +++ b/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`. From 72d7fe4dbb0ea632467cb5dc4ec7a3761a390463 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 3 Jul 2026 21:03:10 +0100 Subject: [PATCH 4/5] Refactor creating barriers from RegexMatch It is better to have the logic in one place, to avoid encountering this bug again. --- java/ql/lib/semmle/code/java/Concepts.qll | 15 +++++- .../code/java/security/LogInjection.qll | 10 +--- .../code/java/security/PathSanitizer.qll | 17 ++----- .../semmle/code/java/security/Sanitizers.qll | 48 ++++++++++++++----- 4 files changed, 55 insertions(+), 35 deletions(-) diff --git a/java/ql/lib/semmle/code/java/Concepts.qll b/java/ql/lib/semmle/code/java/Concepts.qll index 7ed61223ea83..9331704e92c2 100644 --- a/java/ql/lib/semmle/code/java/Concepts.qll +++ b/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()] } /** diff --git a/java/ql/lib/semmle/code/java/security/LogInjection.qll b/java/ql/lib/semmle/code/java/security/LogInjection.qll index b585c249d1eb..db4ea864f8c8 100644 --- a/java/ql/lib/semmle/code/java/security/LogInjection.qll +++ b/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::getABarrierNode() + this = RegexMatchBarrier::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() | diff --git a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll index 1b833b7d9160..4bcc2e7f9412 100644 --- a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll +++ b/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 { } @@ -470,12 +471,7 @@ private class DirectoryCharactersGuard extends PathGuard { Expr checkedExpr; boolean branch; - DirectoryCharactersGuard() { - // Annotations are handled directly as barriers in `DirectoryCharactersSanitizer`, - // since they don't dominate the sanitized expression and so can't act as barrier guards. - not this instanceof Annotation and - isMatchesCall(this, checkedExpr, branch) - } + DirectoryCharactersGuard() { isMatchesCall(this, checkedExpr, branch) } override Expr getCheckedExpr() { result = checkedExpr } @@ -500,13 +496,6 @@ private class DirectoryCharactersSanitizer extends PathInjectionSanitizer { DirectoryCharactersSanitizer() { this.asExpr() instanceof ReplaceDirectoryCharactersSanitizer or - this = DataFlow::BarrierGuard::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 and isMatchesCall(rm, _, true) | - this.asExpr() = rm.getString() - ) + this = RegexMatchBarrier::getABarrierNode() } } diff --git a/java/ql/lib/semmle/code/java/security/Sanitizers.qll b/java/ql/lib/semmle/code/java/security/Sanitizers.qll index 0c5f9b98070a..528929019ec6 100644 --- a/java/ql/lib/semmle/code/java/security/Sanitizers.qll +++ b/java/ql/lib/semmle/code/java/security/Sanitizers.qll @@ -37,31 +37,57 @@ 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 ) } +/** A guard-check predicate over regular-expression matches, as 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 { + /** Gets a node that is sanitized by a regular-expression match. */ + DataFlow::Node getABarrierNode() { + result = DataFlow::BarrierGuard::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::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::getABarrierNode() } } /** From 5b2e0b6804cdbfc8cbd8405e0612d0f4d8942b9e Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 3 Jul 2026 21:20:19 +0100 Subject: [PATCH 5/5] Fix QLDoc --- java/ql/lib/semmle/code/java/security/Sanitizers.qll | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/security/Sanitizers.qll b/java/ql/lib/semmle/code/java/security/Sanitizers.qll index 528929019ec6..cf2ecd342a8d 100644 --- a/java/ql/lib/semmle/code/java/security/Sanitizers.qll +++ b/java/ql/lib/semmle/code/java/security/Sanitizers.qll @@ -51,7 +51,11 @@ predicate regexpMatchGuardChecks(Guard guard, Expr e, boolean branch) { ) } -/** A guard-check predicate over regular-expression matches, as used by `RegexMatchBarrier`. */ +/** + * 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); /**