Skip to content

Commit 27f031d

Browse files
abhinaynagpalabhinaynagpal
committed
RouterFunctions resource handling improvements
Cherry pick of : a) Use encoded resource path instead of input path validation in spring-webflux spring-projects#33568 b) Updates to resource handling for functional endpoints spring-projects#33434 This Addresses CVE-2024-38819 and CVE-2024-38816
1 parent bda04f0 commit 27f031d

File tree

2 files changed

+199
-24
lines changed

2 files changed

+199
-24
lines changed

‎spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java

Lines changed: 97 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
import java.io.IOException;
2020
import java.io.UncheckedIOException;
21+
import java.io.UnsupportedEncodingException;
22+
import java.net.URLDecoder;
2123
import java.nio.charset.StandardCharsets;
2224
import java.util.function.Function;
2325

@@ -30,6 +32,7 @@
3032
import org.springframework.util.Assert;
3133
import org.springframework.util.ResourceUtils;
3234
import org.springframework.util.StringUtils;
35+
import org.springframework.web.util.UriUtils;
3336
import org.springframework.web.util.pattern.PathPattern;
3437
import org.springframework.web.util.pattern.PathPatternParser;
3538

@@ -63,13 +66,17 @@ public Mono<Resource> apply(ServerRequest request) {
6366

6467
pathContainer = this.pattern.extractPathWithinPattern(pathContainer);
6568
String path = processPath(pathContainer.value());
66-
if (path.contains("%")) {
67-
path = StringUtils.uriDecode(path, StandardCharsets.UTF_8);
69+
if (!StringUtils.hasText(path) || isInvalidPath(path)) {
70+
return Mono.empty();
6871
}
69-
if (!StringUtils.hasLength(path) || isInvalidPath(path)) {
72+
if (isInvalidEncodedInputPath(path)) {
7073
return Mono.empty();
7174
}
7275

76+
if (!(this.location instanceof UrlResource)) {
77+
path = UriUtils.decode(path, StandardCharsets.UTF_8);
78+
}
79+
7380
try {
7481
Resource resource = this.location.createRelative(path);
7582
if (resource.isReadable() && isResourceUnderLocation(resource)) {
@@ -84,7 +91,47 @@ public Mono<Resource> apply(ServerRequest request) {
8491
}
8592
}
8693

94+
/**
95+
* Process the given resource path.
96+
* <p>The default implementation replaces:
97+
* <ul>
98+
* <li>Backslash with forward slash.
99+
* <li>Duplicate occurrences of slash with a single slash.
100+
* <li>Any combination of leading slash and control characters (00-1F and 7F)
101+
* with a single "/" or "". For example {@code " / // foo/bar"}
102+
* becomes {@code "/foo/bar"}.
103+
* </ul>
104+
*/
87105
private String processPath(String path) {
106+
path = StringUtils.replace(path, "\\", "/");
107+
path = cleanDuplicateSlashes(path);
108+
return cleanLeadingSlash(path);
109+
}
110+
111+
private String cleanDuplicateSlashes(String path) {
112+
StringBuilder sb = null;
113+
char prev = 0;
114+
for (int i = 0; i < path.length(); i++) {
115+
char curr = path.charAt(i);
116+
try {
117+
if (curr == '/' && prev == '/') {
118+
if (sb == null) {
119+
sb = new StringBuilder(path.substring(0, i));
120+
}
121+
continue;
122+
}
123+
if (sb != null) {
124+
sb.append(path.charAt(i));
125+
}
126+
}
127+
finally {
128+
prev = curr;
129+
}
130+
}
131+
return (sb != null ? sb.toString() : path);
132+
}
133+
134+
private String cleanLeadingSlash(String path) {
88135
boolean slash = false;
89136
for (int i = 0; i < path.length(); i++) {
90137
if (path.charAt(i) == '/') {
@@ -94,8 +141,7 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
94141
if (i == 0 || (i == 1 && slash)) {
95142
return path;
96143
}
97-
path = slash ? "/" + path.substring(i) : path.substring(i);
98-
return path;
144+
return (slash ? "/" + path.substring(i) : path.substring(i));
99145
}
100146
}
101147
return (slash ? "/" : "");
@@ -111,8 +157,33 @@ private boolean isInvalidPath(String path) {
111157
return true;
112158
}
113159
}
114-
if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) {
115-
return true;
160+
return path.contains("..") && StringUtils.cleanPath(path).contains("../");
161+
}
162+
163+
/**
164+
* Check whether the given path contains invalid escape sequences.
165+
* @param path the path to validate
166+
* @return {@code true} if the path is invalid, {@code false} otherwise
167+
*/
168+
private boolean isInvalidEncodedInputPath(String path) {
169+
if (path.contains("%")) {
170+
try {
171+
// Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars
172+
String decodedPath = URLDecoder.decode(path, StandardCharsets.UTF_8.name());
173+
if (isInvalidPath(decodedPath)) {
174+
return true;
175+
}
176+
decodedPath = processPath(decodedPath);
177+
if (isInvalidPath(decodedPath)) {
178+
return true;
179+
}
180+
}
181+
catch (IllegalArgumentException ex) {
182+
// May not be possible to decode...
183+
}
184+
catch (UnsupportedEncodingException ex) {
185+
// May not be possible to decode...
186+
}
116187
}
117188
return false;
118189
}
@@ -142,15 +213,27 @@ else if (resource instanceof ClassPathResource) {
142213
return true;
143214
}
144215
locationPath = (locationPath.endsWith("/") || locationPath.isEmpty() ? locationPath : locationPath + "/");
145-
if (!resourcePath.startsWith(locationPath)) {
146-
return false;
147-
}
148-
if (resourcePath.contains("%") && StringUtils.uriDecode(resourcePath, StandardCharsets.UTF_8).contains("../")) {
149-
return false;
150-
}
151-
return true;
216+
return (resourcePath.startsWith(locationPath) && !isInvalidEncodedResourcePath(resourcePath));
152217
}
153218

219+
private boolean isInvalidEncodedResourcePath(String resourcePath) {
220+
if (resourcePath.contains("%")) {
221+
// Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars...
222+
try {
223+
String decodedPath = URLDecoder.decode(resourcePath, StandardCharsets.UTF_8.name());
224+
if (decodedPath.contains("../") || decodedPath.contains("..\\")) {
225+
return true;
226+
}
227+
}
228+
catch (IllegalArgumentException ex) {
229+
// May not be possible to decode...
230+
}
231+
catch (UnsupportedEncodingException ex) {
232+
// May not be possible to decode...
233+
}
234+
}
235+
return false;
236+
}
154237

155238
@Override
156239
public String toString() {

‎spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java

Lines changed: 102 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
import java.io.IOException;
2020
import java.io.UncheckedIOException;
21+
import java.io.UnsupportedEncodingException;
22+
import java.net.URLDecoder;
2123
import java.nio.charset.StandardCharsets;
2224
import java.util.Optional;
2325
import java.util.function.Function;
@@ -29,6 +31,8 @@
2931
import org.springframework.util.Assert;
3032
import org.springframework.util.ResourceUtils;
3133
import org.springframework.util.StringUtils;
34+
import org.springframework.web.context.support.ServletContextResource;
35+
import org.springframework.web.util.UriUtils;
3236
import org.springframework.web.util.pattern.PathPattern;
3337
import org.springframework.web.util.pattern.PathPatternParser;
3438

@@ -62,12 +66,15 @@ public Optional<Resource> apply(ServerRequest request) {
6266

6367
pathContainer = this.pattern.extractPathWithinPattern(pathContainer);
6468
String path = processPath(pathContainer.value());
65-
if (path.contains("%")) {
66-
path = StringUtils.uriDecode(path, StandardCharsets.UTF_8);
69+
if (!StringUtils.hasText(path) || isInvalidPath(path)) {
70+
return Optional.empty();
6771
}
68-
if (!StringUtils.hasLength(path) || isInvalidPath(path)) {
72+
if (isInvalidEncodedInputPath(path)) {
6973
return Optional.empty();
7074
}
75+
if (!(this.location instanceof UrlResource)) {
76+
path = UriUtils.decode(path, StandardCharsets.UTF_8);
77+
}
7178

7279
try {
7380
Resource resource = this.location.createRelative(path);
@@ -83,7 +90,47 @@ public Optional<Resource> apply(ServerRequest request) {
8390
}
8491
}
8592

93+
/**
94+
* Process the given resource path.
95+
* <p>The default implementation replaces:
96+
* <ul>
97+
* <li>Backslash with forward slash.
98+
* <li>Duplicate occurrences of slash with a single slash.
99+
* <li>Any combination of leading slash and control characters (00-1F and 7F)
100+
* with a single "/" or "". For example {@code " / // foo/bar"}
101+
* becomes {@code "/foo/bar"}.
102+
* </ul>
103+
*/
86104
private String processPath(String path) {
105+
path = StringUtils.replace(path, "\\", "/");
106+
path = cleanDuplicateSlashes(path);
107+
return cleanLeadingSlash(path);
108+
}
109+
110+
private String cleanDuplicateSlashes(String path) {
111+
StringBuilder sb = null;
112+
char prev = 0;
113+
for (int i = 0; i < path.length(); i++) {
114+
char curr = path.charAt(i);
115+
try {
116+
if (curr == '/' && prev == '/') {
117+
if (sb == null) {
118+
sb = new StringBuilder(path.substring(0, i));
119+
}
120+
continue;
121+
}
122+
if (sb != null) {
123+
sb.append(path.charAt(i));
124+
}
125+
}
126+
finally {
127+
prev = curr;
128+
}
129+
}
130+
return (sb != null ? sb.toString() : path);
131+
}
132+
133+
private String cleanLeadingSlash(String path) {
87134
boolean slash = false;
88135
for (int i = 0; i < path.length(); i++) {
89136
if (path.charAt(i) == '/') {
@@ -93,8 +140,7 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
93140
if (i == 0 || (i == 1 && slash)) {
94141
return path;
95142
}
96-
path = slash ? "/" + path.substring(i) : path.substring(i);
97-
return path;
143+
return (slash ? "/" + path.substring(i) : path.substring(i));
98144
}
99145
}
100146
return (slash ? "/" : "");
@@ -113,6 +159,34 @@ private boolean isInvalidPath(String path) {
113159
return path.contains("..") && StringUtils.cleanPath(path).contains("../");
114160
}
115161

162+
/**
163+
* Check whether the given path contains invalid escape sequences.
164+
* @param path the path to validate
165+
* @return {@code true} if the path is invalid, {@code false} otherwise
166+
*/
167+
private boolean isInvalidEncodedInputPath(String path) {
168+
if (path.contains("%")) {
169+
try {
170+
// Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars
171+
String decodedPath = URLDecoder.decode(path, StandardCharsets.UTF_8.name());
172+
if (isInvalidPath(decodedPath)) {
173+
return true;
174+
}
175+
decodedPath = processPath(decodedPath);
176+
if (isInvalidPath(decodedPath)) {
177+
return true;
178+
}
179+
}
180+
catch (IllegalArgumentException ex) {
181+
// May not be possible to decode...
182+
}
183+
catch (UnsupportedEncodingException ex) {
184+
// May not be possible to decode...
185+
}
186+
}
187+
return false;
188+
}
189+
116190
private boolean isResourceUnderLocation(Resource resource) throws IOException {
117191
if (resource.getClass() != this.location.getClass()) {
118192
return false;
@@ -129,6 +203,10 @@ else if (resource instanceof ClassPathResource) {
129203
resourcePath = ((ClassPathResource) resource).getPath();
130204
locationPath = StringUtils.cleanPath(((ClassPathResource) this.location).getPath());
131205
}
206+
else if (resource instanceof ServletContextResource) {
207+
resourcePath = ((ServletContextResource) resource).getPath();
208+
locationPath = StringUtils.cleanPath(((ServletContextResource) this.location).getPath());
209+
}
132210
else {
133211
resourcePath = resource.getURL().getPath();
134212
locationPath = StringUtils.cleanPath(this.location.getURL().getPath());
@@ -138,13 +216,27 @@ else if (resource instanceof ClassPathResource) {
138216
return true;
139217
}
140218
locationPath = (locationPath.endsWith("/") || locationPath.isEmpty() ? locationPath : locationPath + "/");
141-
if (!resourcePath.startsWith(locationPath)) {
142-
return false;
143-
}
144-
return !resourcePath.contains("%") ||
145-
!StringUtils.uriDecode(resourcePath, StandardCharsets.UTF_8).contains("../");
219+
return (resourcePath.startsWith(locationPath) && !isInvalidEncodedResourcePath(resourcePath));
146220
}
147221

222+
private boolean isInvalidEncodedResourcePath(String resourcePath) {
223+
if (resourcePath.contains("%")) {
224+
// Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars...
225+
try {
226+
String decodedPath = URLDecoder.decode(resourcePath, StandardCharsets.UTF_8.name());
227+
if (decodedPath.contains("../") || decodedPath.contains("..\\")) {
228+
return true;
229+
}
230+
}
231+
catch (IllegalArgumentException ex) {
232+
// May not be possible to decode...
233+
}
234+
catch (UnsupportedEncodingException ex) {
235+
// May not be possible to decode...
236+
}
237+
}
238+
return false;
239+
}
148240

149241
@Override
150242
public String toString() {

0 commit comments

Comments
 (0)