From 372cbab5c3bbd037e18277be5519457bb8a0acbd Mon Sep 17 00:00:00 2001 From: Tomas Kyjovsky Date: Tue, 13 Oct 2026 19:24:43 +0200 Subject: [PATCH] Harden PathMatcher URI normalization for leading dot segments and authority-less schemes Signed-off-by: Tomas Kyjovsky --- .../org/keycloak/common/util/PathMatcher.java | 26 +++++---- .../keycloak/common/util/PathMatcherTest.java | 54 +++++++++++++++++++ 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/common/src/main/java/org/keycloak/common/util/PathMatcher.java b/common/src/main/java/org/keycloak/common/util/PathMatcher.java index 63d9444aa8e..7bb0a0b14d8 100644 --- a/common/src/main/java/org/keycloak/common/util/PathMatcher.java +++ b/common/src/main/java/org/keycloak/common/util/PathMatcher.java @@ -346,9 +346,11 @@ public abstract class PathMatcher

{ // locates where the authority ends so the prefix can be sliced off by length. String prefix = ""; String path = withoutQueryOrFragment; - // cheap pre-check - avoid constructing a URI (and the associated parsing cost) for the common case of a - // plain relative path, which can never have a scheme/authority prefix to preserve - if (withoutQueryOrFragment.contains(SCHEME_AUTHORITY_SEPARATOR)) { + // Only a colon before the first slash can introduce a scheme, so ordinary paths skip costly URI parsing. + // This also catches authority-less URIs such as "https:/api/admin", while leaving colons inside paths alone. + int colon = withoutQueryOrFragment.indexOf(':'); + int slash = withoutQueryOrFragment.indexOf('/'); + if (colon > 0 && (slash == -1 || colon < slash)) { try { URI parsed = new URI(withoutQueryOrFragment.replace("{", "%7B").replace("}", "%7D")); String scheme = parsed.getScheme(); @@ -368,13 +370,9 @@ public abstract class PathMatcher

{ prefix = withoutQueryOrFragment.substring(0, prefixLength); path = withoutQueryOrFragment.substring(prefixLength); } - // scheme == null: not actually an absolute URI - the "://" was just incidental text inside an - // ordinary relative path (e.g. "/api/redirect-to-https://example.com", which RFC 3986 permits - // unrestricted since a leading '/' can never be confused with a scheme) - fall through and treat - // the whole value as a plain path, as before + // scheme == null: the colon did not introduce a valid scheme; treat the value as a plain path } catch (URISyntaxException e) { - // the pre-check found "://", so this was meant to be an absolute URI - reject it outright rather - // than falling back to treating the raw, unmangled "scheme://" text as a plain path + // a possible scheme prefix failed to parse; do not treat it as a plain path return null; } } @@ -421,6 +419,16 @@ public abstract class PathMatcher

{ return null; } + // URI.normalize() leaves leading parent segments unresolved. RFC 3986 section 5.2.4 removes them + // even when there is no preceding segment to remove. Do this after decoding so encoded dots and slashes + // are handled as well. + while (result.startsWith("/../")) { + result = result.substring(3); + } + if (result.equals("/..")) { + result = "/"; + } + // strip trailing slash - prevents bypass via /api/admin/ which routes to /api/admin on the server if (result.length() > 1 && result.endsWith("/")) { result = result.substring(0, result.length() - 1); diff --git a/common/src/test/java/org/keycloak/common/util/PathMatcherTest.java b/common/src/test/java/org/keycloak/common/util/PathMatcherTest.java index 14baffac0fc..db33bc943ef 100644 --- a/common/src/test/java/org/keycloak/common/util/PathMatcherTest.java +++ b/common/src/test/java/org/keycloak/common/util/PathMatcherTest.java @@ -8,6 +8,8 @@ import java.util.Map; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class PathMatcherTest { @@ -56,6 +58,43 @@ public class PathMatcherTest { Assertions.assertEquals("/api/admin", matcher.matches("/api/./admin")); } + @ParameterizedTest + @ValueSource(strings = { + "/../api/admin", + "/../../api/admin", + "/api/../../api/admin", + "/%2E%2E/api/admin", + "/%2e./api/admin", + "/.%2e/api/admin", + "/..%2Fapi/admin", + "https://my.domain/../api/admin", + "https://my.domain/%2E%2E/api/admin" + }) + public void testAboveRootDotSegmentsClampToRoot(String uri) { + // Extra ".." segments stop at the root; the remaining path still matches the resource. + String resource = uri.startsWith("https://") ? "https://my.domain/api/admin" : "/api/admin"; + String wildcard = uri.startsWith("https://") ? "https://my.domain/*" : "/*"; + PathMatcher matcher = createMatcher(resource, wildcard); + + Assertions.assertEquals(resource, matcher.matches(uri), "Should clamp to the root for: " + uri); + } + + @ParameterizedTest + @ValueSource(strings = { + "/..", + "/../", + "/../../", + "/%2E%2E", + "https://my.domain/.." + }) + public void testAboveRootDotSegmentsResolveToRoot(String uri) { + // When nothing follows the extra ".." segments, the result is the root itself. + String root = uri.startsWith("https://") ? "https://my.domain/" : "/"; + PathMatcher matcher = createMatcher(root); + + Assertions.assertEquals(root, matcher.matches(uri), "Should resolve to the root for: " + uri); + } + @Test public void testPercentEncodedUnreservedDecoded() { PathMatcher matcher = createMatcher("/api/admin", "/*"); @@ -333,6 +372,21 @@ public class PathMatcherTest { Assertions.assertNull(matcher.matches("https:///api/admin")); } + @ParameterizedTest + @ValueSource(strings = { + "https:/api/admin", + "http:/api/admin", + "HTTPS:/api/admin", + "https:/api/%61dmin", + "https:/api/admin?x=1", + "https:/api/admin#section" + }) + public void testSingleSlashSchemeUriReturnsNoMatch(String uri) { + PathMatcher matcher = createMatcher("/api/admin", "/*"); + + Assertions.assertNull(matcher.matches(uri), "Should reject authority-less scheme URI: " + uri); + } + @Test public void testIncidentalSchemeSeparatorInRelativePathStillNormalized() { // "://" embedded in an ordinary relative path (no recognized scheme, since it doesn't start with one) is