Tighten UNSAFE_PATH_PATTERN against encoded path-traversal terminators (#49000)

* Tighten UNSAFE_PATH_PATTERN against encoded path-traversal terminators

Fixes #48978

Extends the regex to cover encoded forms that previously bypassed
detection:
  - %3B / %3b (encoded semicolon)
  - %09, %0A, %0D, %00 (control characters)
  - %252E (double-encoded dot)

These encodings do not produce actual path traversal on conformant
servers per RFC 3986 (percent-encoded characters are literals, not
delimiters), but are semantically close enough to the patterns the
regex was designed to block to warrant defense-in-depth coverage.

The end-of-input anchor ($) is moved into the terminator class to
collapse the two pattern alternatives into one, keeping the diff
minimal.

Test changes:
  - 8 new assertions covering encoded semicolons, control character
    terminators, and double-encoded dots.
  - 3 prior assertEquals flipped to assertNull (lines that previously
    asserted %252E%252E/, %252E%252E/#fragment, and ..%3Bsomething/
    were allowed are now expected to be blocked).
  - 1 new negative test confirming %3B as legitimate path content (not
    following a parent-folder sequence) still resolves.

Triple-encoded variants (e.g., %25252E) remain allowed; out of scope
for this issue.

Signed-off-by: Michał Kosiorek <michal.kosiorek@arklink.co>

* Update OAuthRedirectUriTest expectations for double-encoded dots

Follow-up to 36b0b10dd2 — Base IT (6) CI run for #49000 caught a
cross-module integration test that needed updating alongside the
regex change. Local verification of the previous commit covered the
services module (RedirectUtilsTest); testsuite/integration-arquillian
was outside that scope, so the existing OAuthRedirectUriTest.testWildcard
expectations for %252E%252E variants didn't flip with the regex.

Four assertions in testWildcard flipped from true → false to match the
Option A semantic introduced in 36b0b10dd2 (double-encoded dots are
now blocked by UNSAFE_PATH_PATTERN):

  http://example.com/foo/%252E%252E/
  http://example.com/foo/%252E%252E/?some_query_param=some_value
  http://example.com/foo/%252E%252E/?encodeTest=a%3Cb
  http://example.com/foo/%252E%252E/#encodeTest=a%3Cb

Triple-encoded (%25252E) and septuple-encoded variants remain
expected:true — recursive decoding is explicitly out of scope for
#48978.

Verified locally:
  - mvn -pl services -Dtest=RedirectUtilsTest test → 11/11 green.
  - Direct regex match against the four flipped URIs confirms
    UNSAFE_PATH_PATTERN matches each rawPath, mechanically equivalent
    to the verifyRedirectUri code path exercised by the arquillian test.

Refs #48978

Signed-off-by: Michał Kosiorek <michal.kosiorek@arklink.co>

---------

Signed-off-by: Michał Kosiorek <michal.kosiorek@arklink.co>

services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java

@@ -150,8 +150,11 @@ public class RedirectUtils {
     }
 
     // any access to parent folder /../ is unsafe with or without encoding
+    //   <sep>             = / | %2F | %5C | \
+    //   <dots>            = "..", including %2E and %252E (double-encoded) variants
+    //   <terminator>      = / | %2F | %5C | \ | ; | %3B | %09 | %0A | %0D | %00 | end-of-input
     private final static Pattern UNSAFE_PATH_PATTERN = Pattern.compile(
-            "(/|%2[fF]|%5[cC]|\\\\)(%2[eE]|\\.){2}(/|%2[fF]|%5[cC]|\\\\|;)|(/|%2[fF]|%5[cC]|\\\\)(%2[eE]|\\.){2}$");
+            "(/|%2[fF]|%5[cC]|\\\\)(%2[eE]|%252[eE]|\\.){2}(/|%2[fF]|%5[cC]|\\\\|;|%3[bB]|%09|%0[aAdD]|%00|$)");
 
     private static boolean areWildcardsAllowed(URI redirectUri) {
         // wildcars are only allowed if no user-info and no unparsed authority and no unsafe pattern in path

services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java

@@ -269,15 +269,26 @@ public class RedirectUtilsTest {
         Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test%5C..", set, false));
         Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test%5C..;/", set, false));
         Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2F%2E%2E%2Fdocumentation", set, false));
+
+        // Issue #48978 — encoded terminators that previously bypassed the regex.
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/..%3B/", set, false));
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/..%3b/", set, false));
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/..%09/", set, false));
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/..%0A/", set, false));
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/..%0D/", set, false));
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/..%00/", set, false));
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/..%3Bsomething/", set, false));
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/", set, false)); // double-encoded dots
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/#encodeTest=a%3Cb", set, false));
+
         Assert.assertEquals("https://keycloak.org/test/.../", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/.../", set, false));
         Assert.assertEquals("https://keycloak.org/test/%2E../", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2E../", set, false));  // encoded
         Assert.assertEquals("https://keycloak.org/test/some%2Fthing/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/some%2Fthing/", set, false));  // encoded
         Assert.assertEquals("https://keycloak.org/test/./", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/./", set, false));
-        Assert.assertEquals("https://keycloak.org/test/%252E%252E/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/", set, false)); // double-encoded
+        Assert.assertEquals("https://keycloak.org/test/%25252E%25252E/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%25252E%25252E/", set, false)); // triple-encoded; out of scope for #48978
-        Assert.assertEquals("https://keycloak.org/test/%252E%252E/#encodeTest=a%3Cb", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/#encodeTest=a%3Cb", set, false)); // double-encoded
-        Assert.assertEquals("https://keycloak.org/test/%25252E%25252E/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%25252E%25252E/", set, false)); // triple-encoded
         Assert.assertEquals("https://keycloak.org/exact/%5C%2F/..", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/exact/%5C%2F/..", set, false));
-        Assert.assertEquals("https://keycloak.org/test/..%3Bsomething/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/..%3Bsomething/", set, false));
+        // Negative test: %3B as legitimate path content (not following `..`) must still resolve.
+        Assert.assertEquals("https://keycloak.org/test/file%3Bname/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/file%3Bname/", set, false));
 
         Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak%2Eorg/test/", set, false));
         Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org%2Ftest%2F%40sample.com", set, false));

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java

@@ -357,12 +357,12 @@ public class OAuthRedirectUriTest extends AbstractKeycloakTest {
         checkRedirectUri("http://example.com/foo/../", false);
         checkRedirectUri("http://example.com/foo/%2E%2E/", false); // url-encoded "http://example.com/foobar/../"
         checkRedirectUri("http://example.com/foo%2F%2E%2E%2F", false); // url-encoded "http://example.com/foobar/../"
-        checkRedirectUri("http://example.com/foo/%252E%252E/", true); // double-encoded "http://example.com/foobar/../"
+        checkRedirectUri("http://example.com/foo/%252E%252E/", false); // double-encoded "../" — blocked by UNSAFE_PATH_PATTERN per #48978
-        checkRedirectUri("http://example.com/foo/%252E%252E/?some_query_param=some_value", true); // double-encoded "http://example.com/foobar/../?some_query_param=some_value"
+        checkRedirectUri("http://example.com/foo/%252E%252E/?some_query_param=some_value", false); // double-encoded "../" — blocked per #48978
-        checkRedirectUri("http://example.com/foo/%252E%252E/?encodeTest=a%3Cb", true); // double-encoded "http://example.com/foobar/../?encodeTest=a<b"
+        checkRedirectUri("http://example.com/foo/%252E%252E/?encodeTest=a%3Cb", false); // double-encoded "../" — blocked per #48978
-        checkRedirectUri("http://example.com/foo/%252E%252E/#encodeTest=a%3Cb", true); // double-encoded "http://example.com/foobar/../?encodeTest=a<b"
+        checkRedirectUri("http://example.com/foo/%252E%252E/#encodeTest=a%3Cb", false); // double-encoded "../" — blocked per #48978
-        checkRedirectUri("http://example.com/foo/%25252E%25252E/", true); // triple-encoded "http://example.com/foobar/../"
+        checkRedirectUri("http://example.com/foo/%25252E%25252E/", true); // triple-encoded — remains allowed (recursive-decode out of scope for #48978)
-        checkRedirectUri("http://example.com/foo/%2525252525252E%2525252525252E/", true); // seventh-encoded "http://example.com/foobar/../"
+        checkRedirectUri("http://example.com/foo/%2525252525252E%2525252525252E/", true); // seventh-encoded — remains allowed (recursive-decode out of scope for #48978)
 
         checkRedirectUri("http://example.com/foo?encodeTest=a%3Cb", true);
         checkRedirectUri("http://example.com/foo?encodeTest=a%3Cb#encode2=a%3Cb", true);