Skip to content

Commit 32f21d6

Browse files
authored
Merge pull request #20688 from hvitved/java/request-forgery-matches-sanitizer
Java: Treat `x.matches(regexp)` as a sanitizer for request forgery
2 parents 8c277bd + a4eab48 commit 32f21d6

File tree

4 files changed

+44
-1
lines changed

4 files changed

+44
-1
lines changed

‎java/ql/lib/semmle/code/java/security/RequestForgery.qll‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,3 +164,24 @@ private class HostComparisonSanitizer extends RequestForgerySanitizer {
164164
this = DataFlow::BarrierGuard<isHostComparisonSanitizer/3>::getABarrierNode()
165165
}
166166
}
167+
168+
/**
169+
* A qualifier in a call to a `.matches()` method that is a sanitizer for URL redirects.
170+
*
171+
* Matches any method call where the method is named `matches`.
172+
*/
173+
private predicate isMatchesSanitizer(Guard guard, Expr e, boolean branch) {
174+
guard =
175+
any(MethodCall method |
176+
method.getMethod().getName() = "matches" and
177+
e = method.getQualifier() and
178+
branch = true
179+
)
180+
}
181+
182+
/**
183+
* A qualifier in a call to `.matches()` that is a sanitizer for URL redirects.
184+
*/
185+
private class MatchesSanitizer extends RequestForgerySanitizer {
186+
MatchesSanitizer() { this = DataFlow::BarrierGuard<isMatchesSanitizer/3>::getABarrierNode() }
187+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Calls to `String.matches` are now treated as sanitizers for the `java/ssrf` query.

‎java/ql/test/query-tests/security/CWE-918/SanitizationTests.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,26 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
119119
String unsafeUri10 = String.format("%s://%s:%s%s", "http", "myserver.com", "80", request.getParameter("baduri10")); // $ Source
120120
HttpRequest unsafer10 = HttpRequest.newBuilder(new URI(unsafeUri10)).build(); // $ Alert
121121
client.send(unsafer10, null); // $ Alert
122+
123+
// GOOD: sanitisation by regexp validation
124+
String param10 = request.getParameter("uri10");
125+
if (param10.matches("[a-zA-Z0-9_-]+")) {
126+
HttpRequest r10 = HttpRequest.newBuilder(new URI(param10)).build();
127+
client.send(r10, null);
128+
}
129+
130+
String param11 = request.getParameter("uri11");
131+
validate(param11);
132+
HttpRequest r11 = HttpRequest.newBuilder(new URI(param11)).build();
133+
client.send(r11, null);
122134
} catch (Exception e) {
123135
// TODO: handle exception
124136
}
125137
}
138+
139+
private void validate(String s) {
140+
if (!s.matches("[a-zA-Z0-9_-]+")) {
141+
throw new IllegalArgumentException("Invalid ID");
142+
}
143+
}
126144
}

‎misc/scripts/create-change-note.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
# - What language the change note is for
88
# - Whether it's a query or library change (the string `src` or `lib`)
99
# - The name of the change note (in kebab-case)
10-
# - The category of the change.
10+
# - The category of the change (see https://github.com/github/codeql/blob/main/docs/change-notes.md#change-categories).
1111

1212
# The change note will be created in the `{language}/ql/{subdir}/change-notes` directory, where `subdir` is either `src` or `lib`.
1313

0 commit comments

Comments
 (0)