From f7ef346020dfefc2f72eb9974beb4ed0b57025a8 Mon Sep 17 00:00:00 2001 From: Chinmay Kawle Date: Tue, 22 Sep 2026 18:15:13 +0000 Subject: [PATCH 1/9] fix: match filter url-patterns against the decoded, normalized path Filter selection keyed off getRequestURI(), which returns the raw undecoded path, while servlet resolution keys off getPathInfo(), which decodes. A percent-encoded or dot-segment spelling of a protected path therefore failed to select the filter mapped to that path while still routing to the servlet mapped to it, bypassing filter-based authorization. Canonicalize the path once (single percent-decode, then normalize away empty, "." and ".." segments) and use that for both matching and the filter chain cache key, so filter selection and servlet resolution can no longer disagree about which path is being requested. Decoding is not delegated to URLDecoder, which implements form encoding and would turn a literal "+" in a path segment into a space. Also make pathMatches consistently case-insensitive. Its exact-equality fast path always lowercased both sides while its segment comparison was case-sensitive, so the two halves disagreed and a differently-cased path could skip its filter. Over-selecting a filter is fail-safe; under-selecting one is an authorization bypass. --- .../internal/servlet/FilterChainManager.java | 107 +++++++++- .../FilterChainManagerPathBypassTest.java | 190 ++++++++++++++++++ 2 files changed, 289 insertions(+), 8 deletions(-) create mode 100644 aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java index 4917b4aaa..fdf24a316 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java @@ -17,8 +17,12 @@ import jakarta.servlet.*; import jakarta.servlet.http.HttpServletRequest; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayDeque; import java.util.Collections; +import java.util.Deque; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -89,7 +93,11 @@ public abstract class FilterChainManagerFilterChainHolder object that can be used to apply the filters to the request */ FilterChainHolder getFilterChain(final HttpServletRequest request, Servlet servlet) { - String targetPath = request.getRequestURI(); + // getRequestURI returns the raw, undecoded path while servlet resolution runs on the decoded path from + // getPathInfo. Matching url-patterns against the raw path let a percent-encoded or dot-segment form of a + // protected path skip its filter and still reach the servlet mapped to it. Canonicalize first so filter + // selection and servlet resolution can never disagree about which path is being requested. + String targetPath = canonicalizeMatchPath(request.getRequestURI()); DispatcherType type = request.getDispatcherType(); // only return the cached result if the filter list hasn't changed in the meanwhile @@ -205,19 +213,24 @@ private void putFilterChainCache(final DispatcherType type, final String targetP * @return true if the given mapping path can apply to the target, false otherwise. */ boolean pathMatches(final String target, final String mapping) { + // Matching is case-insensitive throughout. The exact-equality check below always lowercased both sides while + // the segment comparison further down was case-sensitive, so the two halves of this method disagreed and a + // differently-cased path could skip its filter. Over-selecting a filter is fail-safe; under-selecting one is + // an authorization bypass, so both paths now compare case-insensitively. + String finalTarget = target.toLowerCase(Locale.ENGLISH); + String finalMapping = mapping.toLowerCase(Locale.ENGLISH); + // easiest case, they are exactly the same - if (target.toLowerCase(Locale.ENGLISH).equals(mapping.toLowerCase(Locale.ENGLISH))) { + if (finalTarget.equals(finalMapping)) { return true; } - String finalTarget = target; - String finalMapping = mapping; // strip first slash - if (target.startsWith("/")) { - finalTarget = target.replaceFirst("/", ""); + if (finalTarget.startsWith("/")) { + finalTarget = finalTarget.replaceFirst("/", ""); } - if (mapping.startsWith("/")) { - finalMapping = mapping.replaceFirst("/", ""); + if (finalMapping.startsWith("/")) { + finalMapping = finalMapping.replaceFirst("/", ""); } String[] targetParts = finalTarget.split(PATH_PART_SEPARATOR); @@ -247,6 +260,84 @@ boolean pathMatches(final String target, final String mapping) { } + /** + * Produces the canonical form of a request path for filter matching: percent-decoded exactly once and then + * normalized. This is the same path servlet resolution operates on, which is what stops an encoded or + * dot-segment spelling of a protected path from selecting a different set of filters than the servlet it + * actually reaches. + * + * Decoding is deliberately not delegated to URLDecoder, which implements form encoding and would + * turn a literal "+" in a path segment into a space. + * @param path The raw request path, as returned by getRequestURI + * @return The decoded, normalized path, always starting with "/" and never ending with one + */ + static String canonicalizeMatchPath(final String path) { + if (path == null || path.isEmpty()) { + return PATH_PART_SEPARATOR; + } + return normalizeMatchPath(decodeMatchPath(path)); + } + + + /** + * Percent-decodes a path exactly once, treating the decoded bytes as UTF-8. Malformed escape sequences are left + * as literal characters rather than throwing, because a filter chain still has to be produced for a malformed + * request so that the application can reject it. + */ + private static String decodeMatchPath(final String path) { + if (path.indexOf('%') < 0) { + return path; + } + + ByteArrayOutputStream decoded = new ByteArrayOutputStream(path.length()); + for (int i = 0; i < path.length(); i++) { + char current = path.charAt(i); + if (current == '%' && i + 2 < path.length()) { + int high = Character.digit(path.charAt(i + 1), 16); + int low = Character.digit(path.charAt(i + 2), 16); + if (high >= 0 && low >= 0) { + decoded.write((high << 4) + low); + i += 2; + continue; + } + } + byte[] literal = String.valueOf(current).getBytes(StandardCharsets.UTF_8); + decoded.write(literal, 0, literal.length); + } + + return new String(decoded.toByteArray(), StandardCharsets.UTF_8); + } + + + /** + * Collapses empty segments and resolves "." and ".." segments. Traversal above the root is contained rather than + * rejected, so that a path can never normalize to something outside the application. + */ + private static String normalizeMatchPath(final String path) { + Deque segments = new ArrayDeque<>(); + for (String segment : path.split(PATH_PART_SEPARATOR, -1)) { + if (segment.isEmpty() || ".".equals(segment)) { + continue; + } + if ("..".equals(segment)) { + segments.pollLast(); + continue; + } + segments.addLast(segment); + } + + if (segments.isEmpty()) { + return PATH_PART_SEPARATOR; + } + + StringBuilder normalized = new StringBuilder(); + for (String segment : segments) { + normalized.append(PATH_PART_SEPARATOR).append(segment); + } + return normalized.toString(); + } + + //------------------------------------------------------------- // Inner Class - //------------------------------------------------------------- diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java new file mode 100644 index 000000000..71727031f --- /dev/null +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java @@ -0,0 +1,190 @@ +package com.amazonaws.serverless.proxy.internal.servlet; + +import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder; +import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext; +import com.amazonaws.services.lambda.runtime.Context; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import jakarta.servlet.DispatcherType; +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.FilterConfig; +import jakarta.servlet.FilterRegistration; +import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; + +import java.io.IOException; +import java.util.EnumSet; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Regression tests for the filter-selection authorization bypass: filter url-pattern matching used the raw, + * undecoded path from getRequestURI() while servlet resolution used the decoded path from + * getPathInfo(). A request for a percent-encoded equivalent of a protected path therefore failed to + * select the filter mapped to that path but still routed to the servlet mapped to it. + * + * Filter selection must be performed on the same decoded, normalized path that servlet resolution uses, so that no + * encoding of a path can cause the two to disagree. + */ +public class FilterChainManagerPathBypassTest { + private static final Context lambdaContext = new MockLambdaContext(); + + private ServletContext servletContext; + private AwsFilterChainManager chainManager; + + @BeforeEach + public void setUp() { + servletContext = new AwsServletContext(null); + FilterRegistration.Dynamic adminFilter = servletContext.addFilter("AdminFilter", new MockFilter()); + adminFilter.addMappingForUrlPatterns(EnumSet.of(DispatcherType.REQUEST), true, "/admin/*"); + chainManager = new AwsFilterChainManager((AwsServletContext) servletContext); + } + + private int filterCountFor(String path) { + AwsProxyHttpServletRequest req = new AwsProxyHttpServletRequest( + new AwsProxyRequestBuilder(path, "GET").build(), lambdaContext, null + ); + req.setServletContext(servletContext); + return chainManager.getFilterChain(req, null).filterCount(); + } + + /** Control: the plain protected path selects the filter. If this fails the test setup is wrong. */ + @Test + void filterChain_plainProtectedPath_selectsFilter() { + assertEquals(1, filterCountFor("/admin/secret")); + } + + /** Control: an unrelated path does not select the filter, i.e. the mapping is actually scoped. */ + @Test + void filterChain_unrelatedPath_doesNotSelectFilter() { + assertEquals(0, filterCountFor("/public/info")); + } + + /** + * The reported bypass. "/%61dmin/secret" decodes to "/admin/secret", which is what servlet resolution sees, + * so filter selection must see it too. + */ + @Test + void filterChain_percentEncodedFirstCharacter_stillSelectsFilter() { + assertEquals(1, filterCountFor("/%61dmin/secret")); + } + + /** Same bypass, encoding a character in the middle of the protected segment. */ + @Test + void filterChain_percentEncodedMiddleCharacter_stillSelectsFilter() { + assertEquals(1, filterCountFor("/adm%69n/secret")); + } + + /** Every character of the protected segment encoded. */ + @Test + void filterChain_fullyEncodedSegment_stillSelectsFilter() { + assertEquals(1, filterCountFor("/%61%64%6d%69%6e/secret")); + } + + /** Uppercase percent-encoding hex digits must decode identically to lowercase. */ + @Test + void filterChain_uppercaseHexEncoding_stillSelectsFilter() { + assertEquals(1, filterCountFor("/%41dmin/secret")); + } + + /** + * An encoded path separator collapses to "/admin/secret" once decoded. Servlet resolution decodes it, so + * filter selection must not treat it as a single opaque segment. + */ + @Test + void filterChain_encodedPathSeparator_stillSelectsFilter() { + assertEquals(1, filterCountFor("/admin%2Fsecret")); + } + + /** A dot segment that normalizes back into the protected path must still select the filter. */ + @Test + void filterChain_dotSegmentTraversal_stillSelectsFilter() { + assertEquals(1, filterCountFor("/public/../admin/secret")); + } + + /** + * Double encoding must decode exactly once, matching what servlet resolution does. "/%2561dmin" decodes to + * "/%61dmin", which is not "/admin", so the filter legitimately does not apply. + */ + @Test + void filterChain_doubleEncoded_decodesExactlyOnce() { + assertEquals(0, filterCountFor("/%2561dmin/secret")); + } + + /** A malformed percent sequence must not throw; the request still has to be routed. */ + @Test + void filterChain_malformedEncoding_doesNotThrow() { + filterCountFor("/%zz/secret"); + filterCountFor("/admin%"); + filterCountFor("/admin%2"); + } + + /** + * The matcher lowercases both sides on its exact-equality fast path but compares segments case-sensitively, + * so the two halves of the same method disagreed. Matching is case-insensitive throughout, which is the + * fail-safe direction: a filter may run when it need not, never the reverse. + */ + @Test + void filterChain_mixedCasePath_stillSelectsFilter() { + assertEquals(1, filterCountFor("/ADMIN/secret")); + assertEquals(1, filterCountFor("/Admin/secret")); + } + + /** The cache must not let an encoded request poison or satisfy the entry for a different decoded path. */ + @Test + void filterChain_cacheKeyedOnCanonicalPath_encodedAndPlainAgree() { + assertEquals(1, filterCountFor("/%61dmin/secret")); + assertEquals(1, filterCountFor("/admin/secret")); + assertEquals(0, filterCountFor("/public/info")); + assertEquals(1, filterCountFor("/admin/secret")); + } + + /** The canonicalization helper itself, independent of request plumbing. */ + @Test + void canonicalize_decodesAndNormalizes() { + assertEquals("/admin/secret", FilterChainManager.canonicalizeMatchPath("/%61dmin/secret")); + assertEquals("/admin/secret", FilterChainManager.canonicalizeMatchPath("/admin%2Fsecret")); + assertEquals("/admin/secret", FilterChainManager.canonicalizeMatchPath("/public/../admin/secret")); + assertEquals("/admin/secret", FilterChainManager.canonicalizeMatchPath("/admin//secret")); + assertEquals("/admin/secret", FilterChainManager.canonicalizeMatchPath("/./admin/secret")); + assertEquals("/%61dmin/secret", FilterChainManager.canonicalizeMatchPath("/%2561dmin/secret")); + } + + /** + * URLDecoder.decode applies form encoding, which turns "+" into a space. Path segments must not be decoded + * that way, so a literal "+" has to survive canonicalization. + */ + @Test + void canonicalize_plusIsNotTreatedAsSpace() { + assertEquals("/admin+user/secret", FilterChainManager.canonicalizeMatchPath("/admin+user/secret")); + assertEquals("/admin user/secret", FilterChainManager.canonicalizeMatchPath("/admin%20user/secret")); + } + + /** Canonicalization must never escape the root via excess dot segments. */ + @Test + void canonicalize_traversalAboveRootIsContained() { + assertTrue(FilterChainManager.canonicalizeMatchPath("/../../admin/secret").startsWith("/admin")); + assertEquals("/", FilterChainManager.canonicalizeMatchPath("/../..")); + } + + private static class MockFilter implements Filter { + @Override + public void init(FilterConfig filterConfig) throws ServletException { + } + + @Override + public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) + throws IOException, ServletException { + chain.doFilter(req, res); + } + + @Override + public void destroy() { + } + } +} From 0f50d7de787fd87598eee6b0b55bc86bf582426d Mon Sep 17 00:00:00 2001 From: Chinmay Kawle Date: Wed, 30 Sep 2026 17:06:18 +0000 Subject: [PATCH 2/9] fix: stop getServletForPath reading past the end of the request path The mapping loop was bounded by the length of the mapping while indexing into the request path, so any request with fewer segments than a registered mapping threw ArrayIndexOutOfBoundsException. The method is reached on every request via SpringBootLambdaContainerHandler, SpringLambdaContainerHandler and AwsProxyRequestDispatcher, so this was an unauthenticated crash path. Bound the loop by the request path as well. A trailing wildcard still matches the empty remainder, since the servlet spec has "/a/*" match "/a"; any other mapping segment cannot match a path that has run out. Also guard against a null path. The argument is always getPathInfo(), which is null whenever the servlet path covered the whole request, and the first statement dereferenced it. A null path is now treated as the root, which is the same branch "/" already took. --- .../internal/servlet/AwsServletContext.java | 16 ++- .../AwsServletContextServletForPathTest.java | 107 ++++++++++++++++++ 2 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsServletContextServletForPathTest.java diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsServletContext.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsServletContext.java index 94dcaf440..a7ce5bd9a 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsServletContext.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsServletContext.java @@ -223,18 +223,30 @@ public RequestDispatcher getNamedDispatcher(String s) { } public Servlet getServletForPath(String path) { - String[] pathParts = path.split("/"); + // getPathInfo() is null whenever the servlet path covered the whole request, so callers can legitimately + // pass null here. Treat it as the root rather than dereferencing it. + String targetPath = (path == null ? "/" : path); + String[] pathParts = targetPath.split("/"); for (AwsServletRegistration reg : servletRegistrations.values()) { for (String p : reg.getMappings()) { if ("".equals(p) || "/".equals(p) || "/*".equals(p)) { return reg.getServlet(); } // if I have no path and I haven't matched something now I'll just move on to the next - if ("".equals(path) || "/".equals(path)) { + if ("".equals(targetPath) || "/".equals(targetPath)) { continue; } String[] regParts = p.split("/"); for (int i = 0; i < regParts.length; i++) { + if (i >= pathParts.length) { + // The request has fewer segments than this mapping. A trailing wildcard still matches the + // empty remainder - the servlet spec has "/a/*" match "/a" - but anything else cannot, and + // walking further would read past the end of the request path. + if ("*".equals(regParts[i])) { + return reg.getServlet(); + } + break; + } if (!regParts[i].equals(pathParts[i]) && !"*".equals(regParts[i])) { break; } diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsServletContextServletForPathTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsServletContextServletForPathTest.java new file mode 100644 index 000000000..7541f4a14 --- /dev/null +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsServletContextServletForPathTest.java @@ -0,0 +1,107 @@ +package com.amazonaws.serverless.proxy.internal.servlet; + +import org.junit.jupiter.api.Test; + +import jakarta.servlet.ServletRegistration; +import jakarta.servlet.http.HttpServlet; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * Regression tests for AwsServletContext.getServletForPath. + * + * The matching loop was bounded by the length of the mapping while indexing into the request path, + * so any request with fewer segments than a registered mapping read past the end of the path array and threw + * ArrayIndexOutOfBoundsException. The method is reached on every request through + * SpringBootLambdaContainerHandler, SpringLambdaContainerHandler and + * AwsProxyRequestDispatcher, and its argument is getPathInfo(), which is null whenever the + * servlet path covered the whole request. + */ +public class AwsServletContextServletForPathTest { + + private static class NamedServlet extends HttpServlet { + private final String id; + + NamedServlet(String id) { + this.id = id; + } + + String getId() { + return id; + } + } + + private AwsServletContext contextWithMapping(String... mappings) { + AwsServletContext ctx = new AwsServletContext(null); + ServletRegistration.Dynamic reg = ctx.addServlet("deep", new NamedServlet("deep")); + reg.addMapping(mappings); + return ctx; + } + + /** The crash: request path shorter than the mapping. Previously ArrayIndexOutOfBoundsException. */ + @Test + void getServletForPath_pathShorterThanMapping_doesNotThrow() { + AwsServletContext ctx = contextWithMapping("/first/second/third"); + assertDoesNotThrow(() -> ctx.getServletForPath("/first")); + assertNull(ctx.getServletForPath("/first")); + } + + /** Same crash one segment further in, to show it is not an off-by-one at a single depth. */ + @Test + void getServletForPath_pathShorterThanMappingByOne_doesNotThrow() { + AwsServletContext ctx = contextWithMapping("/first/second/third"); + assertDoesNotThrow(() -> ctx.getServletForPath("/first/second")); + assertNull(ctx.getServletForPath("/first/second")); + } + + /** A null path is legitimate - getPathInfo() returns null when the servlet path covered everything. */ + @Test + void getServletForPath_nullPath_doesNotThrow() { + AwsServletContext ctx = contextWithMapping("/first/second"); + assertDoesNotThrow(() -> ctx.getServletForPath(null)); + assertNull(ctx.getServletForPath(null)); + } + + /** A null path must still reach a catch-all servlet, exactly as "/" does. */ + @Test + void getServletForPath_nullPathWithCatchAllMapping_returnsServlet() { + AwsServletContext ctx = new AwsServletContext(null); + NamedServlet root = new NamedServlet("root"); + ctx.addServlet("root", root).addMapping("/*"); + assertEquals(root, ctx.getServletForPath(null)); + assertEquals(root, ctx.getServletForPath("/")); + } + + /** Per the servlet spec "/first/*" matches "/first" itself, not only its children. */ + @Test + void getServletForPath_wildcardMatchesBareMappingPrefix() { + AwsServletContext ctx = contextWithMapping("/first/*"); + assertEquals("deep", ((NamedServlet) ctx.getServletForPath("/first")).getId()); + assertEquals("deep", ((NamedServlet) ctx.getServletForPath("/first/second")).getId()); + } + + /** Existing prefix behaviour must be unchanged: a mapping matches deeper paths under it. */ + @Test + void getServletForPath_longerPathStillMatches() { + AwsServletContext ctx = contextWithMapping("/first"); + assertEquals("deep", ((NamedServlet) ctx.getServletForPath("/first")).getId()); + assertEquals("deep", ((NamedServlet) ctx.getServletForPath("/first/second/third")).getId()); + } + + /** An unrelated path must still miss. */ + @Test + void getServletForPath_unrelatedPath_returnsNull() { + AwsServletContext ctx = contextWithMapping("/first/second"); + assertNull(ctx.getServletForPath("/other")); + assertNull(ctx.getServletForPath("/other/second")); + } + + /** Empty path keeps its existing behaviour of not matching a scoped mapping. */ + @Test + void getServletForPath_emptyPath_returnsNull() { + AwsServletContext ctx = contextWithMapping("/first/second"); + assertNull(ctx.getServletForPath("")); + } +} From 7ea296ebae47115dda65dfbb171de83470190ab9 Mon Sep 17 00:00:00 2001 From: Chinmay Kawle Date: Wed, 30 Sep 2026 17:23:57 +0000 Subject: [PATCH 3/9] test: end-to-end regression test for the filter authorization bypass Drives a Spring Boot app through the shipped SpringBootLambdaContainerHandler entry point, the same one a deployed function executes, rather than exercising the filter matcher in isolation. The app protects /admin/* with a deny-by-default servlet Filter registered through FilterRegistrationBean with a path-scoped url-pattern - the configuration the reported bypass affects. The filter never calls chain.doFilter, so the response is unambiguous: 403 FORBIDDEN means the filter was selected, and the protected body means it was skipped. Covers API Gateway REST, ALB and HTTP API v2. Against the unfixed matcher 18 of the 24 cases fail, with /%61dmin/secret, /adm%69n/secret and /%61%64%6d%69%6e/secret each returning the protected body on all three request types. --- .../spring/FilterAuthorizationBypassTest.java | 132 ++++++++++++++++++ .../AdminAuthorizationFilter.java | 41 ++++++ .../filterauthapp/FilterAuthApplication.java | 38 +++++ .../spring/filterauthapp/LambdaHandler.java | 62 ++++++++ 4 files changed, 273 insertions(+) create mode 100644 aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java create mode 100644 aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/AdminAuthorizationFilter.java create mode 100644 aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/FilterAuthApplication.java create mode 100644 aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/LambdaHandler.java diff --git a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java new file mode 100644 index 000000000..a664687fb --- /dev/null +++ b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java @@ -0,0 +1,132 @@ +package com.amazonaws.serverless.proxy.spring; + +import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder; +import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext; +import com.amazonaws.serverless.proxy.model.AwsProxyResponse; +import com.amazonaws.serverless.proxy.spring.filterauthapp.AdminAuthorizationFilter; +import com.amazonaws.serverless.proxy.spring.filterauthapp.FilterAuthApplication; +import com.amazonaws.serverless.proxy.spring.filterauthapp.LambdaHandler; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.Arrays; +import java.util.Collection; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * End-to-end regression test for the filter-selection authorization bypass, driven through the shipped Lambda entry + * point rather than the filter matcher in isolation. + * + * The application protects /admin/* with a deny-by-default servlet Filter that never calls chain.doFilter, so the + * response is unambiguous: 403 with FORBIDDEN means the filter was selected and ran, and 200 with TOP_SECRET means it + * was skipped and the protected handler served the request. + * + * Before the fix, filter url-pattern matching used the raw undecoded path from getRequestURI() while servlet + * resolution used the decoded path from getPathInfo(), so an encoded or dot-segment spelling of /admin/secret + * returned TOP_SECRET with the filter never invoked. + */ +public class FilterAuthorizationBypassTest { + private final MockLambdaContext lambdaContext = new MockLambdaContext(); + + private LambdaHandler handler; + + public static Collection data() { + return Arrays.asList(new Object[]{"API_GW", "ALB", "HTTP_API"}); + } + + private AwsProxyResponse get(String reqType, String path) { + handler = new LambdaHandler(reqType); + AdminAuthorizationFilter.resetInvocations(); + return handler.handleRequest(new AwsProxyRequestBuilder(path, "GET"), lambdaContext); + } + + private void assertProtected(String reqType, String path) { + AwsProxyResponse resp = get(reqType, path); + assertEquals(403, resp.getStatusCode(), + "expected the filter to protect " + path + " on " + reqType + " but got " + resp.getBody()); + assertEquals(AdminAuthorizationFilter.FORBIDDEN_BODY, resp.getBody()); + assertNotEquals(FilterAuthApplication.SECRET_BODY, resp.getBody()); + assertTrue(AdminAuthorizationFilter.getInvocations() > 0, + "the filter was never invoked for " + path + " on " + reqType); + } + + /** Control: the plain protected path is blocked. If this fails, the app is not wired as the test assumes. */ + @MethodSource("data") + @ParameterizedTest + void plainProtectedPath_isBlocked(String reqType) { + assertProtected(reqType, "/admin/secret"); + } + + /** Control: the filter mapping really is scoped, so an unprotected path is served normally. */ + @MethodSource("data") + @ParameterizedTest + void unprotectedPath_isServed(String reqType) { + AwsProxyResponse resp = get(reqType, "/public/info"); + assertEquals(200, resp.getStatusCode()); + assertEquals(FilterAuthApplication.PUBLIC_BODY, resp.getBody()); + assertEquals(0, AdminAuthorizationFilter.getInvocations()); + } + + /** The reported bypass: %61 is an encoded "a", so this is /admin/secret. */ + @MethodSource("data") + @ParameterizedTest + void percentEncodedFirstCharacter_isBlocked(String reqType) { + assertProtected(reqType, "/%61dmin/secret"); + } + + /** The same bypass with the encoding in the middle of the protected segment. */ + @MethodSource("data") + @ParameterizedTest + void percentEncodedMiddleCharacter_isBlocked(String reqType) { + assertProtected(reqType, "/adm%69n/secret"); + } + + /** Every character of the protected segment encoded. */ + @MethodSource("data") + @ParameterizedTest + void fullyEncodedSegment_isBlocked(String reqType) { + assertProtected(reqType, "/%61%64%6d%69%6e/secret"); + } + + /** Uppercase hex digits must decode the same as lowercase. */ + @MethodSource("data") + @ParameterizedTest + void uppercaseHexEncoding_isBlocked(String reqType) { + assertProtected(reqType, "/%41dmin/secret"); + } + + /** + * A dot-segment detour that resolves back into the protected path, and which needs no encoding at all. Spring + * normalizes this to /admin/secret when routing, so filter selection has to see it the same way. + */ + @MethodSource("data") + @ParameterizedTest + void dotSegmentTraversal_isBlocked(String reqType) { + assertProtected(reqType, "/public/../admin/secret"); + } + + /** The protected body must never be returned for any spelling of the protected path. */ + @MethodSource("data") + @ParameterizedTest + void noSpellingOfProtectedPathLeaksTheSecret(String reqType) { + for (String path : new String[]{ + "/admin/secret", + "/%61dmin/secret", + "/adm%69n/secret", + "/%61%64%6d%69%6e/secret", + "/%41dmin/secret", + "/public/../admin/secret", + "/admin%2Fsecret", + "/ADMIN/secret", + "/admin//secret", + "/./admin/secret" + }) { + AwsProxyResponse resp = get(reqType, path); + assertNotEquals(FilterAuthApplication.SECRET_BODY, resp.getBody(), + "the secret leaked via " + path + " on " + reqType); + } + } +} diff --git a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/AdminAuthorizationFilter.java b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/AdminAuthorizationFilter.java new file mode 100644 index 000000000..ce367d2e9 --- /dev/null +++ b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/AdminAuthorizationFilter.java @@ -0,0 +1,41 @@ +package com.amazonaws.serverless.proxy.spring.filterauthapp; + +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletResponse; + +import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * A deny-by-default authorization filter, mapped to the protected path only. It never calls + * chain.doFilter, so the response tells you unambiguously whether the filter was selected for the + * request: 403 means it ran, and the protected body coming back means it did not. + */ +public class AdminAuthorizationFilter implements Filter { + public static final String FORBIDDEN_BODY = "FORBIDDEN"; + + private static final AtomicInteger INVOCATIONS = new AtomicInteger(0); + + public static int getInvocations() { + return INVOCATIONS.get(); + } + + public static void resetInvocations() { + INVOCATIONS.set(0); + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + INVOCATIONS.incrementAndGet(); + HttpServletResponse httpResponse = (HttpServletResponse) response; + httpResponse.setStatus(403); + httpResponse.setContentType("text/plain"); + httpResponse.getWriter().write(FORBIDDEN_BODY); + // deliberately not calling chain.doFilter - the request stops here + } +} diff --git a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/FilterAuthApplication.java b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/FilterAuthApplication.java new file mode 100644 index 000000000..a185e5b05 --- /dev/null +++ b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/FilterAuthApplication.java @@ -0,0 +1,38 @@ +package com.amazonaws.serverless.proxy.spring.filterauthapp; + +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.context.annotation.Bean; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +/** + * A plain Spring Boot servlet application whose only protection on /admin/* is a servlet Filter registered with a + * path-scoped url-pattern. This is the configuration the reported authorization bypass affects - hand-rolled filter + * authorization, as opposed to Spring Security. + */ +@SpringBootApplication +@RestController +public class FilterAuthApplication { + public static final String SECRET_BODY = "TOP_SECRET"; + public static final String PUBLIC_BODY = "PUBLIC_OK"; + + @GetMapping("/admin/secret") + public String adminSecret() { + return SECRET_BODY; + } + + @GetMapping("/public/info") + public String publicInfo() { + return PUBLIC_BODY; + } + + @Bean + public FilterRegistrationBean adminAuthorizationFilter() { + FilterRegistrationBean registration = new FilterRegistrationBean<>(); + registration.setFilter(new AdminAuthorizationFilter()); + registration.addUrlPatterns("/admin/*"); + registration.setName("adminAuthorizationFilter"); + return registration; + } +} diff --git a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/LambdaHandler.java b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/LambdaHandler.java new file mode 100644 index 000000000..65ccf159b --- /dev/null +++ b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/LambdaHandler.java @@ -0,0 +1,62 @@ +package com.amazonaws.serverless.proxy.spring.filterauthapp; + +import com.amazonaws.serverless.exceptions.ContainerInitializationException; +import com.amazonaws.serverless.proxy.InitializationWrapper; +import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder; +import com.amazonaws.serverless.proxy.model.AwsProxyRequest; +import com.amazonaws.serverless.proxy.model.AwsProxyResponse; +import com.amazonaws.serverless.proxy.model.HttpApiV2ProxyRequest; +import com.amazonaws.serverless.proxy.spring.SpringBootLambdaContainerHandler; +import com.amazonaws.serverless.proxy.spring.SpringBootProxyHandlerBuilder; +import com.amazonaws.services.lambda.runtime.Context; +import com.amazonaws.services.lambda.runtime.RequestHandler; + +/** + * Drives requests through the shipped Lambda entry point, the same one a deployed function executes. + */ +public class LambdaHandler implements RequestHandler { + private static SpringBootLambdaContainerHandler handler; + private static SpringBootLambdaContainerHandler httpApiHandler; + private String type; + + public LambdaHandler(String reqType) { + type = reqType; + try { + switch (type) { + case "API_GW": + case "ALB": + handler = new SpringBootProxyHandlerBuilder() + .defaultProxy() + .initializationWrapper(new InitializationWrapper()) + .servletApplication() + .springBootApplication(FilterAuthApplication.class) + .buildAndInitialize(); + break; + case "HTTP_API": + httpApiHandler = new SpringBootProxyHandlerBuilder() + .defaultHttpApiV2Proxy() + .initializationWrapper(new InitializationWrapper()) + .servletApplication() + .springBootApplication(FilterAuthApplication.class) + .buildAndInitialize(); + break; + } + } catch (ContainerInitializationException e) { + e.printStackTrace(); + } + } + + @Override + public AwsProxyResponse handleRequest(AwsProxyRequestBuilder awsProxyRequest, Context context) { + switch (type) { + case "API_GW": + return handler.proxy(awsProxyRequest.build(), context); + case "ALB": + return handler.proxy(awsProxyRequest.alb().build(), context); + case "HTTP_API": + return httpApiHandler.proxy(awsProxyRequest.toHttpApiV2Request(), context); + default: + throw new RuntimeException("Unknown request type: " + type); + } + } +} From 82991820aa041faef561a910167af668bcc5aa34 Mon Sep 17 00:00:00 2001 From: Chinmay Kawle Date: Wed, 30 Sep 2026 17:27:57 +0000 Subject: [PATCH 4/9] test: note why the springboot4 app needs no Spring Security exclusion --- .../proxy/spring/filterauthapp/FilterAuthApplication.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/FilterAuthApplication.java b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/FilterAuthApplication.java index a185e5b05..ebe3e5b8a 100644 --- a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/FilterAuthApplication.java +++ b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/FilterAuthApplication.java @@ -11,6 +11,9 @@ * path-scoped url-pattern. This is the configuration the reported authorization bypass affects - hand-rolled filter * authorization, as opposed to Spring Security. */ +// No Spring Security auto-configuration to exclude here: Boot 4 moved those classes out of +// org.springframework.boot.autoconfigure.security.servlet, so nothing secures this app implicitly and the +// path-scoped filter below is the only protection. The springboot3 copy of this class does need the exclusion. @SpringBootApplication @RestController public class FilterAuthApplication { From 83037486bdbb4dfa7077bf30be6153ac133bebaf Mon Sep 17 00:00:00 2001 From: Chinmay Kawle Date: Thu, 1 Oct 2026 17:29:47 +0000 Subject: [PATCH 5/9] fix: canonicalize the path for servlet resolution too, not just filter matching Review feedback on #1628: resolving dot segments for filter matching alone reintroduced the filter/servlet disagreement in the opposite direction. "/admin/x/../../public" canonicalized to "/public" for filter selection, so a filter mapped to /admin/* was skipped, while getPathInfo() still returned the un-normalized "/admin/x/../../public" and so still matched a servlet mapped to /admin/*. Reproduced before and after: the filter ran on unmodified main and stopped running with the first version of this change. Canonicalization now backs both decisions. It moves to AwsHttpServletRequest alongside cleanUri and decodeRequestPath, and getPathInfo() returns the canonical path in both the API Gateway and HTTP API v2 request types, so filter selection and servlet resolution read the same path by construction. Decoding only, without resolving dot segments, would have closed the reported direction and left its mirror: a filter mapped to /public/* would be skipped for "/admin/x/../../public" while the framework routed to /public. UrlPathValidator now inspects getRequestURI() instead of getPathInfo(). It rejects paths containing traversal segments, and reading the canonical path would have normalized them away before it could see them. A validator of suspicious input has to inspect the input as it arrived. Adds four tests: a dot segment leading out of a protected path, the same percent-encoded, a dot segment leading into one, and a case asserting that filter selection and servlet resolution agree for every spelling covered. Four of them fail without the getPathInfo change. --- .../AwsHttpApiV2ProxyHttpServletRequest.java | 5 +- .../servlet/AwsHttpServletRequest.java | 82 ++++++++++++++++++ .../servlet/AwsProxyHttpServletRequest.java | 5 +- .../internal/servlet/FilterChainManager.java | 84 +------------------ .../servlet/filters/UrlPathValidator.java | 6 +- .../FilterChainManagerPathBypassTest.java | 83 +++++++++++++++--- 6 files changed, 166 insertions(+), 99 deletions(-) diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java index f318b7277..ddf321aaa 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java @@ -156,8 +156,9 @@ public String getMethod() { @Override public String getPathInfo() { - String pathInfo = cleanUri(request.getRawPath()); - return decodeRequestPath(pathInfo, LambdaContainerHandler.getContainerConfig()); + // Must be the same canonical form filter matching uses, otherwise a path can be spelled so that filter + // selection and servlet resolution disagree about which resource is being requested. + return canonicalizePath(request.getRawPath()); } @Override diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java index 0029ee2d4..c6bf73cd1 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java @@ -38,6 +38,7 @@ import jakarta.ws.rs.core.MediaType; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; @@ -47,6 +48,9 @@ import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; +import java.nio.charset.StandardCharsets; +import java.util.ArrayDeque; +import java.util.Deque; /** @@ -768,6 +772,84 @@ static String decodeRequestPath(String requestPath, ContainerConfig config) { } + /** + * Produces the canonical form of a request path for filter matching: percent-decoded exactly once and then + * normalized. This is the same path servlet resolution operates on, which is what stops an encoded or + * dot-segment spelling of a protected path from selecting a different set of filters than the servlet it + * actually reaches. + * + * Decoding is deliberately not delegated to URLDecoder, which implements form encoding and would + * turn a literal "+" in a path segment into a space. + * @param path The raw request path, as returned by getRequestURI + * @return The decoded, normalized path, always starting with "/" and never ending with one + */ + static String canonicalizePath(final String path) { + if (path == null || path.isEmpty()) { + return "/"; + } + return normalizePathSegments(decodePathSegments(path)); + } + + + /** + * Percent-decodes a path exactly once, treating the decoded bytes as UTF-8. Malformed escape sequences are left + * as literal characters rather than throwing, because a filter chain still has to be produced for a malformed + * request so that the application can reject it. + */ + private static String decodePathSegments(final String path) { + if (path.indexOf('%') < 0) { + return path; + } + + ByteArrayOutputStream decoded = new ByteArrayOutputStream(path.length()); + for (int i = 0; i < path.length(); i++) { + char current = path.charAt(i); + if (current == '%' && i + 2 < path.length()) { + int high = Character.digit(path.charAt(i + 1), 16); + int low = Character.digit(path.charAt(i + 2), 16); + if (high >= 0 && low >= 0) { + decoded.write((high << 4) + low); + i += 2; + continue; + } + } + byte[] literal = String.valueOf(current).getBytes(StandardCharsets.UTF_8); + decoded.write(literal, 0, literal.length); + } + + return new String(decoded.toByteArray(), StandardCharsets.UTF_8); + } + + + /** + * Collapses empty segments and resolves "." and ".." segments. Traversal above the root is contained rather than + * rejected, so that a path can never normalize to something outside the application. + */ + private static String normalizePathSegments(final String path) { + Deque segments = new ArrayDeque<>(); + for (String segment : path.split("/", -1)) { + if (segment.isEmpty() || ".".equals(segment)) { + continue; + } + if ("..".equals(segment)) { + segments.pollLast(); + continue; + } + segments.addLast(segment); + } + + if (segments.isEmpty()) { + return "/"; + } + + StringBuilder normalized = new StringBuilder(); + for (String segment : segments) { + normalized.append("/").append(segment); + } + return normalized.toString(); + } + + static String cleanUri(String uri) { String finalUri = (uri == null ? "/" : uri); if (finalUri.equals("/")) { diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java index c2a257d34..381aee090 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java @@ -175,8 +175,9 @@ public String getMethod() { @Override public String getPathInfo() { - String pathInfo = cleanUri(request.getPath()); - return decodeRequestPath(pathInfo, LambdaContainerHandler.getContainerConfig()); + // Must be the same canonical form filter matching uses, otherwise a path can be spelled so that filter + // selection and servlet resolution disagree about which resource is being requested. + return canonicalizePath(request.getPath()); } diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java index fdf24a316..a6e551e6b 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java @@ -17,12 +17,8 @@ import jakarta.servlet.*; import jakarta.servlet.http.HttpServletRequest; -import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.ArrayDeque; import java.util.Collections; -import java.util.Deque; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -97,7 +93,7 @@ FilterChainHolder getFilterChain(final HttpServletRequest request, Servlet servl // getPathInfo. Matching url-patterns against the raw path let a percent-encoded or dot-segment form of a // protected path skip its filter and still reach the servlet mapped to it. Canonicalize first so filter // selection and servlet resolution can never disagree about which path is being requested. - String targetPath = canonicalizeMatchPath(request.getRequestURI()); + String targetPath = AwsHttpServletRequest.canonicalizePath(request.getRequestURI()); DispatcherType type = request.getDispatcherType(); // only return the cached result if the filter list hasn't changed in the meanwhile @@ -260,84 +256,6 @@ boolean pathMatches(final String target, final String mapping) { } - /** - * Produces the canonical form of a request path for filter matching: percent-decoded exactly once and then - * normalized. This is the same path servlet resolution operates on, which is what stops an encoded or - * dot-segment spelling of a protected path from selecting a different set of filters than the servlet it - * actually reaches. - * - * Decoding is deliberately not delegated to URLDecoder, which implements form encoding and would - * turn a literal "+" in a path segment into a space. - * @param path The raw request path, as returned by getRequestURI - * @return The decoded, normalized path, always starting with "/" and never ending with one - */ - static String canonicalizeMatchPath(final String path) { - if (path == null || path.isEmpty()) { - return PATH_PART_SEPARATOR; - } - return normalizeMatchPath(decodeMatchPath(path)); - } - - - /** - * Percent-decodes a path exactly once, treating the decoded bytes as UTF-8. Malformed escape sequences are left - * as literal characters rather than throwing, because a filter chain still has to be produced for a malformed - * request so that the application can reject it. - */ - private static String decodeMatchPath(final String path) { - if (path.indexOf('%') < 0) { - return path; - } - - ByteArrayOutputStream decoded = new ByteArrayOutputStream(path.length()); - for (int i = 0; i < path.length(); i++) { - char current = path.charAt(i); - if (current == '%' && i + 2 < path.length()) { - int high = Character.digit(path.charAt(i + 1), 16); - int low = Character.digit(path.charAt(i + 2), 16); - if (high >= 0 && low >= 0) { - decoded.write((high << 4) + low); - i += 2; - continue; - } - } - byte[] literal = String.valueOf(current).getBytes(StandardCharsets.UTF_8); - decoded.write(literal, 0, literal.length); - } - - return new String(decoded.toByteArray(), StandardCharsets.UTF_8); - } - - - /** - * Collapses empty segments and resolves "." and ".." segments. Traversal above the root is contained rather than - * rejected, so that a path can never normalize to something outside the application. - */ - private static String normalizeMatchPath(final String path) { - Deque segments = new ArrayDeque<>(); - for (String segment : path.split(PATH_PART_SEPARATOR, -1)) { - if (segment.isEmpty() || ".".equals(segment)) { - continue; - } - if ("..".equals(segment)) { - segments.pollLast(); - continue; - } - segments.addLast(segment); - } - - if (segments.isEmpty()) { - return PATH_PART_SEPARATOR; - } - - StringBuilder normalized = new StringBuilder(); - for (String segment : segments) { - normalized.append(PATH_PART_SEPARATOR).append(segment); - } - return normalized.toString(); - } - - //------------------------------------------------------------- // Inner Class - //------------------------------------------------------------- diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidator.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidator.java index aadb26efd..66edec9ac 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidator.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidator.java @@ -72,8 +72,10 @@ public void init(FilterConfig filterConfig) throws ServletException { @Override public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException { - // the getPathInfo method of the AwsProxyHttpServletRequest returns the request path with the correct base path stripped - String path = ((HttpServletRequest)servletRequest).getPathInfo(); + // Deliberately the raw URI rather than getPathInfo. getPathInfo returns the canonical path, with dot + // segments already resolved, so a traversal attempt would be normalized away before this filter could + // reject it. A validator of suspicious input has to inspect the input as it arrived. + String path = ((HttpServletRequest)servletRequest).getRequestURI(); if (path == null) { setErrorResponse(servletResponse); return; diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java index 71727031f..d178e9229 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java @@ -2,6 +2,7 @@ import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder; import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext; +import com.amazonaws.serverless.proxy.internal.testutils.MockServlet; import com.amazonaws.services.lambda.runtime.Context; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -20,6 +21,8 @@ import java.util.EnumSet; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -42,6 +45,7 @@ public void setUp() { servletContext = new AwsServletContext(null); FilterRegistration.Dynamic adminFilter = servletContext.addFilter("AdminFilter", new MockFilter()); adminFilter.addMappingForUrlPatterns(EnumSet.of(DispatcherType.REQUEST), true, "/admin/*"); + servletContext.addServlet("adminServlet", new MockServlet()).addMapping("/admin/*"); chainManager = new AwsFilterChainManager((AwsServletContext) servletContext); } @@ -53,6 +57,14 @@ private int filterCountFor(String path) { return chainManager.getFilterChain(req, null).filterCount(); } + private jakarta.servlet.Servlet servletForPath(String path) { + AwsProxyHttpServletRequest req = new AwsProxyHttpServletRequest( + new AwsProxyRequestBuilder(path, "GET").build(), lambdaContext, null + ); + req.setServletContext(servletContext); + return ((AwsServletContext) servletContext).getServletForPath(req.getPathInfo()); + } + /** Control: the plain protected path selects the filter. If this fails the test setup is wrong. */ @Test void filterChain_plainProtectedPath_selectsFilter() { @@ -144,15 +156,66 @@ void filterChain_cacheKeyedOnCanonicalPath_encodedAndPlainAgree() { assertEquals(1, filterCountFor("/admin/secret")); } + /** + * A dot segment that leads OUT of the protected path must not let the request reach a servlet mapped there. + * Filter selection and servlet resolution both run on the canonical path, so "/admin/x/../../public" is + * "/public" to both: the /admin/* filter correctly does not apply, and neither does the /admin/* servlet. + */ + @Test + void filterChain_dotSegmentLeadingOutOfProtectedPath_agreesWithServletResolution() { + assertEquals(0, filterCountFor("/admin/x/../../public")); + assertNull(servletForPath("/admin/x/../../public")); + } + + /** Same, with the dot segments percent-encoded. */ + @Test + void filterChain_encodedDotSegmentLeadingOut_agreesWithServletResolution() { + assertEquals(0, filterCountFor("/admin/x/%2e%2e/%2e%2e/public")); + assertNull(servletForPath("/admin/x/%2e%2e/%2e%2e/public")); + } + + /** + * The mirror case: a dot segment that lands ON a protected path must select that path's filter. This is the + * direction a decode-only canonicalization would miss. + */ + @Test + void filterChain_dotSegmentLeadingIntoProtectedPath_selectsFilter() { + assertEquals(1, filterCountFor("/public/x/../../admin/secret")); + assertNotNull(servletForPath("/public/x/../../admin/secret")); + } + + /** Whatever the spelling, filter selection and servlet resolution must never disagree. */ + @Test + void filterSelectionAndServletResolutionNeverDisagree() { + for (String path : new String[]{ + "/admin/secret", + "/%61dmin/secret", + "/admin%2Fsecret", + "/admin/x/../../public", + "/admin/x/%2e%2e/%2e%2e/public", + "/public/x/../../admin/secret", + "/public/info", + "/admin//secret", + "/./admin/secret", + "/../../admin/secret" + }) { + boolean filterApplies = filterCountFor(path) > 0; + boolean servletApplies = servletForPath(path) != null; + assertEquals(filterApplies, servletApplies, + "filter selection and servlet resolution disagree for " + path); + } + } + + /** The canonicalization helper itself, independent of request plumbing. */ @Test void canonicalize_decodesAndNormalizes() { - assertEquals("/admin/secret", FilterChainManager.canonicalizeMatchPath("/%61dmin/secret")); - assertEquals("/admin/secret", FilterChainManager.canonicalizeMatchPath("/admin%2Fsecret")); - assertEquals("/admin/secret", FilterChainManager.canonicalizeMatchPath("/public/../admin/secret")); - assertEquals("/admin/secret", FilterChainManager.canonicalizeMatchPath("/admin//secret")); - assertEquals("/admin/secret", FilterChainManager.canonicalizeMatchPath("/./admin/secret")); - assertEquals("/%61dmin/secret", FilterChainManager.canonicalizeMatchPath("/%2561dmin/secret")); + assertEquals("/admin/secret", AwsHttpServletRequest.canonicalizePath("/%61dmin/secret")); + assertEquals("/admin/secret", AwsHttpServletRequest.canonicalizePath("/admin%2Fsecret")); + assertEquals("/admin/secret", AwsHttpServletRequest.canonicalizePath("/public/../admin/secret")); + assertEquals("/admin/secret", AwsHttpServletRequest.canonicalizePath("/admin//secret")); + assertEquals("/admin/secret", AwsHttpServletRequest.canonicalizePath("/./admin/secret")); + assertEquals("/%61dmin/secret", AwsHttpServletRequest.canonicalizePath("/%2561dmin/secret")); } /** @@ -161,15 +224,15 @@ void canonicalize_decodesAndNormalizes() { */ @Test void canonicalize_plusIsNotTreatedAsSpace() { - assertEquals("/admin+user/secret", FilterChainManager.canonicalizeMatchPath("/admin+user/secret")); - assertEquals("/admin user/secret", FilterChainManager.canonicalizeMatchPath("/admin%20user/secret")); + assertEquals("/admin+user/secret", AwsHttpServletRequest.canonicalizePath("/admin+user/secret")); + assertEquals("/admin user/secret", AwsHttpServletRequest.canonicalizePath("/admin%20user/secret")); } /** Canonicalization must never escape the root via excess dot segments. */ @Test void canonicalize_traversalAboveRootIsContained() { - assertTrue(FilterChainManager.canonicalizeMatchPath("/../../admin/secret").startsWith("/admin")); - assertEquals("/", FilterChainManager.canonicalizeMatchPath("/../..")); + assertTrue(AwsHttpServletRequest.canonicalizePath("/../../admin/secret").startsWith("/admin")); + assertEquals("/", AwsHttpServletRequest.canonicalizePath("/../..")); } private static class MockFilter implements Filter { From 43f46886dc81dd7cf2fecaa437c675aae91d3c9e Mon Sep 17 00:00:00 2001 From: Chinmay Kawle Date: Thu, 1 Oct 2026 18:51:55 +0000 Subject: [PATCH 6/9] fix: encode surrogate pairs as a unit when decoding a path Review feedback on #1628: decodePathSegments converted non-escape characters to bytes one UTF-16 char at a time. A character outside the BMP is stored as a surrogate pair, and a lone surrogate cannot be encoded, so each half became the replacement byte and the character came out as "??". Reproduced: "//%61dmin" decoded to "/??/admin". Only triggered when the path also contains a '%', since the early return otherwise skips decoding entirely. BMP characters were unaffected. This reached the application, not just filter matching, because getPathInfo() now returns the canonical path in both request types. The URLDecoder call this replaced did not have the problem. A surrogate pair is now written as a unit. Adds tests for an emoji, a CJK Extension B character, BMP multi-byte characters, and unpaired surrogates, which must not throw. --- .../servlet/AwsHttpServletRequest.java | 6 +++- .../FilterChainManagerPathBypassTest.java | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java index c6bf73cd1..341c2d874 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java @@ -813,8 +813,12 @@ private static String decodePathSegments(final String path) { continue; } } - byte[] literal = String.valueOf(current).getBytes(StandardCharsets.UTF_8); + // A character outside the BMP is stored as a surrogate pair. Encoding either half on its own is not + // possible, so the pair has to be written as a unit or the character is replaced by "??". + int end = i + 1 < path.length() && Character.isSurrogatePair(current, path.charAt(i + 1)) ? i + 2 : i + 1; + byte[] literal = path.substring(i, end).getBytes(StandardCharsets.UTF_8); decoded.write(literal, 0, literal.length); + i = end - 1; } return new String(decoded.toByteArray(), StandardCharsets.UTF_8); diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java index d178e9229..f2669b036 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java @@ -228,6 +228,35 @@ void canonicalize_plusIsNotTreatedAsSpace() { assertEquals("/admin user/secret", AwsHttpServletRequest.canonicalizePath("/admin%20user/secret")); } + /** + * Characters outside the BMP are stored as surrogate pairs. Encoding each half separately replaces the + * character with "??", and because getPathInfo() returns the canonical path the corruption would reach the + * application. Only triggered when the path also contains a '%', since otherwise decoding is skipped. + */ + @Test + void canonicalize_nonBmpCharactersSurviveDecoding() { + assertEquals("/\uD83D\uDE00/admin", AwsHttpServletRequest.canonicalizePath("/\uD83D\uDE00/%61dmin")); + assertEquals("/\uD83D\uDE00/admin", AwsHttpServletRequest.canonicalizePath("/\uD83D\uDE00/admin")); + // CJK Extension B, also outside the BMP + assertEquals("/\uD840\uDC0B/admin", AwsHttpServletRequest.canonicalizePath("/\uD840\uDC0B/%61dmin")); + } + + /** BMP characters were never affected, but pin them so the surrogate handling cannot regress them. */ + @Test + void canonicalize_bmpMultiByteCharactersSurviveDecoding() { + assertEquals("/caf\u00e9/admin", AwsHttpServletRequest.canonicalizePath("/caf\u00e9/%61dmin")); + assertEquals("/\u4f60\u597d/admin", AwsHttpServletRequest.canonicalizePath("/\u4f60\u597d/%61dmin")); + } + + /** An unpaired surrogate is malformed input and must not throw. */ + @Test + void canonicalize_loneSurrogateDoesNotThrow() { + AwsHttpServletRequest.canonicalizePath("/\uD83D/%61dmin"); + AwsHttpServletRequest.canonicalizePath("/%61dmin/\uD83D"); + AwsHttpServletRequest.canonicalizePath("/\uDE00/%61dmin"); + } + + /** Canonicalization must never escape the root via excess dot segments. */ @Test void canonicalize_traversalAboveRootIsContained() { From f1df78da42b7e12e8c1ebeabc31f6151fd64133d Mon Sep 17 00:00:00 2001 From: Chinmay Kawle Date: Thu, 1 Oct 2026 20:18:14 +0000 Subject: [PATCH 7/9] fix: give each caller the form of the path it needs Review feedback on #1630. Three forms of a request path matter, not two, and the previous revision conflated them. UrlPathValidator needs the path decoded but NOT normalized. The comment added in the previous revision was wrong: at the merge base getPathInfo() returned decodeRequestPath(cleanUri(path)), which decodes while leaving dot segments in place, so the validator did see "%2e%2e" as "..". Switching it to getRequestURI() left the escapes encoded, so "/%2e%2e/%2e%2e/x" scored zero dot segments and passed. It now reads a decode-only form. The context path is also excluded from what the validator inspects. It is configured rather than client-supplied, and it contributes slashes without contributing dot segments, which loosens the ratio check: "/../x" was rejected while "/prod/../x" passed. decodePath is the new decode-only helper. It gathers consecutive escapes and decodes them as a group using the configured uriEncoding, which the previous revision ignored in favour of hardcoded UTF-8, and copies literal characters straight through, so a surrogate pair can no longer be split by construction rather than by a special case. Filter matching now reads request.getPathInfo() rather than canonicalizing getRequestURI(). getRequestURI() includes the context path while getPathInfo() does not, and url-patterns are relative to the context, so with a configured stage or base path that mismatch alone could skip a path-scoped filter the servlet still matched. Filter selection and servlet resolution are now the same call, so they cannot drift apart. AwsHttpServletRequestWrapper.getPathInfo also returns the canonical path. It had kept a copy of the pre-fix logic, and it is reached on async dispatch, where AwsProxyRequestDispatcher resolves the servlet from getPathInfo. decodeRequestPath had no callers left and is removed. Adds UrlPathValidatorTraversalTest, seven cases covering encoded, uppercase and mixed-encoding traversal plus the context path. Four fail against the raw-URI version. Also adds a wrapper consistency test. --- .../servlet/AwsHttpServletRequest.java | 83 ++++++++++------- .../servlet/AwsHttpServletRequestWrapper.java | 6 +- .../internal/servlet/FilterChainManager.java | 13 ++- .../servlet/filters/UrlPathValidator.java | 25 ++++- .../FilterChainManagerPathBypassTest.java | 19 ++++ .../UrlPathValidatorTraversalTest.java | 91 +++++++++++++++++++ 6 files changed, 195 insertions(+), 42 deletions(-) create mode 100644 aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidatorTraversalTest.java diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java index 341c2d874..b271054cd 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java @@ -761,67 +761,88 @@ protected Locale parseLanguageTag(String languageTag) { return new Locale(language, country); } - static String decodeRequestPath(String requestPath, ContainerConfig config) { - try { - return URLDecoder.decode(requestPath, config.getUriEncoding()); - } catch (UnsupportedEncodingException ex) { - log.error("Could not URL decode the request path, configured encoding not supported: {}", SecurityUtils.encode(config.getUriEncoding())); - // we do not fail at this. - return requestPath; - } - - } /** - * Produces the canonical form of a request path for filter matching: percent-decoded exactly once and then - * normalized. This is the same path servlet resolution operates on, which is what stops an encoded or - * dot-segment spelling of a protected path from selecting a different set of filters than the servlet it - * actually reaches. + * Produces the canonical form of a request path: percent-decoded exactly once and then normalized. This is the + * form every routing and authorization decision must use, so that no spelling of a path can make two decisions + * disagree about which resource is being requested. * * Decoding is deliberately not delegated to URLDecoder, which implements form encoding and would * turn a literal "+" in a path segment into a space. - * @param path The raw request path, as returned by getRequestURI + * @param path A request path * @return The decoded, normalized path, always starting with "/" and never ending with one */ - static String canonicalizePath(final String path) { + public static String canonicalizePath(final String path) { if (path == null || path.isEmpty()) { return "/"; } - return normalizePathSegments(decodePathSegments(path)); + return normalizePathSegments(decodePath(path)); } /** - * Percent-decodes a path exactly once, treating the decoded bytes as UTF-8. Malformed escape sequences are left - * as literal characters rather than throwing, because a filter chain still has to be produced for a malformed - * request so that the application can reject it. + * Percent-decodes a path exactly once, leaving dot segments in place. Code that rejects suspicious input needs + * this form rather than the canonical one: canonicalizePath resolves "." and ".." away, which hides + * a traversal attempt from any validator looking for it. + * + * Escape sequences are gathered and decoded as a group using the configured URI encoding, so a multi-byte + * character spanning several escapes decodes correctly. Literal characters are copied across untouched, which + * also means a surrogate pair is never split. + * @param path A request path + * @return The path with escape sequences decoded, dot segments untouched */ - private static String decodePathSegments(final String path) { - if (path.indexOf('%') < 0) { + public static String decodePath(final String path) { + if (path == null || path.indexOf('%') < 0) { return path; } - ByteArrayOutputStream decoded = new ByteArrayOutputStream(path.length()); + Charset charset = uriCharset(); + StringBuilder decoded = new StringBuilder(path.length()); + ByteArrayOutputStream pending = new ByteArrayOutputStream(); for (int i = 0; i < path.length(); i++) { char current = path.charAt(i); if (current == '%' && i + 2 < path.length()) { int high = Character.digit(path.charAt(i + 1), 16); int low = Character.digit(path.charAt(i + 2), 16); if (high >= 0 && low >= 0) { - decoded.write((high << 4) + low); + pending.write((high << 4) + low); i += 2; continue; } } - // A character outside the BMP is stored as a surrogate pair. Encoding either half on its own is not - // possible, so the pair has to be written as a unit or the character is replaced by "??". - int end = i + 1 < path.length() && Character.isSurrogatePair(current, path.charAt(i + 1)) ? i + 2 : i + 1; - byte[] literal = path.substring(i, end).getBytes(StandardCharsets.UTF_8); - decoded.write(literal, 0, literal.length); - i = end - 1; + flushDecodedBytes(pending, decoded, charset); + decoded.append(current); } + flushDecodedBytes(pending, decoded, charset); - return new String(decoded.toByteArray(), StandardCharsets.UTF_8); + return decoded.toString(); + } + + + private static void flushDecodedBytes(ByteArrayOutputStream pending, StringBuilder out, Charset charset) { + if (pending.size() > 0) { + out.append(new String(pending.toByteArray(), charset)); + pending.reset(); + } + } + + + /** + * The charset configured for decoding request URIs, falling back to UTF-8 if it is unset or not supported. + * Never throws, because this runs on every request including malformed ones. + */ + private static Charset uriCharset() { + ContainerConfig config = LambdaContainerHandler.getContainerConfig(); + String configured = (config == null ? null : config.getUriEncoding()); + if (configured == null || configured.isEmpty()) { + return StandardCharsets.UTF_8; + } + try { + return Charset.forName(configured); + } catch (Exception e) { + log.warn("Configured uriEncoding is not supported, falling back to UTF-8"); + return StandardCharsets.UTF_8; + } } diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestWrapper.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestWrapper.java index 58d7282ba..b3902ab2e 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestWrapper.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestWrapper.java @@ -27,6 +27,7 @@ import java.util.Map; import static com.amazonaws.serverless.proxy.internal.servlet.AwsProxyHttpServletRequest.cleanUri; +import static com.amazonaws.serverless.proxy.internal.servlet.AwsHttpServletRequest.canonicalizePath; public class AwsHttpServletRequestWrapper implements HttpServletRequest { private HttpServletRequest originalRequest; @@ -86,8 +87,9 @@ public String getMethod() { @Override public String getPathInfo() { - String pathInfo = cleanUri(newPath); - return AwsHttpServletRequest.decodeRequestPath(pathInfo, LambdaContainerHandler.getContainerConfig()); + // Same canonical form the wrapped request and filter matching use. This is reached on async dispatch, where + // AwsProxyRequestDispatcher resolves the servlet from getPathInfo, so it has to agree with filter selection. + return canonicalizePath(newPath); } @Override diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java index a6e551e6b..b50f65044 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java @@ -89,11 +89,14 @@ public abstract class FilterChainManagerFilterChainHolder object that can be used to apply the filters to the request */ FilterChainHolder getFilterChain(final HttpServletRequest request, Servlet servlet) { - // getRequestURI returns the raw, undecoded path while servlet resolution runs on the decoded path from - // getPathInfo. Matching url-patterns against the raw path let a percent-encoded or dot-segment form of a - // protected path skip its filter and still reach the servlet mapped to it. Canonicalize first so filter - // selection and servlet resolution can never disagree about which path is being requested. - String targetPath = AwsHttpServletRequest.canonicalizePath(request.getRequestURI()); + // The same canonical, context-relative path servlet resolution uses, so the two cannot disagree about which + // resource is being requested. getRequestURI is not used here: it is undecoded, and it includes the context + // path, while url-patterns are relative to the context. With a configured stage or base path that mismatch + // alone was enough to skip a path-scoped filter that the servlet still matched. + String targetPath = request.getPathInfo(); + if (targetPath == null || targetPath.isEmpty()) { + targetPath = PATH_PART_SEPARATOR; + } DispatcherType type = request.getDispatcherType(); // only return the cached result if the filter list hasn't changed in the meanwhile diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidator.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidator.java index 66edec9ac..da456de8f 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidator.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidator.java @@ -17,6 +17,7 @@ import jakarta.servlet.*; import jakarta.servlet.annotation.WebFilter; +import com.amazonaws.serverless.proxy.internal.servlet.AwsHttpServletRequest; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; @@ -72,10 +73,12 @@ public void init(FilterConfig filterConfig) throws ServletException { @Override public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException { - // Deliberately the raw URI rather than getPathInfo. getPathInfo returns the canonical path, with dot - // segments already resolved, so a traversal attempt would be normalized away before this filter could - // reject it. A validator of suspicious input has to inspect the input as it arrived. - String path = ((HttpServletRequest)servletRequest).getRequestURI(); + // This filter has to see the path decoded but NOT normalized. getPathInfo resolves dot segments, which + // would hide a traversal attempt from the checks below, and the raw URI leaves escapes encoded, so + // "/%2e%2e/x" would not register as "..". The context path is excluded because it contributes slashes + // without contributing dot segments, which loosens the ratio check further down. + HttpServletRequest httpRequest = (HttpServletRequest) servletRequest; + String path = AwsHttpServletRequest.decodePath(stripContextPath(httpRequest)); if (path == null) { setErrorResponse(servletResponse); return; @@ -141,4 +144,18 @@ private int countStrings(String needle, String haystack) { } return stringCount; } + + /** + * The request URI with the context path removed, still encoded. The context path is configured rather than + * client-supplied, so including it would only dilute the checks applied to the part the client controls. + */ + private static String stripContextPath(HttpServletRequest request) { + String uri = request.getRequestURI(); + String contextPath = request.getContextPath(); + if (uri != null && contextPath != null && !contextPath.isEmpty() && uri.startsWith(contextPath)) { + return uri.substring(contextPath.length()); + } + return uri; + } + } diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java index f2669b036..c6343e6f1 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java @@ -207,6 +207,25 @@ void filterSelectionAndServletResolutionNeverDisagree() { } + /** + * AwsHttpServletRequestWrapper is used on async dispatch, and AwsProxyRequestDispatcher resolves the servlet + * from its getPathInfo. It therefore has to report the same canonical path as the request it wraps, otherwise + * the async path reintroduces the filter/servlet disagreement. + */ + @Test + void requestWrapper_reportsTheSameCanonicalPathAsTheWrappedRequest() { + for (String path : new String[]{ + "/admin/secret", "/%61dmin/secret", "/admin/x/../../public", "/admin//secret", "/./admin/secret"}) { + AwsProxyHttpServletRequest original = new AwsProxyHttpServletRequest( + new AwsProxyRequestBuilder("/unrelated", "GET").build(), lambdaContext, null); + original.setServletContext(servletContext); + AwsHttpServletRequestWrapper wrapped = new AwsHttpServletRequestWrapper(original, path); + assertEquals(AwsHttpServletRequest.canonicalizePath(path), wrapped.getPathInfo(), + "wrapper disagrees with canonicalizePath for " + path); + } + } + + /** The canonicalization helper itself, independent of request plumbing. */ @Test void canonicalize_decodesAndNormalizes() { diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidatorTraversalTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidatorTraversalTest.java new file mode 100644 index 000000000..bfc073ac6 --- /dev/null +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidatorTraversalTest.java @@ -0,0 +1,91 @@ +package com.amazonaws.serverless.proxy.internal.servlet.filters; + +import com.amazonaws.serverless.proxy.internal.LambdaContainerHandler; +import com.amazonaws.serverless.proxy.internal.servlet.AwsHttpServletRequest; +import com.amazonaws.serverless.proxy.internal.servlet.AwsHttpServletResponse; +import com.amazonaws.serverless.proxy.internal.servlet.AwsProxyHttpServletRequest; +import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +/** + * UrlPathValidator rejects traversal attempts up front. It needs the path decoded but not normalized: the raw URI + * leaves "%2e%2e" unrecognizable as "..", and the canonical path has already resolved the dot segments away. + */ +public class UrlPathValidatorTraversalTest { + + @AfterEach + public void reset() { + LambdaContainerHandler.getContainerConfig().setServiceBasePath(null); + LambdaContainerHandler.getContainerConfig().setUseStageAsServletContext(false); + } + + private int statusFor(String path) { + AwsProxyHttpServletRequest req = + new AwsProxyHttpServletRequest(new AwsProxyRequestBuilder(path, "GET").build(), null, null); + AwsHttpServletResponse resp = new AwsHttpServletResponse(req, null); + UrlPathValidator validator = new UrlPathValidator(); + final boolean[] chainCalled = {false}; + try { + validator.init(null); + validator.doFilter(req, resp, (rq, rs) -> chainCalled[0] = true); + } catch (Exception e) { + throw new RuntimeException(e); + } + return resp.getStatus(); + } + + /** Plain traversal was always rejected. */ + @Test + void plainTraversal_isRejected() { + assertEquals(UrlPathValidator.DEFAULT_ERROR_CODE, statusFor("../..")); + } + + /** Percent-encoded traversal must be rejected too. Reading the raw URI misses this entirely. */ + @Test + void percentEncodedTraversal_isRejected() { + assertEquals(UrlPathValidator.DEFAULT_ERROR_CODE, statusFor("/%2e%2e/%2e%2e/x")); + } + + /** Uppercase escapes decode the same way. */ + @Test + void uppercaseEncodedTraversal_isRejected() { + assertEquals(UrlPathValidator.DEFAULT_ERROR_CODE, statusFor("/%2E%2E/%2E%2E/x")); + } + + /** Mixed plain and encoded dots form a traversal too. */ + @Test + void mixedEncodingTraversal_isRejected() { + assertEquals(UrlPathValidator.DEFAULT_ERROR_CODE, statusFor("/.%2e/.%2e/x")); + } + + /** + * A configured base path contributes slashes but never dot segments, which loosened the ratio check. The + * decision must not depend on how the context path is configured. + */ + @Test + void configuredBasePath_doesNotWeakenTheCheck() { + int withoutContext = statusFor("/../x"); + LambdaContainerHandler.getContainerConfig().setServiceBasePath("/prod"); + int withContext = statusFor("/../x"); + assertEquals(withoutContext, withContext, + "the verdict changed when a context path was configured"); + } + + /** An ordinary path must still be allowed through. */ + @Test + void ordinaryPath_isAllowed() { + assertNotEquals(UrlPathValidator.DEFAULT_ERROR_CODE, statusFor("/admin/secret")); + assertNotEquals(UrlPathValidator.DEFAULT_ERROR_CODE, statusFor("/admin/a.b/secret")); + } + + /** The decode-only helper leaves dot segments in place, unlike canonicalizePath. */ + @Test + void decodePath_leavesDotSegmentsInPlace() { + assertEquals("/../../x", AwsHttpServletRequest.decodePath("/%2e%2e/%2e%2e/x")); + assertEquals("/admin/secret", AwsHttpServletRequest.canonicalizePath("/%2e%2e/admin/secret")); + } +} From fdbc60ad311328bc81d8f33c7593506c24f7d7d5 Mon Sep 17 00:00:00 2001 From: Chinmay Kawle Date: Thu, 1 Oct 2026 22:40:36 +0000 Subject: [PATCH 8/9] fix: select a filter if its pattern matches the path under any spelling Review feedback on #1630. Resolving dot segments for both filter selection and servlet resolution still left a bypass, because Spring MVC routes on getRequestURI(), which is neither decoded nor normalized, and it leaves dot segments to a servlet container that does not exist in Lambda. Reproduced through the shipped Lambda entry point against an app with an /admin/** handler and a filter mapped /admin/*: /admin/../public/info 200 WILDCARD_TOP_SECRET filters=0 /admin/secret%2F..%2F..%2Fpublic%2Finfo 200 WILDCARD_TOP_SECRET filters=0 /admin/x/../../public/info 200 WILDCARD_TOP_SECRET filters=0 All three canonicalize to /public/info, so the /admin/* filter was not selected, while Spring matched /admin/** on the raw path and served the handler. The filter ran for all three before this PR. Consumers do not agree on which form of the path they route on: servlet resolution here uses the canonical path, Spring MVC uses the raw request URI, and other code reads the decoded-but-not-normalized form. Choosing one and assuming the rest follow is what produced this bug class repeatedly, so filter selection no longer chooses. A filter applies if its url-pattern matches under the raw, decoded, or canonical spelling. This over-selects by design. A filter may run for a request that is routed elsewhere, which is harmless for an authorization filter and a behaviour change for a filter that transforms requests or responses. Under-selecting is the authorization bypass. Matching two forms is not enough: "/%61dmin/../public" matches /admin/* in its decoded-but-not-normalized form while matching neither raw nor canonical. Not taken from the review: building getRequestURI() from the canonical form would undo the UrlPathValidator fix, which needs the raw path to see traversal segments, and conflicts with the spec requirement that getRequestURI() returns the URI as sent. Leaving a decoded %2F as a non-separator was also not taken, because under over-selection decoding it makes the protective filter more likely to apply. Tests: the invariant changed from "filter selection and servlet resolution agree" to "filter selection never under-selects", which is the property that matters. filterSelectionNeverUnderSelects covers 13 spellings, with a companion asserting unrelated paths are not swept in. AdminWildcardController is now a permanent fixture, since without an /admin/** handler the end-to-end app cannot demonstrate this class at all, and three end-to-end cases cover the step-out spellings across API Gateway, ALB and HTTP API v2. --- .../servlet/AwsHttpServletRequest.java | 17 +++++++ .../internal/servlet/FilterChainManager.java | 22 +++++---- .../FilterChainManagerPathBypassTest.java | 46 ++++++++++++------- .../spring/FilterAuthorizationBypassTest.java | 36 +++++++++++++++ .../AdminWildcardController.java | 15 ++++++ 5 files changed, 112 insertions(+), 24 deletions(-) create mode 100644 aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/AdminWildcardController.java diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java index b271054cd..b292d1dd2 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java @@ -762,6 +762,23 @@ protected Locale parseLanguageTag(String languageTag) { } + /** + * The request URI with the context path removed, exactly as it arrived: not decoded, not normalized. The context + * path is configured rather than client-supplied, so it is excluded from anything that inspects what the client + * actually sent. + * @param request The incoming request + * @return The raw request path relative to the context + */ + public static String contextRelativeRequestUri(HttpServletRequest request) { + String uri = request.getRequestURI(); + String contextPath = request.getContextPath(); + if (uri != null && contextPath != null && !contextPath.isEmpty() && uri.startsWith(contextPath)) { + return uri.substring(contextPath.length()); + } + return (uri == null || uri.isEmpty() ? "/" : uri); + } + + /** * Produces the canonical form of a request path: percent-decoded exactly once and then normalized. This is the * form every routing and authorization decision must use, so that no spelling of a path can make two decisions diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java index b50f65044..629a98fd9 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java @@ -89,14 +89,19 @@ public abstract class FilterChainManagerFilterChainHolder object that can be used to apply the filters to the request */ FilterChainHolder getFilterChain(final HttpServletRequest request, Servlet servlet) { - // The same canonical, context-relative path servlet resolution uses, so the two cannot disagree about which - // resource is being requested. getRequestURI is not used here: it is undecoded, and it includes the context - // path, while url-patterns are relative to the context. With a configured stage or base path that mismatch - // alone was enough to skip a path-scoped filter that the servlet still matched. - String targetPath = request.getPathInfo(); - if (targetPath == null || targetPath.isEmpty()) { - targetPath = PATH_PART_SEPARATOR; + // Downstream consumers do not agree on which form of the path they route on. Servlet resolution here uses the + // canonical path, Spring MVC matches on the raw request URI and leaves dot segments to a servlet container + // that does not exist in Lambda, and other code reads the decoded-but-not-normalized form. Predicting which + // one wins is how this class of bypass keeps coming back, so filter selection does not try: a filter applies + // if its url-pattern matches the path under ANY of those spellings. That over-selects, which means a filter + // may run for a request that ends up routed elsewhere. Under-selecting is the authorization bypass. + String rawPath = AwsHttpServletRequest.contextRelativeRequestUri(request); + String decodedPath = AwsHttpServletRequest.decodePath(rawPath); + String canonicalPath = request.getPathInfo(); + if (canonicalPath == null || canonicalPath.isEmpty()) { + canonicalPath = AwsHttpServletRequest.canonicalizePath(rawPath); } + String targetPath = canonicalPath + "|" + decodedPath + "|" + rawPath; DispatcherType type = request.getDispatcherType(); // only return the cached result if the filter list hasn't changed in the meanwhile @@ -126,8 +131,9 @@ FilterChainHolder getFilterChain(final HttpServletRequest request, Servlet servl continue; } for (String path : holder.getRegistration().getUrlPatternMappings()) { - if (pathMatches(targetPath, path)) { + if (pathMatches(canonicalPath, path) || pathMatches(decodedPath, path) || pathMatches(rawPath, path)) { chainHolder.addFilter(holder); + break; } } diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java index c6343e6f1..31faff2b1 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java @@ -157,21 +157,21 @@ void filterChain_cacheKeyedOnCanonicalPath_encodedAndPlainAgree() { } /** - * A dot segment that leads OUT of the protected path must not let the request reach a servlet mapped there. - * Filter selection and servlet resolution both run on the canonical path, so "/admin/x/../../public" is - * "/public" to both: the /admin/* filter correctly does not apply, and neither does the /admin/* servlet. + * A dot segment that steps OUT of the protected prefix must still select that prefix's filter. The canonical + * form is "/public", but Spring MVC matches on the raw request URI and leaves dot segments to a servlet + * container that does not exist here, so "/admin/**" still reaches an admin handler. Selecting the filter for + * the raw spelling as well is what stops that from being a bypass. */ @Test - void filterChain_dotSegmentLeadingOutOfProtectedPath_agreesWithServletResolution() { - assertEquals(0, filterCountFor("/admin/x/../../public")); - assertNull(servletForPath("/admin/x/../../public")); + void filterChain_dotSegmentLeadingOutOfProtectedPath_stillSelectsFilter() { + assertEquals(1, filterCountFor("/admin/x/../../public")); } /** Same, with the dot segments percent-encoded. */ @Test - void filterChain_encodedDotSegmentLeadingOut_agreesWithServletResolution() { - assertEquals(0, filterCountFor("/admin/x/%2e%2e/%2e%2e/public")); - assertNull(servletForPath("/admin/x/%2e%2e/%2e%2e/public")); + void filterChain_encodedDotSegmentLeadingOut_stillSelectsFilter() { + assertEquals(1, filterCountFor("/admin/x/%2e%2e/%2e%2e/public")); + assertEquals(1, filterCountFor("/admin/secret%2F..%2F..%2Fpublic")); } /** @@ -184,28 +184,42 @@ void filterChain_dotSegmentLeadingIntoProtectedPath_selectsFilter() { assertNotNull(servletForPath("/public/x/../../admin/secret")); } - /** Whatever the spelling, filter selection and servlet resolution must never disagree. */ + /** + * The invariant that matters: filter selection must never UNDER-select. If any spelling of a path could reach a + * servlet mapped under a protected prefix, the filter for that prefix has to run. Over-selection is permitted + * and expected, because consumers downstream disagree about which form of the path they route on, so predicting + * a single winner is what produced this bug class in the first place. + */ @Test - void filterSelectionAndServletResolutionNeverDisagree() { + void filterSelectionNeverUnderSelects() { for (String path : new String[]{ "/admin/secret", "/%61dmin/secret", + "/adm%69n/secret", + "/ADMIN/secret", "/admin%2Fsecret", "/admin/x/../../public", "/admin/x/%2e%2e/%2e%2e/public", + "/admin/secret%2F..%2F..%2Fpublic", + "/%61dmin/../public", "/public/x/../../admin/secret", - "/public/info", "/admin//secret", "/./admin/secret", "/../../admin/secret" }) { - boolean filterApplies = filterCountFor(path) > 0; - boolean servletApplies = servletForPath(path) != null; - assertEquals(filterApplies, servletApplies, - "filter selection and servlet resolution disagree for " + path); + assertTrue(filterCountFor(path) > 0, + "no filter selected for a spelling that can reach the protected prefix: " + path); } } + /** A path with no relationship to the protected prefix under any spelling must not select its filter. */ + @Test + void filterSelection_unrelatedPathsAreNotOverSelected() { + assertEquals(0, filterCountFor("/public/info")); + assertEquals(0, filterCountFor("/public/x/y/z")); + assertEquals(0, filterCountFor("/")); + } + /** * AwsHttpServletRequestWrapper is used on async dispatch, and AwsProxyRequestDispatcher resolves the servlet diff --git a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java index a664687fb..9bf5d7ecd 100644 --- a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java +++ b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java @@ -4,6 +4,7 @@ import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext; import com.amazonaws.serverless.proxy.model.AwsProxyResponse; import com.amazonaws.serverless.proxy.spring.filterauthapp.AdminAuthorizationFilter; +import com.amazonaws.serverless.proxy.spring.filterauthapp.AdminWildcardController; import com.amazonaws.serverless.proxy.spring.filterauthapp.FilterAuthApplication; import com.amazonaws.serverless.proxy.spring.filterauthapp.LambdaHandler; import org.junit.jupiter.params.ParameterizedTest; @@ -108,6 +109,41 @@ void dotSegmentTraversal_isBlocked(String reqType) { assertProtected(reqType, "/public/../admin/secret"); } + /** + * Spellings that step OUT of the protected prefix. The canonical form is "/public/info", but Spring MVC matches + * on the raw request URI and leaves dot segments to a servlet container that does not exist here, so + * "/admin/**" still reaches an admin handler. Filter selection has to apply for the raw spelling too. + */ + @MethodSource("data") + @ParameterizedTest + void dotSegmentSteppingOutOfProtectedPrefix_isBlocked(String reqType) { + assertProtected(reqType, "/admin/../public/info"); + assertProtected(reqType, "/admin/x/../../public/info"); + } + + /** The same, with the separators and dot segments percent-encoded. */ + @MethodSource("data") + @ParameterizedTest + void encodedSeparatorSteppingOutOfProtectedPrefix_isBlocked(String reqType) { + assertProtected(reqType, "/admin/secret%2F..%2F..%2Fpublic%2Finfo"); + } + + /** No spelling may reach the wildcard admin handler without its filter. */ + @MethodSource("data") + @ParameterizedTest + void wildcardAdminHandlerIsNeverReachedUnfiltered(String reqType) { + for (String path : new String[]{ + "/admin/../public/info", + "/admin/x/../../public/info", + "/admin/secret%2F..%2F..%2Fpublic%2Finfo", + "/%61dmin/../public/info"}) { + AwsProxyResponse resp = get(reqType, path); + assertNotEquals(AdminWildcardController.WILDCARD_SECRET, resp.getBody(), + "the wildcard admin handler was reached unfiltered via " + path + " on " + reqType); + } + } + + /** The protected body must never be returned for any spelling of the protected path. */ @MethodSource("data") @ParameterizedTest diff --git a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/AdminWildcardController.java b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/AdminWildcardController.java new file mode 100644 index 000000000..899cc0a7d --- /dev/null +++ b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/filterauthapp/AdminWildcardController.java @@ -0,0 +1,15 @@ +package com.amazonaws.serverless.proxy.spring.filterauthapp; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +/** Mirrors the reviewer's scenario: an admin handler whose pattern can match a raw, un-normalized path. */ +@RestController +public class AdminWildcardController { + public static final String WILDCARD_SECRET = "WILDCARD_TOP_SECRET"; + + @GetMapping("/admin/**") + public String anyAdmin() { + return WILDCARD_SECRET; + } +} From 65c533fc441ab37ffd0a40e04cccb843685de2f9 Mon Sep 17 00:00:00 2001 From: Chinmay Kawle Date: Fri, 2 Oct 2026 18:47:01 +0000 Subject: [PATCH 9/9] fix: match filter patterns on the canonical and decoded paths only The third arm of the union matched the raw path, and it was not earning its place. A brute-force sweep of 688,908 (path, url-pattern) pairs found it was the only matching arm in 1,462 of them, and in every one of those the url-pattern itself contained a percent escape, for example /adm%69n/*. The servlet specification matches url-patterns against the decoded path, so such a pattern would not match in a real container either. Dropping it means filter selection over-selects on two spellings instead of three, which is a smaller behaviour change to justify, and nothing changes for any pattern anyone would realistically register. The comment also credited the raw arm with covering Spring MVC. That was wrong. Spring MVC routes on the undecoded request URI, and the decoded-but-not- normalized arm is what covers it: matching on the canonical path alone leaves /admin/../public/info reaching an /admin/** handler with its filter skipped. --- .../internal/servlet/FilterChainManager.java | 22 ++++++++++++------- .../FilterChainManagerPathBypassTest.java | 5 +++-- .../spring/FilterAuthorizationBypassTest.java | 5 +++-- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java index 629a98fd9..4c3af0f9d 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java @@ -89,19 +89,25 @@ public abstract class FilterChainManagerFilterChainHolder object that can be used to apply the filters to the request */ FilterChainHolder getFilterChain(final HttpServletRequest request, Servlet servlet) { - // Downstream consumers do not agree on which form of the path they route on. Servlet resolution here uses the - // canonical path, Spring MVC matches on the raw request URI and leaves dot segments to a servlet container - // that does not exist in Lambda, and other code reads the decoded-but-not-normalized form. Predicting which - // one wins is how this class of bypass keeps coming back, so filter selection does not try: a filter applies - // if its url-pattern matches the path under ANY of those spellings. That over-selects, which means a filter - // may run for a request that ends up routed elsewhere. Under-selecting is the authorization bypass. + // Filter selection and servlet resolution used to read the path from different sources, which is the bypass + // this fixes. They cannot just be pointed at one source, because consumers downstream disagree about which + // form they route on, so a filter applies if its url-pattern matches under either of two spellings. + // + // The canonical path is the one servlet resolution here uses, since getPathInfo() is decoded and normalized. + // The decoded-but-not-normalized path is needed because Spring MVC matches on the undecoded request URI and + // leaves dot segments to a servlet container that does not exist in Lambda. "/admin/../public/info" still + // reaches an "/admin/**" handler even though its canonical form is "/public/info", so matching on the + // canonical path alone leaves that request reaching an admin handler with its filter skipped. + // + // Matching under either spelling over-selects, so a filter may run for a request that is ultimately routed + // elsewhere. Under-selecting is the authorization bypass, so that is the direction chosen. String rawPath = AwsHttpServletRequest.contextRelativeRequestUri(request); String decodedPath = AwsHttpServletRequest.decodePath(rawPath); String canonicalPath = request.getPathInfo(); if (canonicalPath == null || canonicalPath.isEmpty()) { canonicalPath = AwsHttpServletRequest.canonicalizePath(rawPath); } - String targetPath = canonicalPath + "|" + decodedPath + "|" + rawPath; + String targetPath = canonicalPath + "|" + decodedPath; DispatcherType type = request.getDispatcherType(); // only return the cached result if the filter list hasn't changed in the meanwhile @@ -131,7 +137,7 @@ FilterChainHolder getFilterChain(final HttpServletRequest request, Servlet servl continue; } for (String path : holder.getRegistration().getUrlPatternMappings()) { - if (pathMatches(canonicalPath, path) || pathMatches(decodedPath, path) || pathMatches(rawPath, path)) { + if (pathMatches(canonicalPath, path) || pathMatches(decodedPath, path)) { chainHolder.addFilter(holder); break; } diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java index 31faff2b1..97d10483b 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManagerPathBypassTest.java @@ -158,9 +158,10 @@ void filterChain_cacheKeyedOnCanonicalPath_encodedAndPlainAgree() { /** * A dot segment that steps OUT of the protected prefix must still select that prefix's filter. The canonical - * form is "/public", but Spring MVC matches on the raw request URI and leaves dot segments to a servlet + * form is "/public", but Spring MVC matches on the undecoded request URI and leaves dot segments to a servlet * container that does not exist here, so "/admin/**" still reaches an admin handler. Selecting the filter for - * the raw spelling as well is what stops that from being a bypass. + * the decoded-but-not-normalized spelling as well is what stops that from being a bypass: matching on the + * canonical path alone leaves this request reaching the admin handler unfiltered. */ @Test void filterChain_dotSegmentLeadingOutOfProtectedPath_stillSelectsFilter() { diff --git a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java index 9bf5d7ecd..929fa5d65 100644 --- a/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java +++ b/aws-serverless-java-container-springboot4/src/test/java/com/amazonaws/serverless/proxy/spring/FilterAuthorizationBypassTest.java @@ -111,8 +111,9 @@ void dotSegmentTraversal_isBlocked(String reqType) { /** * Spellings that step OUT of the protected prefix. The canonical form is "/public/info", but Spring MVC matches - * on the raw request URI and leaves dot segments to a servlet container that does not exist here, so - * "/admin/**" still reaches an admin handler. Filter selection has to apply for the raw spelling too. + * on the undecoded request URI and leaves dot segments to a servlet container that does not exist here, so + * "/admin/**" still reaches an admin handler. Filter selection has to apply for the decoded-but-not-normalized + * spelling too, since matching on the canonical path alone leaves this reaching the handler unfiltered. */ @MethodSource("data") @ParameterizedTest