diff --git a/java/ql/lib/semmle/code/java/frameworks/Regex.qll b/java/ql/lib/semmle/code/java/frameworks/Regex.qll index f63f46c3878..56be77eae82 100644 --- a/java/ql/lib/semmle/code/java/frameworks/Regex.qll +++ b/java/ql/lib/semmle/code/java/frameworks/Regex.qll @@ -4,11 +4,46 @@ module; import java +/** The class `java.util.regex.Matcher`. */ +class TypeRegexMatcher extends Class { + TypeRegexMatcher() { this.hasQualifiedName("java.util.regex", "Matcher") } +} + +/** + * The `matches` method of `java.util.regex.Matcher`. + */ +class MatcherMatchesMethod extends Method { + MatcherMatchesMethod() { + this.getDeclaringType() instanceof TypeRegexMatcher and + this.hasName("matches") + } +} + /** The class `java.util.regex.Pattern`. */ class TypeRegexPattern extends Class { TypeRegexPattern() { this.hasQualifiedName("java.util.regex", "Pattern") } } +/** + * The `matches` method of `java.util.regex.Pattern`. + */ +class PatternMatchesMethod extends Method { + PatternMatchesMethod() { + this.getDeclaringType() instanceof TypeRegexPattern and + this.hasName("matches") + } +} + +/** + * The `matcher` method of `java.util.regex.Pattern`. + */ +class PatternMatcherMethod extends Method { + PatternMatcherMethod() { + this.getDeclaringType() instanceof TypeRegexPattern and + this.hasName("matcher") + } +} + /** The `quote` method of the `java.util.regex.Pattern` class. */ class PatternQuoteMethod extends Method { PatternQuoteMethod() { diff --git a/java/ql/lib/semmle/code/java/security/Sanitizers.qll b/java/ql/lib/semmle/code/java/security/Sanitizers.qll index 3dac78cf1e0..3f909864d2c 100644 --- a/java/ql/lib/semmle/code/java/security/Sanitizers.qll +++ b/java/ql/lib/semmle/code/java/security/Sanitizers.qll @@ -5,6 +5,7 @@ module; import java private import semmle.code.java.controlflow.Guards private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.frameworks.Regex /** * A node whose type is a simple type unlikely to carry taint, such as primitives and their boxed counterparts, @@ -40,12 +41,25 @@ class SimpleTypeSanitizer extends DataFlow::Node { * make the type recursive. Otherwise use `RegexpCheckBarrier`. */ predicate regexpMatchGuardChecks(Guard guard, Expr e, boolean branch) { - guard = - any(MethodCall method | - method.getMethod().getName() = "matches" and - e = method.getQualifier() and - branch = true + exists(Method method, MethodCall mc | + method = mc.getMethod() and + guard = mc and + branch = true + | + // `String.matches` and other `matches` methods. + method.getName() = "matches" and + e = mc.getQualifier() + or + method instanceof PatternMatchesMethod and + e = mc.getArgument(1) + or + method instanceof MatcherMatchesMethod and + exists(MethodCall matcherCall | + matcherCall.getMethod() instanceof PatternMatcherMethod and + e = matcherCall.getArgument(0) and + DataFlow::localExprFlow(matcherCall, mc.getQualifier()) ) + ) } /** diff --git a/java/ql/test/query-tests/security/CWE-918/RequestForgery.expected b/java/ql/test/query-tests/security/CWE-918/RequestForgery.expected index 555279c2a96..66dc968cb23 100644 --- a/java/ql/test/query-tests/security/CWE-918/RequestForgery.expected +++ b/java/ql/test/query-tests/security/CWE-918/RequestForgery.expected @@ -252,10 +252,6 @@ | SanitizationTests.java:119:25:119:32 | unsafer9 | SanitizationTests.java:117:33:117:63 | getParameter(...) : String | SanitizationTests.java:119:25:119:32 | unsafer9 | Potential server-side request forgery due to a $@. | SanitizationTests.java:117:33:117:63 | getParameter(...) | user-provided value | | SanitizationTests.java:122:60:122:79 | new URI(...) | SanitizationTests.java:121:94:121:125 | getParameter(...) : String | SanitizationTests.java:122:60:122:79 | new URI(...) | Potential server-side request forgery due to a $@. | SanitizationTests.java:121:94:121:125 | getParameter(...) | user-provided value | | SanitizationTests.java:123:25:123:33 | unsafer10 | SanitizationTests.java:121:94:121:125 | getParameter(...) : String | SanitizationTests.java:123:25:123:33 | unsafer10 | Potential server-side request forgery due to a $@. | SanitizationTests.java:121:94:121:125 | getParameter(...) | user-provided value | -| SanitizationTests.java:139:58:139:73 | new URI(...) | SanitizationTests.java:137:30:137:58 | getParameter(...) : String | SanitizationTests.java:139:58:139:73 | new URI(...) | Potential server-side request forgery due to a $@. | SanitizationTests.java:137:30:137:58 | getParameter(...) | user-provided value | -| SanitizationTests.java:140:29:140:31 | r12 | SanitizationTests.java:137:30:137:58 | getParameter(...) : String | SanitizationTests.java:140:29:140:31 | r12 | Potential server-side request forgery due to a $@. | SanitizationTests.java:137:30:137:58 | getParameter(...) | user-provided value | -| SanitizationTests.java:147:58:147:73 | new URI(...) | SanitizationTests.java:144:30:144:58 | getParameter(...) : String | SanitizationTests.java:147:58:147:73 | new URI(...) | Potential server-side request forgery due to a $@. | SanitizationTests.java:144:30:144:58 | getParameter(...) | user-provided value | -| SanitizationTests.java:148:29:148:31 | r13 | SanitizationTests.java:144:30:144:58 | getParameter(...) : String | SanitizationTests.java:148:29:148:31 | r13 | Potential server-side request forgery due to a $@. | SanitizationTests.java:144:30:144:58 | getParameter(...) | user-provided value | | SpringSSRF.java:32:39:32:59 | ... + ... | SpringSSRF.java:28:33:28:60 | getParameter(...) : String | SpringSSRF.java:32:39:32:59 | ... + ... | Potential server-side request forgery due to a $@. | SpringSSRF.java:28:33:28:60 | getParameter(...) | user-provided value | | SpringSSRF.java:33:35:33:48 | fooResourceUrl | SpringSSRF.java:28:33:28:60 | getParameter(...) : String | SpringSSRF.java:33:35:33:48 | fooResourceUrl | Potential server-side request forgery due to a $@. | SpringSSRF.java:28:33:28:60 | getParameter(...) | user-provided value | | SpringSSRF.java:34:34:34:47 | fooResourceUrl | SpringSSRF.java:28:33:28:60 | getParameter(...) : String | SpringSSRF.java:34:34:34:47 | fooResourceUrl | Potential server-side request forgery due to a $@. | SpringSSRF.java:28:33:28:60 | getParameter(...) | user-provided value | @@ -781,22 +777,6 @@ edges | SanitizationTests.java:122:68:122:78 | unsafeUri10 : String | SanitizationTests.java:122:60:122:79 | new URI(...) | provenance | MaD:285 Sink:MaD:6 | | SanitizationTests.java:122:68:122:78 | unsafeUri10 : String | SanitizationTests.java:122:60:122:79 | new URI(...) : URI | provenance | Config | | SanitizationTests.java:122:68:122:78 | unsafeUri10 : String | SanitizationTests.java:122:60:122:79 | new URI(...) : URI | provenance | MaD:285 | -| SanitizationTests.java:137:30:137:58 | getParameter(...) : String | SanitizationTests.java:139:66:139:72 | param12 : String | provenance | Src:MaD:277 | -| SanitizationTests.java:139:35:139:74 | newBuilder(...) : Builder | SanitizationTests.java:139:35:139:82 | build(...) : HttpRequest | provenance | MaD:283 | -| SanitizationTests.java:139:35:139:82 | build(...) : HttpRequest | SanitizationTests.java:140:29:140:31 | r12 | provenance | Sink:MaD:4 | -| SanitizationTests.java:139:58:139:73 | new URI(...) : URI | SanitizationTests.java:139:35:139:74 | newBuilder(...) : Builder | provenance | MaD:284 | -| SanitizationTests.java:139:66:139:72 | param12 : String | SanitizationTests.java:139:58:139:73 | new URI(...) | provenance | Config Sink:MaD:6 | -| SanitizationTests.java:139:66:139:72 | param12 : String | SanitizationTests.java:139:58:139:73 | new URI(...) | provenance | MaD:285 Sink:MaD:6 | -| SanitizationTests.java:139:66:139:72 | param12 : String | SanitizationTests.java:139:58:139:73 | new URI(...) : URI | provenance | Config | -| SanitizationTests.java:139:66:139:72 | param12 : String | SanitizationTests.java:139:58:139:73 | new URI(...) : URI | provenance | MaD:285 | -| SanitizationTests.java:144:30:144:58 | getParameter(...) : String | SanitizationTests.java:147:66:147:72 | param13 : String | provenance | Src:MaD:277 | -| SanitizationTests.java:147:35:147:74 | newBuilder(...) : Builder | SanitizationTests.java:147:35:147:82 | build(...) : HttpRequest | provenance | MaD:283 | -| SanitizationTests.java:147:35:147:82 | build(...) : HttpRequest | SanitizationTests.java:148:29:148:31 | r13 | provenance | Sink:MaD:4 | -| SanitizationTests.java:147:58:147:73 | new URI(...) : URI | SanitizationTests.java:147:35:147:74 | newBuilder(...) : Builder | provenance | MaD:284 | -| SanitizationTests.java:147:66:147:72 | param13 : String | SanitizationTests.java:147:58:147:73 | new URI(...) | provenance | Config Sink:MaD:6 | -| SanitizationTests.java:147:66:147:72 | param13 : String | SanitizationTests.java:147:58:147:73 | new URI(...) | provenance | MaD:285 Sink:MaD:6 | -| SanitizationTests.java:147:66:147:72 | param13 : String | SanitizationTests.java:147:58:147:73 | new URI(...) : URI | provenance | Config | -| SanitizationTests.java:147:66:147:72 | param13 : String | SanitizationTests.java:147:58:147:73 | new URI(...) : URI | provenance | MaD:285 | | SpringSSRF.java:28:33:28:60 | getParameter(...) : String | SpringSSRF.java:32:39:32:59 | ... + ... | provenance | Src:MaD:277 Sink:MaD:264 | | SpringSSRF.java:28:33:28:60 | getParameter(...) : String | SpringSSRF.java:33:35:33:48 | fooResourceUrl | provenance | Src:MaD:277 Sink:MaD:262 | | SpringSSRF.java:28:33:28:60 | getParameter(...) : String | SpringSSRF.java:34:34:34:47 | fooResourceUrl | provenance | Src:MaD:277 Sink:MaD:263 | @@ -1701,20 +1681,6 @@ nodes | SanitizationTests.java:122:60:122:79 | new URI(...) : URI | semmle.label | new URI(...) : URI | | SanitizationTests.java:122:68:122:78 | unsafeUri10 : String | semmle.label | unsafeUri10 : String | | SanitizationTests.java:123:25:123:33 | unsafer10 | semmle.label | unsafer10 | -| SanitizationTests.java:137:30:137:58 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| SanitizationTests.java:139:35:139:74 | newBuilder(...) : Builder | semmle.label | newBuilder(...) : Builder | -| SanitizationTests.java:139:35:139:82 | build(...) : HttpRequest | semmle.label | build(...) : HttpRequest | -| SanitizationTests.java:139:58:139:73 | new URI(...) | semmle.label | new URI(...) | -| SanitizationTests.java:139:58:139:73 | new URI(...) : URI | semmle.label | new URI(...) : URI | -| SanitizationTests.java:139:66:139:72 | param12 : String | semmle.label | param12 : String | -| SanitizationTests.java:140:29:140:31 | r12 | semmle.label | r12 | -| SanitizationTests.java:144:30:144:58 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| SanitizationTests.java:147:35:147:74 | newBuilder(...) : Builder | semmle.label | newBuilder(...) : Builder | -| SanitizationTests.java:147:35:147:82 | build(...) : HttpRequest | semmle.label | build(...) : HttpRequest | -| SanitizationTests.java:147:58:147:73 | new URI(...) | semmle.label | new URI(...) | -| SanitizationTests.java:147:58:147:73 | new URI(...) : URI | semmle.label | new URI(...) : URI | -| SanitizationTests.java:147:66:147:72 | param13 : String | semmle.label | param13 : String | -| SanitizationTests.java:148:29:148:31 | r13 | semmle.label | r13 | | SpringSSRF.java:28:33:28:60 | getParameter(...) : String | semmle.label | getParameter(...) : String | | SpringSSRF.java:32:39:32:59 | ... + ... | semmle.label | ... + ... | | SpringSSRF.java:33:35:33:48 | fooResourceUrl | semmle.label | fooResourceUrl | @@ -1857,10 +1823,3 @@ nodes | mad/Test.java:112:15:112:31 | (...)... | semmle.label | (...)... | | mad/Test.java:112:24:112:31 | source(...) : String | semmle.label | source(...) : String | subpaths -testFailures -| SanitizationTests.java:137:30:137:58 | getParameter(...) : String | Unexpected result: Source | -| SanitizationTests.java:139:58:139:73 | new URI(...) | Unexpected result: Alert | -| SanitizationTests.java:140:29:140:31 | r12 | Unexpected result: Alert | -| SanitizationTests.java:144:30:144:58 | getParameter(...) : String | Unexpected result: Source | -| SanitizationTests.java:147:58:147:73 | new URI(...) | Unexpected result: Alert | -| SanitizationTests.java:148:29:148:31 | r13 | Unexpected result: Alert |