From 3bfbe30a7814c9ea1556d40df9bd87ddb3ba372d Mon Sep 17 00:00:00 2001
From: rstoyanchev <rossen.stoyanchev@broadcom.com>
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 aa314b6235..2f0d5fd108 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 @@ class PathResourceLookupFunction implements Function<ServerRequest, Mono<Resourc
 	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 @@ class PathResourceLookupFunction implements Function<ServerRequest, Mono<Resourc
 		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 @@ class PathResourceLookupFunction implements Function<ServerRequest, Mono<Resourc
 				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 8c6819517d..0fb3926fca 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 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
 	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 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
 		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 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
 				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 9915b3f660..8b516dce3a 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 @@ class ResourceWebHandlerTests {
 			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 @@ class ResourceWebHandlerTests {
 			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 e9c700f10f..135949c9f9 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 @@ class PathResourceLookupFunction implements Function<ServerRequest, Optional<Res
 	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 @@ class PathResourceLookupFunction implements Function<ServerRequest, Optional<Res
 		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 @@ class PathResourceLookupFunction implements Function<ServerRequest, Optional<Res
 				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 23726ae4db..d14cba0be9 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 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 	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 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 		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 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 				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 28db86a979..9cd47ab8de 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 @@ class ResourceHttpRequestHandlerTests {
 			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) {