From 0d98c96e2b6e494f0007aa8922144febd489da11 Mon Sep 17 00:00:00 2001 From: Andrey Popov Date: Thu, 19 Sep 2024 15:29:29 +0400 Subject: [PATCH 1/3] Use correct method to check encoded resource path Closes: gh-33568 (cherry picked from commit 94c04821ffcf0935077d0bf49c7467a12009806f) --- .../reactive/function/server/PathResourceLookupFunction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java index b5ee5b469678..aa314b623565 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java @@ -212,7 +212,7 @@ else if (resource instanceof ClassPathResource classPathResource) { return true; } locationPath = (locationPath.endsWith("/") || locationPath.isEmpty() ? locationPath : locationPath + "/"); - return (resourcePath.startsWith(locationPath) && !isInvalidEncodedInputPath(resourcePath)); + return (resourcePath.startsWith(locationPath) && !isInvalidEncodedResourcePath(resourcePath)); } private boolean isInvalidEncodedResourcePath(String resourcePath) { From c3187ecd809ee97e28ca751c85aadf9c99e0abf7 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 14 Oct 2024 05:55:18 -0600 Subject: [PATCH 2/3] Normalize static resource path early Rather than leaving it to the Resource implementation, and potentially normalizing twice, we apply it once as part of the initial processPath checks. Closes gh-33689 (cherry picked from commit 3bfbe30a7814c9ea1556d40df9bd87ddb3ba372d) --- .../server/PathResourceLookupFunction.java | 23 +++++++++++++++---- .../reactive/resource/ResourceWebHandler.java | 20 ++++++++++++++-- .../resource/ResourceWebHandlerTests.java | 2 -- .../function/PathResourceLookupFunction.java | 20 ++++++++++++++-- .../resource/ResourceHttpRequestHandler.java | 20 ++++++++++++++-- .../ResourceHttpRequestHandlerTests.java | 1 - 6 files changed, 72 insertions(+), 14 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java index aa314b623565..2f0d5fd1088f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java @@ -104,7 +104,8 @@ public Mono apply(ServerRequest request) { protected String processPath(String path) { path = StringUtils.replace(path, "\\", "/"); path = cleanDuplicateSlashes(path); - return cleanLeadingSlash(path); + path = cleanLeadingSlash(path); + return normalizePath(path); } private String cleanDuplicateSlashes(String path) { @@ -146,6 +147,21 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { return (slash ? "/" : ""); } + private static String normalizePath(String path) { + if (path.contains("%")) { + try { + path = URLDecoder.decode(path, StandardCharsets.UTF_8); + } + catch (Exception ex) { + return ""; + } + if (path.contains("../")) { + path = StringUtils.cleanPath(path); + } + } + return path; + } + private boolean isInvalidPath(String path) { if (path.contains("WEB-INF") || path.contains("META-INF")) { return true; @@ -156,10 +172,7 @@ private boolean isInvalidPath(String path) { return true; } } - if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) { - return true; - } - return false; + return path.contains("../"); } /** diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java index e665a68bbda8..59586d34502c 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java @@ -496,7 +496,8 @@ private String getResourcePath(ServerWebExchange exchange) { protected String processPath(String path) { path = StringUtils.replace(path, "\\", "/"); path = cleanDuplicateSlashes(path); - return cleanLeadingSlash(path); + path = cleanLeadingSlash(path); + return normalizePath(path); } private String cleanDuplicateSlashes(String path) { @@ -538,6 +539,21 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { return (slash ? "/" : ""); } + private static String normalizePath(String path) { + if (path.contains("%")) { + try { + path = URLDecoder.decode(path, StandardCharsets.UTF_8); + } + catch (Exception ex) { + return ""; + } + if (path.contains("../")) { + path = StringUtils.cleanPath(path); + } + } + return path; + } + /** * Check whether the given path contains invalid escape sequences. * @param path the path to validate @@ -596,7 +612,7 @@ protected boolean isInvalidPath(String path) { return true; } } - if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) { + if (path.contains("../")) { if (logger.isWarnEnabled()) { logger.warn(LogFormatUtils.formatValue( "Path contains \"../\" after call to StringUtils#cleanPath: [" + path + "]", -1, true)); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java index 15432ea1a666..203f58d75d89 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java @@ -357,7 +357,6 @@ void invalidPath() throws Exception { testInvalidPath("/../.." + secretPath, handler); testInvalidPath("/%2E%2E/testsecret/secret.txt", handler); testInvalidPath("/%2E%2E/testsecret/secret.txt", handler); - testInvalidPath("%2F%2F%2E%2E%2F%2F%2E%2E" + secretPath, handler); } private void testInvalidPath(String requestPath, ResourceWebHandler handler) { @@ -392,7 +391,6 @@ void resolvePathWithTraversal(HttpMethod method) throws Exception { testResolvePathWithTraversal(method, "/url:" + secretPath, location); testResolvePathWithTraversal(method, "////../.." + secretPath, location); testResolvePathWithTraversal(method, "/%2E%2E/testsecret/secret.txt", location); - testResolvePathWithTraversal(method, "%2F%2F%2E%2E%2F%2Ftestsecret/secret.txt", location); testResolvePathWithTraversal(method, "url:" + secretPath, location); // The following tests fail with a MalformedURLException on Windows diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java index e9c700f10f75..135949c9f92a 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java @@ -105,7 +105,8 @@ public Optional apply(ServerRequest request) { protected String processPath(String path) { path = StringUtils.replace(path, "\\", "/"); path = cleanDuplicateSlashes(path); - return cleanLeadingSlash(path); + path = cleanLeadingSlash(path); + return normalizePath(path); } private String cleanDuplicateSlashes(String path) { @@ -147,6 +148,21 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { return (slash ? "/" : ""); } + private static String normalizePath(String path) { + if (path.contains("%")) { + try { + path = URLDecoder.decode(path, StandardCharsets.UTF_8); + } + catch (Exception ex) { + return ""; + } + if (path.contains("../")) { + path = StringUtils.cleanPath(path); + } + } + return path; + } + private boolean isInvalidPath(String path) { if (path.contains("WEB-INF") || path.contains("META-INF")) { return true; @@ -157,7 +173,7 @@ private boolean isInvalidPath(String path) { return true; } } - return path.contains("..") && StringUtils.cleanPath(path).contains("../"); + return path.contains("../"); } private boolean isInvalidEncodedInputPath(String path) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index ab632ddfb6c5..764df2e544dc 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java @@ -650,7 +650,8 @@ protected Resource getResource(HttpServletRequest request) throws IOException { protected String processPath(String path) { path = StringUtils.replace(path, "\\", "/"); path = cleanDuplicateSlashes(path); - return cleanLeadingSlash(path); + path = cleanLeadingSlash(path); + return normalizePath(path); } private String cleanDuplicateSlashes(String path) { @@ -692,6 +693,21 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { return (slash ? "/" : ""); } + private static String normalizePath(String path) { + if (path.contains("%")) { + try { + path = URLDecoder.decode(path, StandardCharsets.UTF_8); + } + catch (Exception ex) { + return ""; + } + if (path.contains("../")) { + path = StringUtils.cleanPath(path); + } + } + return path; + } + /** * Check whether the given path contains invalid escape sequences. * @param path the path to validate @@ -751,7 +767,7 @@ protected boolean isInvalidPath(String path) { return true; } } - if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) { + if (path.contains("../")) { if (logger.isWarnEnabled()) { logger.warn(LogFormatUtils.formatValue( "Path contains \"../\" after call to StringUtils#cleanPath: [" + path + "]", -1, true)); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java index 2fa3383bed50..ef0d406d42f5 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java @@ -365,7 +365,6 @@ void testInvalidPath() throws Exception { testInvalidPath("/../.." + secretPath, handler); testInvalidPath("/%2E%2E/testsecret/secret.txt", handler); testInvalidPath("/%2E%2E/testsecret/secret.txt", handler); - testInvalidPath("%2F%2F%2E%2E%2F%2F%2E%2E" + secretPath, handler); } private void testInvalidPath(String requestPath, ResourceHttpRequestHandler handler) throws Exception { From bf44f69918c54f0aa9b51d6fbc189dc1ba057f9e Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 14 Oct 2024 18:13:43 +0100 Subject: [PATCH 3/3] Update processPath for double encoding See gh-33689 (cherry picked from commit fb7890d73975a3d9e0763e0926df2bd0a608e87e) --- .../server/PathResourceLookupFunction.java | 24 ++++++++++++------- .../reactive/resource/ResourceWebHandler.java | 24 ++++++++++++------- .../function/PathResourceLookupFunction.java | 24 ++++++++++++------- .../resource/ResourceHttpRequestHandler.java | 24 ++++++++++++------- 4 files changed, 64 insertions(+), 32 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java index 2f0d5fd1088f..ca337ddef44f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java @@ -148,20 +148,28 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { } private static String normalizePath(String path) { - if (path.contains("%")) { - try { - path = URLDecoder.decode(path, StandardCharsets.UTF_8); + String result = path; + if (result.contains("%")) { + result = decode(result); + if (result.contains("%")) { + result = decode(result); } - catch (Exception ex) { - return ""; - } - if (path.contains("../")) { - path = StringUtils.cleanPath(path); + if (result.contains("../")) { + return StringUtils.cleanPath(result); } } return path; } + private static String decode(String path) { + try { + return URLDecoder.decode(path, StandardCharsets.UTF_8); + } + catch (Exception ex) { + return ""; + } + } + private boolean isInvalidPath(String path) { if (path.contains("WEB-INF") || path.contains("META-INF")) { return true; diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java index 59586d34502c..8eb122df7b65 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java @@ -540,20 +540,28 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { } private static String normalizePath(String path) { - if (path.contains("%")) { - try { - path = URLDecoder.decode(path, StandardCharsets.UTF_8); + String result = path; + if (result.contains("%")) { + result = decode(result); + if (result.contains("%")) { + result = decode(result); } - catch (Exception ex) { - return ""; - } - if (path.contains("../")) { - path = StringUtils.cleanPath(path); + if (result.contains("../")) { + return StringUtils.cleanPath(result); } } return path; } + private static String decode(String path) { + try { + return URLDecoder.decode(path, StandardCharsets.UTF_8); + } + catch (Exception ex) { + return ""; + } + } + /** * Check whether the given path contains invalid escape sequences. * @param path the path to validate diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java index 135949c9f92a..098179e35393 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java @@ -149,20 +149,28 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { } private static String normalizePath(String path) { - if (path.contains("%")) { - try { - path = URLDecoder.decode(path, StandardCharsets.UTF_8); + String result = path; + if (result.contains("%")) { + result = decode(result); + if (result.contains("%")) { + result = decode(result); } - catch (Exception ex) { - return ""; - } - if (path.contains("../")) { - path = StringUtils.cleanPath(path); + if (result.contains("../")) { + return StringUtils.cleanPath(result); } } return path; } + private static String decode(String path) { + try { + return URLDecoder.decode(path, StandardCharsets.UTF_8); + } + catch (Exception ex) { + return ""; + } + } + private boolean isInvalidPath(String path) { if (path.contains("WEB-INF") || path.contains("META-INF")) { return true; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index 764df2e544dc..c116681ec850 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java @@ -694,20 +694,28 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { } private static String normalizePath(String path) { - if (path.contains("%")) { - try { - path = URLDecoder.decode(path, StandardCharsets.UTF_8); + String result = path; + if (result.contains("%")) { + result = decode(result); + if (result.contains("%")) { + result = decode(result); } - catch (Exception ex) { - return ""; - } - if (path.contains("../")) { - path = StringUtils.cleanPath(path); + if (result.contains("../")) { + return StringUtils.cleanPath(result); } } return path; } + private static String decode(String path) { + try { + return URLDecoder.decode(path, StandardCharsets.UTF_8); + } + catch (Exception ex) { + return ""; + } + } + /** * Check whether the given path contains invalid escape sequences. * @param path the path to validate