From 7a9cb64e2e898ccf0d94562cb3e40cc0dbcfeb4c Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 24 Oct 2025 09:06:57 +0200 Subject: [PATCH 1/3] Java: Treat `x.matches(regexp)` as a sanitizer for request forgery --- .../code/java/security/RequestForgery.qll | 21 ++++++++++++++++++ .../security/CWE-918/SanitizationTests.java | 22 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/java/ql/lib/semmle/code/java/security/RequestForgery.qll b/java/ql/lib/semmle/code/java/security/RequestForgery.qll index 5eb35c05cd47..dc15ad943bc7 100644 --- a/java/ql/lib/semmle/code/java/security/RequestForgery.qll +++ b/java/ql/lib/semmle/code/java/security/RequestForgery.qll @@ -164,3 +164,24 @@ private class HostComparisonSanitizer extends RequestForgerySanitizer { this = DataFlow::BarrierGuard::getABarrierNode() } } + +/** + * A qualifier in a call to a `.matches()` method that is a sanitizer for URL redirects. + * + * Matches any method call where the method is named `matches`. + */ +private predicate isMatchesSanitizer(Guard guard, Expr e, boolean branch) { + guard = + any(MethodCall method | + method.getMethod().getName() = "matches" and + e = method.getQualifier() and + branch = true + ) +} + +/** + * A qualifier in a call to `.matches()` that is a sanitizer for URL redirects. + */ +private class MatchesSanitizer extends RequestForgerySanitizer { + MatchesSanitizer() { this = DataFlow::BarrierGuard::getABarrierNode() } +} diff --git a/java/ql/test/query-tests/security/CWE-918/SanitizationTests.java b/java/ql/test/query-tests/security/CWE-918/SanitizationTests.java index 03a61cfcf97d..f7e46b62946a 100644 --- a/java/ql/test/query-tests/security/CWE-918/SanitizationTests.java +++ b/java/ql/test/query-tests/security/CWE-918/SanitizationTests.java @@ -119,8 +119,30 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) String unsafeUri10 = String.format("%s://%s:%s%s", "http", "myserver.com", "80", request.getParameter("baduri10")); // $ Source HttpRequest unsafer10 = HttpRequest.newBuilder(new URI(unsafeUri10)).build(); // $ Alert client.send(unsafer10, null); // $ Alert + + // GOOD: sanitisation by regexp validation + String safeUri10 = "https://example.com/"; + String param10 = request.getParameter("uri10"); + if (param10.matches("[a-zA-Z0-9/_-]+")) { + safeUri10 = safeUri10 + param10; + } + HttpRequest r10 = HttpRequest.newBuilder(new URI(safeUri10)).build(); + client.send(r10, null); + + + String param11 = request.getParameter("uri11"); + validate(param11); + String safeUri11 = "https://example.com/" + param11; + HttpRequest r11 = HttpRequest.newBuilder(new URI(safeUri11)).build(); + client.send(r11, null); } catch (Exception e) { // TODO: handle exception } } + + private void validate(String s) { + if (!s.matches("[a-zA-Z0-9/_-]+")) { + throw new IllegalArgumentException("Invalid ID"); + } + } } From ce379161fc4d8718842d45ab990905a7d2f7b8b9 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 24 Oct 2025 09:34:11 +0200 Subject: [PATCH 2/3] Add change note --- .../2025-10-24-request-forgery-matches-sanitizer.md | 4 ++++ misc/scripts/create-change-note.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 java/ql/src/change-notes/2025-10-24-request-forgery-matches-sanitizer.md diff --git a/java/ql/src/change-notes/2025-10-24-request-forgery-matches-sanitizer.md b/java/ql/src/change-notes/2025-10-24-request-forgery-matches-sanitizer.md new file mode 100644 index 000000000000..a38c43dd7305 --- /dev/null +++ b/java/ql/src/change-notes/2025-10-24-request-forgery-matches-sanitizer.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Calls to `String.matches` are now treated as sanitizers for the `java/ssrf` query. \ No newline at end of file diff --git a/misc/scripts/create-change-note.py b/misc/scripts/create-change-note.py index 1a13a0cf1148..1de42126c90c 100755 --- a/misc/scripts/create-change-note.py +++ b/misc/scripts/create-change-note.py @@ -7,7 +7,7 @@ # - What language the change note is for # - Whether it's a query or library change (the string `src` or `lib`) # - The name of the change note (in kebab-case) -# - The category of the change. +# - The category of the change (see https://github.com/github/codeql/blob/main/docs/change-notes.md#change-categories). # The change note will be created in the `{language}/ql/{subdir}/change-notes` directory, where `subdir` is either `src` or `lib`. From a4eab484ceb06d5da14c8a84f173561d6f097c65 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 24 Oct 2025 13:32:39 +0200 Subject: [PATCH 3/3] Address review comments --- .../security/CWE-918/SanitizationTests.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/java/ql/test/query-tests/security/CWE-918/SanitizationTests.java b/java/ql/test/query-tests/security/CWE-918/SanitizationTests.java index f7e46b62946a..f32de324a1de 100644 --- a/java/ql/test/query-tests/security/CWE-918/SanitizationTests.java +++ b/java/ql/test/query-tests/security/CWE-918/SanitizationTests.java @@ -121,19 +121,15 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) client.send(unsafer10, null); // $ Alert // GOOD: sanitisation by regexp validation - String safeUri10 = "https://example.com/"; String param10 = request.getParameter("uri10"); - if (param10.matches("[a-zA-Z0-9/_-]+")) { - safeUri10 = safeUri10 + param10; + if (param10.matches("[a-zA-Z0-9_-]+")) { + HttpRequest r10 = HttpRequest.newBuilder(new URI(param10)).build(); + client.send(r10, null); } - HttpRequest r10 = HttpRequest.newBuilder(new URI(safeUri10)).build(); - client.send(r10, null); - String param11 = request.getParameter("uri11"); validate(param11); - String safeUri11 = "https://example.com/" + param11; - HttpRequest r11 = HttpRequest.newBuilder(new URI(safeUri11)).build(); + HttpRequest r11 = HttpRequest.newBuilder(new URI(param11)).build(); client.send(r11, null); } catch (Exception e) { // TODO: handle exception @@ -141,7 +137,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) } private void validate(String s) { - if (!s.matches("[a-zA-Z0-9/_-]+")) { + if (!s.matches("[a-zA-Z0-9_-]+")) { throw new IllegalArgumentException("Invalid ID"); } }