Skip to content

Commit 5b0dab5

Browse files
rwinchrwinch
committed
StrictHttpFirewall allows CJKV characters
Closes gh-11264
1 parent 7f5c319 commit 5b0dab5

File tree

2 files changed

+200
-3
lines changed

2 files changed

+200
-3
lines changed

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

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,15 @@ public class StrictHttpFirewall implements HttpFirewall {
107107

108108
private static final List<String> FORBIDDEN_NULL = Collections.unmodifiableList(Arrays.asList("\0", "%00"));
109109

110+
private static final List<String> FORBIDDEN_LF = Collections.unmodifiableList(Arrays.asList("\n", "%0a", "%0A"));
111+
112+
private static final List<String> FORBIDDEN_CR = Collections.unmodifiableList(Arrays.asList("\r", "%0d", "%0D"));
113+
114+
private static final List<String> FORBIDDEN_LINE_SEPARATOR = Collections.unmodifiableList(Arrays.asList("\u2028"));
115+
116+
private static final List<String> FORBIDDEN_PARAGRAPH_SEPARATOR = Collections
117+
.unmodifiableList(Arrays.asList("\u2029"));
118+
110119
private Set<String> encodedUrlBlocklist = new HashSet<>();
111120

112121
private Set<String> decodedUrlBlocklist = new HashSet<>();
@@ -135,10 +144,14 @@ public StrictHttpFirewall() {
135144
urlBlocklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
136145
urlBlocklistsAddAll(FORBIDDEN_BACKSLASH);
137146
urlBlocklistsAddAll(FORBIDDEN_NULL);
147+
urlBlocklistsAddAll(FORBIDDEN_LF);
148+
urlBlocklistsAddAll(FORBIDDEN_CR);
138149

139150
this.encodedUrlBlocklist.add(ENCODED_PERCENT);
140151
this.encodedUrlBlocklist.addAll(FORBIDDEN_ENCODED_PERIOD);
141152
this.decodedUrlBlocklist.add(PERCENT);
153+
this.decodedUrlBlocklist.addAll(FORBIDDEN_LINE_SEPARATOR);
154+
this.decodedUrlBlocklist.addAll(FORBIDDEN_PARAGRAPH_SEPARATOR);
142155
}
143156

144157
/**
@@ -345,6 +358,69 @@ public void setAllowUrlEncodedPercent(boolean allowUrlEncodedPercent) {
345358
}
346359
}
347360

361+
/**
362+
* Determines if a URL encoded Carriage Return is allowed in the path or not. The
363+
* default is not to allow this behavior because it is a frequent source of security
364+
* exploits.
365+
* @param allowUrlEncodedCarriageReturn if URL encoded Carriage Return is allowed in
366+
* the URL or not. Default is false.
367+
*/
368+
public void setAllowUrlEncodedCarriageReturn(boolean allowUrlEncodedCarriageReturn) {
369+
if (allowUrlEncodedCarriageReturn) {
370+
urlBlocklistsRemoveAll(FORBIDDEN_CR);
371+
}
372+
else {
373+
urlBlocklistsAddAll(FORBIDDEN_CR);
374+
}
375+
}
376+
377+
/**
378+
* Determines if a URL encoded Line Feed is allowed in the path or not. The default is
379+
* not to allow this behavior because it is a frequent source of security exploits.
380+
* @param allowUrlEncodedLineFeed if URL encoded Line Feed is allowed in the URL or
381+
* not. Default is false.
382+
*/
383+
public void setAllowUrlEncodedLineFeed(boolean allowUrlEncodedLineFeed) {
384+
if (allowUrlEncodedLineFeed) {
385+
urlBlocklistsRemoveAll(FORBIDDEN_LF);
386+
}
387+
else {
388+
urlBlocklistsAddAll(FORBIDDEN_LF);
389+
}
390+
}
391+
392+
/**
393+
* Determines if a URL encoded paragraph separator is allowed in the path or not. The
394+
* default is not to allow this behavior because it is a frequent source of security
395+
* exploits.
396+
* @param allowUrlEncodedParagraphSeparator if URL encoded paragraph separator is
397+
* allowed in the URL or not. Default is false.
398+
*/
399+
public void setAllowUrlEncodedParagraphSeparator(boolean allowUrlEncodedParagraphSeparator) {
400+
if (allowUrlEncodedParagraphSeparator) {
401+
this.decodedUrlBlocklist.removeAll(FORBIDDEN_PARAGRAPH_SEPARATOR);
402+
}
403+
else {
404+
this.decodedUrlBlocklist.addAll(FORBIDDEN_PARAGRAPH_SEPARATOR);
405+
}
406+
}
407+
408+
/**
409+
* Determines if a URL encoded line separator is allowed in the path or not. The
410+
* default is not to allow this behavior because it is a frequent source of security
411+
* exploits.
412+
* @param allowUrlEncodedLineSeparator if URL encoded line separator is allowed in the
413+
* URL or not. Default is false.
414+
*/
415+
public void setAllowUrlEncodedLineSeparator(boolean allowUrlEncodedLineSeparator) {
416+
if (allowUrlEncodedLineSeparator) {
417+
this.decodedUrlBlocklist.removeAll(FORBIDDEN_LINE_SEPARATOR);
418+
}
419+
else {
420+
this.decodedUrlBlocklist.addAll(FORBIDDEN_LINE_SEPARATOR);
421+
}
422+
}
423+
348424
/**
349425
* <p>
350426
* Determines which header names should be allowed. The default is to reject header
@@ -432,9 +508,6 @@ public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws
432508
throw new RequestRejectedException("The request was rejected because the URL was not normalized.");
433509
}
434510
rejectNonPrintableAsciiCharactersInFieldName(request.getRequestURI(), "requestURI");
435-
rejectNonPrintableAsciiCharactersInFieldName(request.getServletPath(), "servletPath");
436-
rejectNonPrintableAsciiCharactersInFieldName(request.getPathInfo(), "pathInfo");
437-
rejectNonPrintableAsciiCharactersInFieldName(request.getContextPath(), "contextPath");
438511
return new StrictFirewalledRequest(request);
439512
}
440513

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

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,12 @@ public void getFirewalledRequestWhenContainsUpperboundAsciiThenNoException() {
342342
this.firewall.getFirewalledRequest(this.request);
343343
}
344344

345+
@Test
346+
public void getFirewalledRequestWhenJapaneseCharacterThenNoException() {
347+
this.request.setServletPath("/\u3042");
348+
this.firewall.getFirewalledRequest(this.request);
349+
}
350+
345351
@Test
346352
public void getFirewalledRequestWhenExceedsUpperboundAsciiThenException() {
347353
this.request.setRequestURI("/\u007f");
@@ -363,6 +369,20 @@ public void getFirewalledRequestWhenContainsEncodedNullThenException() {
363369
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
364370
}
365371

372+
@Test
373+
public void getFirewalledRequestWhenContainsLowercaseEncodedLineFeedThenException() {
374+
this.request.setRequestURI("/something%0a/");
375+
assertThatExceptionOfType(RequestRejectedException.class)
376+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
377+
}
378+
379+
@Test
380+
public void getFirewalledRequestWhenContainsUppercaseEncodedLineFeedThenException() {
381+
this.request.setRequestURI("/something%0A/");
382+
assertThatExceptionOfType(RequestRejectedException.class)
383+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
384+
}
385+
366386
@Test
367387
public void getFirewalledRequestWhenContainsLineFeedThenException() {
368388
this.request.setRequestURI("/something\n/");
@@ -377,6 +397,20 @@ public void getFirewalledRequestWhenServletPathContainsLineFeedThenException() {
377397
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
378398
}
379399

400+
@Test
401+
public void getFirewalledRequestWhenContainsLowercaseEncodedCarriageReturnThenException() {
402+
this.request.setRequestURI("/something%0d/");
403+
assertThatExceptionOfType(RequestRejectedException.class)
404+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
405+
}
406+
407+
@Test
408+
public void getFirewalledRequestWhenContainsUppercaseEncodedCarriageReturnThenException() {
409+
this.request.setRequestURI("/something%0D/");
410+
assertThatExceptionOfType(RequestRejectedException.class)
411+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
412+
}
413+
380414
@Test
381415
public void getFirewalledRequestWhenContainsCarriageReturnThenException() {
382416
this.request.setRequestURI("/something\r/");
@@ -391,6 +425,96 @@ public void getFirewalledRequestWhenServletPathContainsCarriageReturnThenExcepti
391425
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
392426
}
393427

428+
@Test
429+
public void getFirewalledRequestWhenServletPathContainsLineSeparatorThenException() {
430+
this.request.setServletPath("/something\u2028/");
431+
assertThatExceptionOfType(RequestRejectedException.class)
432+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
433+
}
434+
435+
@Test
436+
public void getFirewalledRequestWhenServletPathContainsParagraphSeparatorThenException() {
437+
this.request.setServletPath("/something\u2029/");
438+
assertThatExceptionOfType(RequestRejectedException.class)
439+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
440+
}
441+
442+
@Test
443+
public void getFirewalledRequestWhenContainsLowercaseEncodedLineFeedAndAllowedThenNoException() {
444+
this.firewall.setAllowUrlEncodedLineFeed(true);
445+
this.request.setRequestURI("/something%0a/");
446+
this.firewall.getFirewalledRequest(this.request);
447+
}
448+
449+
@Test
450+
public void getFirewalledRequestWhenContainsUppercaseEncodedLineFeedAndAllowedThenNoException() {
451+
this.firewall.setAllowUrlEncodedLineFeed(true);
452+
this.request.setRequestURI("/something%0A/");
453+
this.firewall.getFirewalledRequest(this.request);
454+
}
455+
456+
@Test
457+
public void getFirewalledRequestWhenContainsLineFeedAndAllowedThenException() {
458+
this.firewall.setAllowUrlEncodedLineFeed(true);
459+
this.request.setRequestURI("/something\n/");
460+
// Expected an error because the line feed is decoded in an encoded part of the
461+
// URL
462+
assertThatExceptionOfType(RequestRejectedException.class)
463+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
464+
}
465+
466+
@Test
467+
public void getFirewalledRequestWhenServletPathContainsLineFeedAndAllowedThenNoException() {
468+
this.firewall.setAllowUrlEncodedLineFeed(true);
469+
this.request.setServletPath("/something\n/");
470+
this.firewall.getFirewalledRequest(this.request);
471+
}
472+
473+
@Test
474+
public void getFirewalledRequestWhenContainsLowercaseEncodedCarriageReturnAndAllowedThenNoException() {
475+
this.firewall.setAllowUrlEncodedCarriageReturn(true);
476+
this.request.setRequestURI("/something%0d/");
477+
this.firewall.getFirewalledRequest(this.request);
478+
}
479+
480+
@Test
481+
public void getFirewalledRequestWhenContainsUppercaseEncodedCarriageReturnAndAllowedThenNoException() {
482+
this.firewall.setAllowUrlEncodedCarriageReturn(true);
483+
this.request.setRequestURI("/something%0D/");
484+
this.firewall.getFirewalledRequest(this.request);
485+
}
486+
487+
@Test
488+
public void getFirewalledRequestWhenContainsCarriageReturnAndAllowedThenNoException() {
489+
this.firewall.setAllowUrlEncodedCarriageReturn(true);
490+
this.request.setRequestURI("/something\r/");
491+
// Expected an error because the carriage return is decoded in an encoded part of
492+
// the URL
493+
assertThatExceptionOfType(RequestRejectedException.class)
494+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
495+
}
496+
497+
@Test
498+
public void getFirewalledRequestWhenServletPathContainsCarriageReturnAndAllowedThenNoException() {
499+
this.firewall.setAllowUrlEncodedCarriageReturn(true);
500+
this.request.setServletPath("/something\r/");
501+
this.firewall.getFirewalledRequest(this.request);
502+
}
503+
504+
@Test
505+
public void getFirewalledRequestWhenServletPathContainsLineSeparatorAndAllowedThenNoException() {
506+
this.firewall.setAllowUrlEncodedLineSeparator(true);
507+
this.request.setServletPath("/something\u2028/");
508+
this.firewall.getFirewalledRequest(this.request);
509+
}
510+
511+
@Test
512+
public void getFirewalledRequestWhenServletPathContainsParagraphSeparatorAndAllowedThenNoException() {
513+
this.firewall.setAllowUrlEncodedParagraphSeparator(true);
514+
this.request.setServletPath("/something\u2029/");
515+
this.firewall.getFirewalledRequest(this.request);
516+
}
517+
394518
/**
395519
* On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on /a/b/c
396520
* because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c while Spring MVC

0 commit comments

Comments
 (0)