Skip to content

Commit 0ece0e6

Browse files
committed
Extract rejectNonPrintableAsciiCharactersInFieldName
Closes gh-11234
1 parent 1f01203 commit 0ece0e6

File tree

2 files changed

+44
-0
lines changed

2 files changed

+44
-0
lines changed

web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,13 +344,25 @@ public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws
344344
if (!containsOnlyPrintableAsciiCharacters(requestUri)) {
345345
throw new RequestRejectedException("The requestURI was rejected because it can only contain printable ASCII characters.");
346346
}
347+
rejectNonPrintableAsciiCharactersInFieldName(request.getRequestURI(), "requestURI");
348+
rejectNonPrintableAsciiCharactersInFieldName(request.getServletPath(), "servletPath");
349+
rejectNonPrintableAsciiCharactersInFieldName(request.getPathInfo(), "pathInfo");
350+
rejectNonPrintableAsciiCharactersInFieldName(request.getContextPath(), "contextPath");
351+
347352
return new FirewalledRequest(request) {
348353
@Override
349354
public void reset() {
350355
}
351356
};
352357
}
353358

359+
private void rejectNonPrintableAsciiCharactersInFieldName(String toCheck, String propertyName) {
360+
if (!containsOnlyPrintableAsciiCharacters(toCheck)) {
361+
throw new RequestRejectedException(
362+
String.format("The %s was rejected because it can only contain printable ASCII characters.", propertyName));
363+
}
364+
}
365+
354366
private void rejectForbiddenHttpMethod(HttpServletRequest request) {
355367
if (this.allowedHttpMethods == ALLOW_ANY_HTTP_METHOD) {
356368
return;
@@ -434,6 +446,9 @@ private static boolean decodedUrlContains(HttpServletRequest request, String val
434446
}
435447

436448
private static boolean containsOnlyPrintableAsciiCharacters(String uri) {
449+
if (uri == null) {
450+
return true;
451+
}
437452
int length = uri.length();
438453
for (int i = 0; i < length; i++) {
439454
char c = uri.charAt(i);

web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static org.assertj.core.api.Assertions.assertThatCode;
2020
import static org.assertj.core.api.Assertions.assertThatThrownBy;
21+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
2122
import static org.assertj.core.api.Assertions.fail;
2223

2324
import java.util.Arrays;
@@ -379,6 +380,34 @@ public void getFirewalledRequestWhenExceedsUpperboundAsciiThenException() {
379380

380381
// --- from DefaultHttpFirewallTests ---
381382

383+
@Test
384+
public void getFirewalledRequestWhenContainsLineFeedThenException() {
385+
this.request.setRequestURI("/something\n/");
386+
assertThatExceptionOfType(RequestRejectedException.class)
387+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
388+
}
389+
390+
@Test
391+
public void getFirewalledRequestWhenServletPathContainsLineFeedThenException() {
392+
this.request.setServletPath("/something\n/");
393+
assertThatExceptionOfType(RequestRejectedException.class)
394+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
395+
}
396+
397+
@Test
398+
public void getFirewalledRequestWhenContainsCarriageReturnThenException() {
399+
this.request.setRequestURI("/something\r/");
400+
assertThatExceptionOfType(RequestRejectedException.class)
401+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
402+
}
403+
404+
@Test
405+
public void getFirewalledRequestWhenServletPathContainsCarriageReturnThenException() {
406+
this.request.setServletPath("/something\r/");
407+
assertThatExceptionOfType(RequestRejectedException.class)
408+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
409+
}
410+
382411
/**
383412
* On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on
384413
* /a/b/c because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c

0 commit comments

Comments
 (0)