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 spring-projectsgh-33689
wso2 · Oct 14, 2024 · 3bfbe30 · 3bfbe30
1 parent 3296289
commit 3bfbe30
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 14 deletions.
diff --git a/...ain/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java b/...ain/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java
@@ -104,7 +104,8 @@ public Mono<Resource> 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/...g-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/...g-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/...flux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java b/...flux/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/...vc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java b/...vc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java
@@ -105,7 +105,8 @@ public Optional<Resource> 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/...vc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/...vc/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/...c/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java b/...c/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) {