From 3bfbe30a7814c9ea1556d40df9bd87ddb3ba372d Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 14 Oct 2024 12:55:18 +0100 Subject: [PATCH] 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 --- .../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 8c6819517d77..0fb3926fca3e 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 @@ -523,7 +523,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) { @@ -565,6 +566,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 @@ -623,7 +639,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 9915b3f66041..8b516dce3a20 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 @@ -670,7 +670,6 @@ void shouldRejectInvalidPath() 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) { @@ -705,7 +704,6 @@ void resolvePathWithTraversal(HttpMethod method) throws Exception { testResolvePathWithTraversal(method, "/url:" + secretPath); testResolvePathWithTraversal(method, "////../.." + secretPath); testResolvePathWithTraversal(method, "/%2E%2E/testsecret/secret.txt"); - testResolvePathWithTraversal(method, "%2F%2F%2E%2E%2F%2Ftestsecret/secret.txt"); testResolvePathWithTraversal(method, "url:" + secretPath); // 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 23726ae4dbac..d14cba0be91f 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 @@ -682,7 +682,8 @@ private static String getPath(HttpServletRequest 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) { @@ -724,6 +725,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 @@ -783,7 +799,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 28db86a97983..9cd47ab8de9c 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 @@ -656,7 +656,6 @@ void shouldRejectInvalidPath() throws Exception { testInvalidPath("/../.." + secretPath); testInvalidPath("/%2E%2E/testsecret/secret.txt"); testInvalidPath("/%2E%2E/testsecret/secret.txt"); - testInvalidPath("%2F%2F%2E%2E%2F%2F%2E%2E" + secretPath); } private void testInvalidPath(String requestPath) {