diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java index 4e4ad6a4226..ba84b77f291 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java +++ b/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 + // = / | %2F | %5C | \ + // = "..", including %2E and %252E (double-encoded) variants + // = / | %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 unsafe pattern in path diff --git a/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java b/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java index 8212787608b..7ba4ce1ebc5 100644 --- a/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java +++ b/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java @@ -267,15 +267,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/%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/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/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));