Skip to content

Commit

Permalink
StrictHttpFirewall allows CJKV characters
Browse files Browse the repository at this point in the history
Issue gh-11264
  • Loading branch information
rwinch committed May 17, 2022
1 parent 5155719 commit e0a6a9e
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,18 @@ public class StrictHttpFirewall implements HttpFirewall {

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

private static final List<String> FORBIDDEN_LF = Collections
.unmodifiableList(Arrays.asList("\r", "%0a", "%0A"));

private static final List<String> FORBIDDEN_CR = Collections
.unmodifiableList(Arrays.asList("\n", "%0d", "%0D"));

private static final List<String> FORBIDDEN_LINE_SEPARATOR = Collections
.unmodifiableList(Arrays.asList("\u2028"));

private static final List<String> FORBIDDEN_PARAGRAPH_SEPARATOR = Collections
.unmodifiableList(Arrays.asList("\u2029"));

private Set<String> encodedUrlBlocklist = new HashSet<>();

private Set<String> decodedUrlBlocklist = new HashSet<>();
Expand Down Expand Up @@ -135,10 +147,14 @@ public StrictHttpFirewall() {
urlBlocklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
urlBlocklistsAddAll(FORBIDDEN_BACKSLASH);
urlBlocklistsAddAll(FORBIDDEN_NULL);
urlBlocklistsAddAll(FORBIDDEN_LF);
urlBlocklistsAddAll(FORBIDDEN_CR);

this.encodedUrlBlocklist.add(ENCODED_PERCENT);
this.encodedUrlBlocklist.addAll(FORBIDDEN_ENCODED_PERIOD);
this.decodedUrlBlocklist.add(PERCENT);
this.decodedUrlBlocklist.addAll(FORBIDDEN_LINE_SEPARATOR);
this.decodedUrlBlocklist.addAll(FORBIDDEN_PARAGRAPH_SEPARATOR);
}

/**
Expand Down Expand Up @@ -432,9 +448,6 @@ public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws
throw new RequestRejectedException("The request was rejected because the URL was not normalized.");
}
rejectNonPrintableAsciiCharactersInFieldName(request.getRequestURI(), "requestURI");
rejectNonPrintableAsciiCharactersInFieldName(request.getServletPath(), "servletPath");
rejectNonPrintableAsciiCharactersInFieldName(request.getPathInfo(), "pathInfo");
rejectNonPrintableAsciiCharactersInFieldName(request.getContextPath(), "contextPath");
return new StrictFirewalledRequest(request);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,12 @@ public void getFirewalledRequestWhenContainsUpperboundAsciiThenNoException() {
this.firewall.getFirewalledRequest(this.request);
}

@Test
public void getFirewalledRequestWhenJapaneseCharacterThenNoException() {
this.request.setServletPath("/\u3042");
this.firewall.getFirewalledRequest(this.request);
}

@Test
public void getFirewalledRequestWhenExceedsUpperboundAsciiThenException() {
this.request.setRequestURI("/\u007f");
Expand All @@ -364,6 +370,20 @@ public void getFirewalledRequestWhenContainsEncodedNullThenException() {
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
}

@Test
public void getFirewalledRequestWhenContainsLowercaseEncodedLineFeedThenException() {
this.request.setRequestURI("/something%0a/");
assertThatExceptionOfType(RequestRejectedException.class)
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
}

@Test
public void getFirewalledRequestWhenContainsUppercaseEncodedLineFeedThenException() {
this.request.setRequestURI("/something%0A/");
assertThatExceptionOfType(RequestRejectedException.class)
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
}

@Test
public void getFirewalledRequestWhenContainsLineFeedThenException() {
this.request.setRequestURI("/something\n/");
Expand All @@ -378,6 +398,20 @@ public void getFirewalledRequestWhenServletPathContainsLineFeedThenException() {
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
}

@Test
public void getFirewalledRequestWhenContainsLowercaseEncodedCarriageReturnThenException() {
this.request.setRequestURI("/something%0d/");
assertThatExceptionOfType(RequestRejectedException.class)
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
}

@Test
public void getFirewalledRequestWhenContainsUppercaseEncodedCarriageReturnThenException() {
this.request.setRequestURI("/something%0D/");
assertThatExceptionOfType(RequestRejectedException.class)
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
}

@Test
public void getFirewalledRequestWhenContainsCarriageReturnThenException() {
this.request.setRequestURI("/something\r/");
Expand All @@ -392,6 +426,20 @@ public void getFirewalledRequestWhenServletPathContainsCarriageReturnThenExcepti
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
}

@Test
public void getFirewalledRequestWhenServletPathContainsLineSeparatorThenException() {
this.request.setServletPath("/something\u2028/");
assertThatExceptionOfType(RequestRejectedException.class)
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
}

@Test
public void getFirewalledRequestWhenServletPathContainsParagraphSeparatorThenException() {
this.request.setServletPath("/something\u2029/");
assertThatExceptionOfType(RequestRejectedException.class)
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
}

/**
* On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on /a/b/c
* because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c while Spring MVC
Expand Down

0 comments on commit e0a6a9e

Please sign in to comment.