Merge pull request #10 from cesarhernandezgt/v.5.3.39.RELEASE-TT.x-patch

Backports for addressing CVE-2024-38819
tomitribe · Oct 30, 2024 · c4a4325 · c4a4325
2 parents a940bfc + 6f22dbb
commit c4a4325
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 16 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
@@ -105,7 +105,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) {
@@ -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, "UTF-8");
+		}
+		catch (Exception ex) {
+			return "";
+		}
+	}
+
 	private boolean isInvalidPath(String path) {
 		if (path.contains("WEB-INF") || path.contains("META-INF")) {
 			return true;
@@ -157,10 +181,7 @@ private boolean isInvalidPath(String path) {
 				return true;
 			}
 		}
-		if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) {
-			return true;
-		}
-		return false;
+		return path.contains("../");
 	}
 
 	/**
@@ -213,7 +234,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) {

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
@@ -485,7 +485,8 @@ protected Mono<Resource> 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,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, "UTF-8");
+		}
+		catch (Exception ex) {
+			return "";
+		}
+	}
+
 	/**
 	 * Check whether the given path contains invalid escape sequences.
 	 * @param path the path to validate
@@ -588,7 +612,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
@@ -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/...vc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java b/...vc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java
@@ -106,7 +106,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) {
@@ -148,6 +149,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, "UTF-8");
+		}
+		catch (Exception ex) {
+			return "";
+		}
+	}
+
 	private boolean isInvalidPath(String path) {
 		if (path.contains("WEB-INF") || path.contains("META-INF")) {
 			return true;
@@ -158,7 +182,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
@@ -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,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, "UTF-8");
+		}
+		catch (Exception ex) {
+			return "";
+		}
+	}
+
 	/**
 	 * Check whether the given path contains invalid escape sequences.
 	 * @param path the path to validate
@@ -750,7 +774,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
@@ -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();
 	}