webapp/keycloak
1
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-10-23 19:29:23 +03:00
Code Activity

Harden PathMatcher URI normalization for leading dot segments and authority-less schemes

Browse Source 
Signed-off-by: Tomas Kyjovsky <tkyjovsk@ibm.com>
This commit is contained in:
Tomas Kyjovsky
2026-10-13 19:24:43 +02:00
committed by Pedro Igor
parent c104900549
commit 372cbab5c3
2 changed files with 71 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
common/src/main/java/org/keycloak/common/util/PathMatcher.java
View File

@@ -346,9 +346,11 @@ public abstract class PathMatcher<P> {
// 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<P> {
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<P> {
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);

54
common/src/test/java/org/keycloak/common/util/PathMatcherTest.java
View File

@@ -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<String> 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<String> matcher = createMatcher(root);
Assertions.assertEquals(root, matcher.matches(uri), "Should resolve to the root for: " + uri);
}
@Test
public void testPercentEncodedUnreservedDecoded() {
PathMatcher<String> 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<String> 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