Skip to content

Commit 3bfbe30

Browse files
committed
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
1 parent 3296289 commit 3bfbe30

File tree

6 files changed

+72
-14
lines changed

6 files changed

+72
-14
lines changed

Diff for: spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java

+18-5
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ public Mono<Resource> apply(ServerRequest request) {
104104
protected String processPath(String path) {
105105
path = StringUtils.replace(path, "\\", "/");
106106
path = cleanDuplicateSlashes(path);
107-
return cleanLeadingSlash(path);
107+
path = cleanLeadingSlash(path);
108+
return normalizePath(path);
108109
}
109110

110111
private String cleanDuplicateSlashes(String path) {
@@ -146,6 +147,21 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
146147
return (slash ? "/" : "");
147148
}
148149

150+
private static String normalizePath(String path) {
151+
if (path.contains("%")) {
152+
try {
153+
path = URLDecoder.decode(path, StandardCharsets.UTF_8);
154+
}
155+
catch (Exception ex) {
156+
return "";
157+
}
158+
if (path.contains("../")) {
159+
path = StringUtils.cleanPath(path);
160+
}
161+
}
162+
return path;
163+
}
164+
149165
private boolean isInvalidPath(String path) {
150166
if (path.contains("WEB-INF") || path.contains("META-INF")) {
151167
return true;
@@ -156,10 +172,7 @@ private boolean isInvalidPath(String path) {
156172
return true;
157173
}
158174
}
159-
if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) {
160-
return true;
161-
}
162-
return false;
175+
return path.contains("../");
163176
}
164177

165178
/**

Diff for: spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java

+18-2
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,8 @@ private String getResourcePath(ServerWebExchange exchange) {
523523
protected String processPath(String path) {
524524
path = StringUtils.replace(path, "\\", "/");
525525
path = cleanDuplicateSlashes(path);
526-
return cleanLeadingSlash(path);
526+
path = cleanLeadingSlash(path);
527+
return normalizePath(path);
527528
}
528529

529530
private String cleanDuplicateSlashes(String path) {
@@ -565,6 +566,21 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
565566
return (slash ? "/" : "");
566567
}
567568

569+
private static String normalizePath(String path) {
570+
if (path.contains("%")) {
571+
try {
572+
path = URLDecoder.decode(path, StandardCharsets.UTF_8);
573+
}
574+
catch (Exception ex) {
575+
return "";
576+
}
577+
if (path.contains("../")) {
578+
path = StringUtils.cleanPath(path);
579+
}
580+
}
581+
return path;
582+
}
583+
568584
/**
569585
* Check whether the given path contains invalid escape sequences.
570586
* @param path the path to validate
@@ -623,7 +639,7 @@ protected boolean isInvalidPath(String path) {
623639
return true;
624640
}
625641
}
626-
if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) {
642+
if (path.contains("../")) {
627643
if (logger.isWarnEnabled()) {
628644
logger.warn(LogFormatUtils.formatValue(
629645
"Path contains \"../\" after call to StringUtils#cleanPath: [" + path + "]", -1, true));

Diff for: spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java

-2
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,6 @@ void shouldRejectInvalidPath() throws Exception {
670670
testInvalidPath("/../.." + secretPath, handler);
671671
testInvalidPath("/%2E%2E/testsecret/secret.txt", handler);
672672
testInvalidPath("/%2E%2E/testsecret/secret.txt", handler);
673-
testInvalidPath("%2F%2F%2E%2E%2F%2F%2E%2E" + secretPath, handler);
674673
}
675674

676675
private void testInvalidPath(String requestPath, ResourceWebHandler handler) {
@@ -705,7 +704,6 @@ void resolvePathWithTraversal(HttpMethod method) throws Exception {
705704
testResolvePathWithTraversal(method, "/url:" + secretPath);
706705
testResolvePathWithTraversal(method, "////../.." + secretPath);
707706
testResolvePathWithTraversal(method, "/%2E%2E/testsecret/secret.txt");
708-
testResolvePathWithTraversal(method, "%2F%2F%2E%2E%2F%2Ftestsecret/secret.txt");
709707
testResolvePathWithTraversal(method, "url:" + secretPath);
710708

711709
// The following tests fail with a MalformedURLException on Windows

Diff for: spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java

+18-2
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ public Optional<Resource> apply(ServerRequest request) {
105105
protected String processPath(String path) {
106106
path = StringUtils.replace(path, "\\", "/");
107107
path = cleanDuplicateSlashes(path);
108-
return cleanLeadingSlash(path);
108+
path = cleanLeadingSlash(path);
109+
return normalizePath(path);
109110
}
110111

111112
private String cleanDuplicateSlashes(String path) {
@@ -147,6 +148,21 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
147148
return (slash ? "/" : "");
148149
}
149150

151+
private static String normalizePath(String path) {
152+
if (path.contains("%")) {
153+
try {
154+
path = URLDecoder.decode(path, StandardCharsets.UTF_8);
155+
}
156+
catch (Exception ex) {
157+
return "";
158+
}
159+
if (path.contains("../")) {
160+
path = StringUtils.cleanPath(path);
161+
}
162+
}
163+
return path;
164+
}
165+
150166
private boolean isInvalidPath(String path) {
151167
if (path.contains("WEB-INF") || path.contains("META-INF")) {
152168
return true;
@@ -157,7 +173,7 @@ private boolean isInvalidPath(String path) {
157173
return true;
158174
}
159175
}
160-
return path.contains("..") && StringUtils.cleanPath(path).contains("../");
176+
return path.contains("../");
161177
}
162178

163179
private boolean isInvalidEncodedInputPath(String path) {

Diff for: spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java

+18-2
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,8 @@ private static String getPath(HttpServletRequest request) {
682682
protected String processPath(String path) {
683683
path = StringUtils.replace(path, "\\", "/");
684684
path = cleanDuplicateSlashes(path);
685-
return cleanLeadingSlash(path);
685+
path = cleanLeadingSlash(path);
686+
return normalizePath(path);
686687
}
687688

688689
private String cleanDuplicateSlashes(String path) {
@@ -724,6 +725,21 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
724725
return (slash ? "/" : "");
725726
}
726727

728+
private static String normalizePath(String path) {
729+
if (path.contains("%")) {
730+
try {
731+
path = URLDecoder.decode(path, StandardCharsets.UTF_8);
732+
}
733+
catch (Exception ex) {
734+
return "";
735+
}
736+
if (path.contains("../")) {
737+
path = StringUtils.cleanPath(path);
738+
}
739+
}
740+
return path;
741+
}
742+
727743
/**
728744
* Check whether the given path contains invalid escape sequences.
729745
* @param path the path to validate
@@ -783,7 +799,7 @@ protected boolean isInvalidPath(String path) {
783799
return true;
784800
}
785801
}
786-
if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) {
802+
if (path.contains("../")) {
787803
if (logger.isWarnEnabled()) {
788804
logger.warn(LogFormatUtils.formatValue(
789805
"Path contains \"../\" after call to StringUtils#cleanPath: [" + path + "]", -1, true));

Diff for: spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java

-1
Original file line numberDiff line numberDiff line change
@@ -656,7 +656,6 @@ void shouldRejectInvalidPath() throws Exception {
656656
testInvalidPath("/../.." + secretPath);
657657
testInvalidPath("/%2E%2E/testsecret/secret.txt");
658658
testInvalidPath("/%2E%2E/testsecret/secret.txt");
659-
testInvalidPath("%2F%2F%2E%2E%2F%2F%2E%2E" + secretPath);
660659
}
661660

662661
private void testInvalidPath(String requestPath) {

0 commit comments

Comments
 (0)