Java: Add and use RegexExecution concept #21310
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared “RegexExecution” concept for Java CodeQL libraries and migrates existing regex-related models/queries to use it, aiming to unify regex execution modeling across languages and reduce false positives.
Changes:
- Added
semmle.code.java.ConceptswithRegexExecution/RegexExecutionExprconcepts and wired it intojava.qll. - Extended existing Java regex API models (
String.matches,Pattern.compile,Pattern.matches,Matcher.matches) to implement the new concept. - Updated experimental and security libraries to use the new concept types instead of bespoke regex helpers.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| java/ql/src/experimental/Security/CWE/CWE-625/Regex.qll | Removes the (deprecated) local regex helper module. |
| java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegexQuery.qll | Switches to framework regex models/concepts for sinks and matching logic. |
| java/ql/lib/semmle/code/java/security/regexp/RegexInjection.qll | Updates sanitizer modeling to use the new PatternCompileCall abstraction. |
| java/ql/lib/semmle/code/java/security/Sanitizers.qll | Reworks regexp-guard matching to use RegexExecutionExpr::Range. |
| java/ql/lib/semmle/code/java/security/PathSanitizer.qll | Updates directory-character matching guard logic to use the new regex execution concept. |
| java/ql/lib/semmle/code/java/frameworks/Regex.qll | Adds call/model classes (e.g., PatternCompileCall, PatternMatchesCall, MatcherMatchesCall) implementing RegexExecutionExpr::Range. |
| java/ql/lib/semmle/code/java/JDK.qll | Updates StringMatchesCall to implement RegexExecutionExpr::Range. |
| java/ql/lib/semmle/code/java/Concepts.qll | New Java Concepts library including RegexExecution and related modeling modules. |
| java/ql/lib/java.qll | Exports the new Concepts library by importing it. |
Comments suppressed due to low confidence (1)
java/ql/lib/semmle/code/java/frameworks/Regex.qll:96
- Doc comment punctuation: this comment is missing a trailing period, unlike most other doc comments in this file.
/** A call to the `matches` method of `java.util.regex.Matcher` */
class MatcherMatchesCall extends MethodCall, RegexExecutionExpr::Range {
| /** Gets the expression for the regex being executed by this node. */ | ||
| abstract Expr getRegex(); | ||
|
|
||
| /** Gets a expression for the string to be searched or matched against. */ |
There was a problem hiding this comment.
Grammar: "Gets a expression" should be "Gets an expression".
Suggested change
| /** Gets a expression for the string to be searched or matched against. */ | |
| /** Gets an expression for the string to be searched or matched against. */ |
Comment on lines
+74
to
+85
| /** A call to the `compile` method of `java.util.regex.Pattern` */ | ||
| class PatternCompileCall extends MethodCall { | ||
| PatternCompileCall() { this.getMethod() instanceof PatternCompileMethod } | ||
| } | ||
|
|
||
| /** A call to the `matcher` method of `java.util.regex.Pattern` */ | ||
| class PatternMatcherCall extends MethodCall { | ||
| PatternMatcherCall() { this.getMethod() instanceof PatternMatcherMethod } | ||
| } | ||
|
|
||
| /** A call to the `matches` method of `java.util.regex.Pattern` */ | ||
| class PatternMatchesCall extends MethodCall, RegexExecutionExpr::Range { |
There was a problem hiding this comment.
These doc comments are missing trailing periods, while other doc comments in this file include them. Consider adding periods for consistency.
This issue also appears on line 95 of the same file.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I expect some reduction in FPs due to the final commit expanded the scope of some sanitizers to include more regex execution methods. However, I don't want to run DCA/QA/MRVA to quantify that yet because I am doing some follow-up work to add the
@javax.validation.constraints.Patternannotation as well.I welcome discussion about how to best deal with the fact that I want the concept to be the same as other languages, which use data flow nodes, but actually java mostly works at the levels of expressions. I've kind of done both for now. I think as we spread the use of shared libraries this difference in approach will eventually go away, or at least become easier to fix.
Note: currently lacking a change note. If I finish the follow-up work quickly I may end up doing a combined one in that PR, which will build on this one. But it is still worth reviewing this PR as it breaks the review work up into smaller units.