From 302edcaa71d29a5482e3274e7e924391d3421bf1 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 22798eda937d..542c1ad8d5e1 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 @@ -213,7 +213,7 @@ else if (resource instanceof 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 0270f00b8c7c010073f48bd12d799a11dd36d2a2 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 | 5 ++-- 6 files changed, 75 insertions(+), 15 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 542c1ad8d5e1..920330f0df49 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 @@ -105,7 +105,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) { @@ -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, "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,10 +173,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 7c4db588b450..751ac554b3e5 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 @@ -485,7 +485,8 @@ protected Mono getResource(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) { @@ -527,6 +528,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, "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 @@ -588,7 +604,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 c213b1672ac4..a8e714539e48 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 @@ -319,7 +319,6 @@ public 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, ResourceWebHandler handler) { @@ -359,7 +358,6 @@ private void testResolvePathWithTraversal(HttpMethod httpMethod) throws Exceptio testResolvePathWithTraversal(httpMethod, "/url:" + secretPath, location); testResolvePathWithTraversal(httpMethod, "////../.." + secretPath, location); testResolvePathWithTraversal(httpMethod, "/%2E%2E/testsecret/secret.txt", location); - testResolvePathWithTraversal(httpMethod, "%2F%2F%2E%2E%2F%2Ftestsecret/secret.txt", location); testResolvePathWithTraversal(httpMethod, "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 83b4f8fe43fe..9ab20146c465 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 @@ -106,7 +106,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) { @@ -148,6 +149,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, "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; @@ -158,7 +174,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 f5a1ada29408..c609e8a4c24b 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 @@ -646,7 +646,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) { @@ -688,6 +689,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, "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 @@ -750,7 +766,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 834aab2694a6..fb9d14d5d20a 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 @@ -362,7 +362,6 @@ public 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 { @@ -742,6 +741,8 @@ public void ignoreLastModified() throws Exception { assertThat(this.response.getContentAsString()).isEqualTo("h1 { color:red; }"); } + + @Test public void servletContextRootValidation() { StaticWebApplicationContext context = new StaticWebApplicationContext() { @@ -762,7 +763,7 @@ public Resource getResource(String location) { } - private long resourceLastModified(String resourceName) throws IOException { + private long resourceLastModified(String resourceName) throws IOException { return new ClassPathResource(resourceName, getClass()).getFile().lastModified(); } From 6f22dbb04e22df2048559c1935c409f467322019 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 14 Oct 2024 11:13:43 -0600 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 920330f0df49..55f89ad95f47 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 @@ -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, "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, "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 751ac554b3e5..9b11a7e6625c 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 @@ -529,20 +529,28 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { } private static String normalizePath(String path) { - if (path.contains("%")) { - try { - path = URLDecoder.decode(path, "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, "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 9ab20146c465..a989b4aa6bb7 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 @@ -150,20 +150,28 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { } private static String normalizePath(String path) { - if (path.contains("%")) { - try { - path = URLDecoder.decode(path, "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, "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 c609e8a4c24b..687e8ffb6b9a 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 @@ -690,20 +690,28 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { } private static String normalizePath(String path) { - if (path.contains("%")) { - try { - path = URLDecoder.decode(path, "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, "UTF-8"); + } + catch (Exception ex) { + return ""; + } + } + /** * Check whether the given path contains invalid escape sequences. * @param path the path to validate