Skip to content

Commit 66d1cd5

Browse files
rwinchrwinch
committed
StrictHttpFirewall allows CJKV characters
Closes gh-11264
1 parent 353fac4 commit 66d1cd5

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
@@ -343,6 +343,12 @@ public void getFirewalledRequestWhenContainsUpperboundAsciiThenNoException() {
343343
this.firewall.getFirewalledRequest(this.request);
344344
}
345345

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

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

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

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

0 commit comments

Comments
 (0)