Merge pull request #12 from cesarhernandezgt/v.6.0.23.RELEASE-TT.x-patch

Backports for addressing CVE-2024-38819
tomitribe · Nov 4, 2024 · dfd06fd · dfd06fd
2 parents d11f78e + bf44f69
commit dfd06fd
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 15 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,29 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
 		return (slash ? "/" : "");
 	}
 
+	private static String normalizePath(String path) {
+		String result = path;
+		if (result.contains("%")) {
+			result = decode(result);
+			if (result.contains("%")) {
+				result = decode(result);
+			}
+			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;
@@ -156,10 +180,7 @@ private boolean isInvalidPath(String path) {
 				return true;
 			}
 		}
-		if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) {
-			return true;
-		}
-		return false;
+		return path.contains("../");
 	}
 
 	/**
@@ -212,7 +233,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) {

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
@@ -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,29 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
 		return (slash ? "/" : "");
 	}
 
+	private static String normalizePath(String path) {
+		String result = path;
+		if (result.contains("%")) {
+			result = decode(result);
+			if (result.contains("%")) {
+				result = decode(result);
+			}
+			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
@@ -596,7 +620,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
@@ -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/...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,29 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
 		return (slash ? "/" : "");
 	}
 
+	private static String normalizePath(String path) {
+		String result = path;
+		if (result.contains("%")) {
+			result = decode(result);
+			if (result.contains("%")) {
+				result = decode(result);
+			}
+			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;
@@ -157,7 +181,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
@@ -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,29 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
 		return (slash ? "/" : "");
 	}
 
+	private static String normalizePath(String path) {
+		String result = path;
+		if (result.contains("%")) {
+			result = decode(result);
+			if (result.contains("%")) {
+				result = decode(result);
+			}
+			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
@@ -751,7 +775,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
@@ -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 {