This is an automated email from the ASF dual-hosted git repository. smolnar82 pushed a commit to branch knox_idf in repository https://gitbox.apache.org/repos/asf/knox.git
commit ce091a2fb70e134b4c47ece88be73ba42079b9b8 Author: Sandor Molnar <[email protected]> AuthorDate: Tue Aug 11 23:24:13 2026 +0200 KNOX-3414: normalize redirect_uri path before wildcard match (review finding M3) matchesRedirectUri did a startsWith on the un-normalized requested path in the wildcard branch, so a registration of https://app.example/callback/* matched https://app.example/callback/../admin (path "/callback/../admin" starts with "/callback") and the authorization code was delivered to /admin -- a same-host open redirect. The exact, non-wildcard branch was already safe. Normalize both the base and requested paths (URI.normalize()) before the prefix compare so "/callback/../admin" collapses to "/admin" and no longer matches the "/callback" prefix. Genuine sub-paths under the wildcard prefix still match. Covered by AuthorizeResourceRedirectUriMatchTest. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../gateway/service/knoxidf/AuthorizeResource.java | 12 +++- .../AuthorizeResourceRedirectUriMatchTest.java | 74 ++++++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java index 0ba9ac5bd..9747d3c63 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java @@ -422,7 +422,9 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { return basicVerificationResponse; } - private boolean matchesRedirectUri(String requestedUri, Set<String> registeredUris) { + // Package-private for testability (wildcard path-traversal matching is exercised by + // AuthorizeResourceRedirectUriMatchTest); not part of the public resource API. + boolean matchesRedirectUri(String requestedUri, Set<String> registeredUris) { final URI requested = parseUri(requestedUri); if (requested == null) { return false; @@ -434,8 +436,12 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { // "https://good.example*"; match "https://good.example.evil.com";. final URI base = parseUri(registered.substring(0, registered.length() - 1)); if (base != null && sameOrigin(base, requested)) { - final String basePath = base.getPath() == null ? "" : base.getPath(); - final String reqPath = requested.getPath() == null ? "" : requested.getPath(); + // Normalize the requested path before the prefix compare so a traversal segment + // cannot escape the registered prefix: a raw startsWith would let + // ".../callback/../admin" match ".../callback/*" and deliver the code to /admin. + // normalize() collapses "/callback/../admin" to "/admin", which no longer matches. + final String basePath = base.normalize().getPath() == null ? "" : base.normalize().getPath(); + final String reqPath = requested.normalize().getPath() == null ? "" : requested.normalize().getPath(); if (reqPath.startsWith(basePath)) { return true; } diff --git a/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResourceRedirectUriMatchTest.java b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResourceRedirectUriMatchTest.java new file mode 100644 index 000000000..01c88eaf9 --- /dev/null +++ b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResourceRedirectUriMatchTest.java @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.knox.gateway.service.knoxidf; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Collections; +import java.util.Set; + +import org.junit.Test; + +/** + * Verifies wildcard redirect_uri matching rejects path traversal (review finding M3). A wildcard + * registration such as {@code https://app.example/callback/*} must not match + * {@code https://app.example/callback/../admin}: a raw startsWith on the un-normalized path would + * let the traversal escape the registered prefix and deliver the authorization code to /admin + * (a same-host open redirect). The path is normalized before the prefix compare. + */ +public class AuthorizeResourceRedirectUriMatchTest { + + private final AuthorizeResource resource = new AuthorizeResource(); + + private boolean matches(final String requested, final String registered) { + final Set<String> registeredUris = Collections.singleton(registered); + return resource.matchesRedirectUri(requested, registeredUris); + } + + @Test + public void testTraversalEscapingWildcardPrefixIsRejected() { + assertFalse("A traversal that resolves outside the registered prefix must be rejected.", + matches("https://app.example/callback/../admin";, "https://app.example/callback/*";)); + } + + @Test + public void testEncodedPrefixSuffixStillMatches() { + assertTrue("A genuine path under the wildcard prefix must still match.", + matches("https://app.example/callback/oauth";, "https://app.example/callback/*";)); + } + + @Test + public void testExactPrefixMatchesWildcard() { + assertTrue("The wildcard base path itself must match.", + matches("https://app.example/callback/";, "https://app.example/callback/*";)); + } + + @Test + public void testDifferentOriginIsRejected() { + assertFalse("A same-prefix path on a different host must be rejected.", + matches("https://app.example.evil.com/callback/x";, "https://app.example/callback/*";)); + } + + @Test + public void testExact(){ + assertTrue("An exact non-wildcard registration must match verbatim.", + matches("https://app.example/cb";, "https://app.example/cb";)); + assertFalse("An exact registration must not match a different path.", + matches("https://app.example/cb2";, "https://app.example/cb";)); + } +}
