mirror of https://github.com/jacekkow/keycloak-protocol-cas

Kosiorkosa47
2026-05-21 6f46fb81318b943593b7b5c863b13bf5cb2afce0
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>
1 files modified
5 ■■■■ changed files
src/main/java/org/keycloak/protocol/cas/utils/RedirectUtils.java 5 ●●●● patch | view | raw | blame | history
src/main/java/org/keycloak/protocol/cas/utils/RedirectUtils.java
@@ -150,8 +150,11 @@
    }
    // 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