From 687567064ecee52e33485debf897497be23c6855 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 19 Dec 2025 08:13:19 +0000 Subject: [PATCH 1/3] Accept MaD sanitizers for existing sink kinds --- .../code/java/security/AndroidIntentRedirection.qll | 5 +++++ .../lib/semmle/code/java/security/CommandLineQuery.qll | 4 ++++ .../lib/semmle/code/java/security/FragmentInjection.qll | 9 +++++++++ .../semmle/code/java/security/FragmentInjectionQuery.qll | 2 ++ .../ql/lib/semmle/code/java/security/GroovyInjection.qll | 7 +++++++ .../semmle/code/java/security/GroovyInjectionQuery.qll | 2 ++ 6 files changed, 29 insertions(+) diff --git a/java/ql/lib/semmle/code/java/security/AndroidIntentRedirection.qll b/java/ql/lib/semmle/code/java/security/AndroidIntentRedirection.qll index 08a86092afbb..57dfcd29117f 100644 --- a/java/ql/lib/semmle/code/java/security/AndroidIntentRedirection.qll +++ b/java/ql/lib/semmle/code/java/security/AndroidIntentRedirection.qll @@ -35,6 +35,11 @@ private class DefaultIntentRedirectionSink extends IntentRedirectionSink { DefaultIntentRedirectionSink() { sinkNode(this, "intent-redirection") } } +/** External sanitizers for Intent redirection vulnerabilities. */ +private class ExternalIntentRedirectionSanitizer extends IntentRedirectionSanitizer { + ExternalIntentRedirectionSanitizer() { barrierNode(this, "intent-redirection") } +} + /** * A default sanitizer for `Intent` nodes dominated by calls to `ComponentName.getPackageName` * and `ComponentName.getClassName`. These are used to check whether the origin or destination diff --git a/java/ql/lib/semmle/code/java/security/CommandLineQuery.qll b/java/ql/lib/semmle/code/java/security/CommandLineQuery.qll index b6b9d02e289d..273c5360b815 100644 --- a/java/ql/lib/semmle/code/java/security/CommandLineQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CommandLineQuery.qll @@ -37,6 +37,10 @@ private class DefaultCommandInjectionSink extends CommandInjectionSink { DefaultCommandInjectionSink() { sinkNode(this, "command-injection") } } +private class ExternalCommandInjectionSanitizer extends CommandInjectionSanitizer { + ExternalCommandInjectionSanitizer() { barrierNode(this, "command-injection") } +} + private class DefaultCommandInjectionSanitizer extends CommandInjectionSanitizer { DefaultCommandInjectionSanitizer() { this instanceof SimpleTypeSanitizer diff --git a/java/ql/lib/semmle/code/java/security/FragmentInjection.qll b/java/ql/lib/semmle/code/java/security/FragmentInjection.qll index 8cd5e32a5ecd..6a01b2c8b18e 100644 --- a/java/ql/lib/semmle/code/java/security/FragmentInjection.qll +++ b/java/ql/lib/semmle/code/java/security/FragmentInjection.qll @@ -49,6 +49,15 @@ private class DefaultFragmentInjectionSink extends FragmentInjectionSink { DefaultFragmentInjectionSink() { sinkNode(this, "fragment-injection") } } +/** + * A barrier for Fragment injection vulnerabilities. + */ +abstract class FragmentInjectionSanitizer extends DataFlow::Node { } + +private class ExternalFragmentInjectionSanitizer extends FragmentInjectionSanitizer { + ExternalFragmentInjectionSanitizer() { barrierNode(this, "fragment-injection") } +} + private class DefaultFragmentInjectionAdditionalTaintStep extends FragmentInjectionAdditionalTaintStep { override predicate step(DataFlow::Node n1, DataFlow::Node n2) { diff --git a/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll b/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll index 40636ffd8c25..1cb9f711b6fa 100644 --- a/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll +++ b/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll @@ -14,6 +14,8 @@ module FragmentInjectionTaintConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { sink instanceof FragmentInjectionSink } + predicate isBarrier(DataFlow::Node node) { node instanceof FragmentInjectionSanitizer } + predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { any(FragmentInjectionAdditionalTaintStep c).step(n1, n2) } diff --git a/java/ql/lib/semmle/code/java/security/GroovyInjection.qll b/java/ql/lib/semmle/code/java/security/GroovyInjection.qll index 45d664897775..d9a5db7b12d3 100644 --- a/java/ql/lib/semmle/code/java/security/GroovyInjection.qll +++ b/java/ql/lib/semmle/code/java/security/GroovyInjection.qll @@ -26,6 +26,13 @@ private class DefaultGroovyInjectionSink extends GroovyInjectionSink { DefaultGroovyInjectionSink() { sinkNode(this, "groovy-injection") } } +/** A data flow sanitizer for Groovy expression injection vulnerabilities. */ +abstract class GroovyInjectionSanitizer extends DataFlow::ExprNode { } + +private class ExternalGroovyInjectionSanitizer extends GroovyInjectionSanitizer { + ExternalGroovyInjectionSanitizer() { barrierNode(this, "groovy-injection") } +} + /** A set of additional taint steps to consider when taint tracking Groovy related data flows. */ private class DefaultGroovyInjectionAdditionalTaintStep extends GroovyInjectionAdditionalTaintStep { override predicate step(DataFlow::Node node1, DataFlow::Node node2) { diff --git a/java/ql/lib/semmle/code/java/security/GroovyInjectionQuery.qll b/java/ql/lib/semmle/code/java/security/GroovyInjectionQuery.qll index b497873b9bb1..af4645b15917 100644 --- a/java/ql/lib/semmle/code/java/security/GroovyInjectionQuery.qll +++ b/java/ql/lib/semmle/code/java/security/GroovyInjectionQuery.qll @@ -14,6 +14,8 @@ module GroovyInjectionConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { sink instanceof GroovyInjectionSink } + predicate isBarrier(DataFlow::Node node) { node instanceof GroovyInjectionSanitizer } + predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) { any(GroovyInjectionAdditionalTaintStep c).step(fromNode, toNode) } From de69d94dbfd3e60a4e8a76a7dc74c9afa623724f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 7 Jan 2026 11:33:40 +0000 Subject: [PATCH 2/3] Initial plan From 6574f1761542cd787d6b56d8eedadaa2c7a63fe6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 7 Jan 2026 11:41:05 +0000 Subject: [PATCH 3/3] Refactor regex-use sink kind to support polynomial ReDoS query - Extended regexSinkKindInfo to handle plain "regex-use" sink kind - Plain "regex-use" defaults to non-full-string matching with string arg at index 0 - Updated comment in org.apache.commons.lang3.model.yml - Added test cases for RegExUtils methods in polynomial ReDoS test Co-authored-by: owen-mc <62447351+owen-mc@users.noreply.github.com> --- .../ext/org.apache.commons.lang3.model.yml | 6 +-- .../code/java/regex/RegexFlowConfigs.qll | 41 ++++++++++--------- .../CWE-730/PolyRedos/PolyRedosTest.java | 13 ++++++ 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/java/ql/lib/ext/org.apache.commons.lang3.model.yml b/java/ql/lib/ext/org.apache.commons.lang3.model.yml index 7c455d780b13..2f039bf69944 100644 --- a/java/ql/lib/ext/org.apache.commons.lang3.model.yml +++ b/java/ql/lib/ext/org.apache.commons.lang3.model.yml @@ -4,9 +4,9 @@ extensions: extensible: sinkModel data: - ["org.apache.commons.lang3", "SerializationUtils", False, "deserialize", "", "", "Argument[0]", "unsafe-deserialization", "manual"] - # Note these sinks do not use the sink kind `regex-use[0]` because the regex injection query needs to select them separately from - # other `regex-use[0]` sinks in order to avoid FPs. As a result, these sinks are currently not used in the polynomial ReDoS query. - # TODO: refactor the `regex-use%` sink kind so that the polynomial ReDoS query can also use these sinks. + # Note these sinks use the sink kind `regex-use` (without brackets) because the regex injection query needs to select them + # separately from other `regex-use[0]` sinks in order to avoid FPs. The polynomial ReDoS query handles the plain `regex-use` + # sink kind by treating it as `regex-use[0]` (non-full-string matching with the string argument at index 0). - ["org.apache.commons.lang3", "RegExUtils", False, "removeAll", "(String,String)", "", "Argument[1]", "regex-use", "manual"] - ["org.apache.commons.lang3", "RegExUtils", False, "removeFirst", "(String,String)", "", "Argument[1]", "regex-use", "manual"] - ["org.apache.commons.lang3", "RegExUtils", False, "removePattern", "(String,String)", "", "Argument[1]", "regex-use", "manual"] diff --git a/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll b/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll index 4deb2bc812bb..4d09a54c92f6 100644 --- a/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll +++ b/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll @@ -20,29 +20,32 @@ private class ExploitableStringLiteral extends StringLiteral { * `strArg` is the index of the argument to methods with this sink kind that * contain the string to be matched against, where -1 is the qualifier; or -2 * if no such argument exists. - * - * Note that `regex-use` is deliberately not a possible value for `kind` here, - * as it is used for regular expression injection sinks that need to be selected - * separately from existing `regex-use[0]` sinks. - * TODO: refactor the `regex-use%` sink kind so that the polynomial ReDoS query - * can also use the `regex-use` sinks. */ private predicate regexSinkKindInfo(string kind, boolean full, int strArg) { sinkModel(_, _, _, _, _, _, _, kind, _, _) and - exists(string fullStr, string strArgStr | - ( - full = true and fullStr = "f" - or - full = false and fullStr = "" - ) and - ( - strArgStr.toInt() = strArg - or - strArg = -2 and - strArgStr = "" + ( + exists(string fullStr, string strArgStr | + ( + full = true and fullStr = "f" + or + full = false and fullStr = "" + ) and + ( + strArgStr.toInt() = strArg + or + strArg = -2 and + strArgStr = "" + ) + | + kind = "regex-use[" + fullStr + strArgStr + "]" ) - | - kind = "regex-use[" + fullStr + strArgStr + "]" + or + // Plain `regex-use` sinks default to non-full-string matching with the string argument at index 0. + // This is primarily used for methods like `RegExUtils.removeAll(text, regex)` where both the + // pattern and the input are provided in the same call. + kind = "regex-use" and + full = false and + strArg = 0 ) } diff --git a/java/ql/test/query-tests/security/CWE-730/PolyRedos/PolyRedosTest.java b/java/ql/test/query-tests/security/CWE-730/PolyRedos/PolyRedosTest.java index 34e527456c53..d8c467d3d6f2 100644 --- a/java/ql/test/query-tests/security/CWE-730/PolyRedos/PolyRedosTest.java +++ b/java/ql/test/query-tests/security/CWE-730/PolyRedos/PolyRedosTest.java @@ -2,6 +2,7 @@ import java.util.function.Predicate; import javax.servlet.http.HttpServletRequest; import com.google.common.base.Splitter; +import org.apache.commons.lang3.RegExUtils; class PolyRedosTest { void test(HttpServletRequest request) { @@ -81,4 +82,16 @@ void test6(HttpServletRequest request) { p.matcher(request.getRequestURI()).matches(); p.matcher(request.getCookies()[0].getName()).matches(); } + + void test7(HttpServletRequest request) { + String tainted = request.getParameter("inp"); // $ Source + String reg = "0\\.\\d+E?\\d+!"; + + RegExUtils.removeAll(tainted, reg); // $ Alert + RegExUtils.removeFirst(tainted, reg); // $ Alert + RegExUtils.removePattern(tainted, reg); // $ Alert + RegExUtils.replaceAll(tainted, reg, "replacement"); // $ Alert + RegExUtils.replaceFirst(tainted, reg, "replacement"); // $ Alert + RegExUtils.replacePattern(tainted, reg, "replacement"); // $ Alert + } }