From 0ece0e601287f8f0bdaae3ce53bd3567aecf8d94 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Mon, 16 May 2022 09:46:33 -0500 Subject: [PATCH] Extract rejectNonPrintableAsciiCharactersInFieldName Closes gh-11234 --- .../web/firewall/StrictHttpFirewall.java | 15 ++++++++++ .../web/firewall/StrictHttpFirewallTests.java | 29 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java b/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java index 25dfe2b3a1..aef2a7b533 100644 --- a/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java +++ b/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java @@ -344,6 +344,11 @@ public class StrictHttpFirewall implements HttpFirewall { if (!containsOnlyPrintableAsciiCharacters(requestUri)) { throw new RequestRejectedException("The requestURI was rejected because it can only contain printable ASCII characters."); } + rejectNonPrintableAsciiCharactersInFieldName(request.getRequestURI(), "requestURI"); + rejectNonPrintableAsciiCharactersInFieldName(request.getServletPath(), "servletPath"); + rejectNonPrintableAsciiCharactersInFieldName(request.getPathInfo(), "pathInfo"); + rejectNonPrintableAsciiCharactersInFieldName(request.getContextPath(), "contextPath"); + return new FirewalledRequest(request) { @Override public void reset() { @@ -351,6 +356,13 @@ public class StrictHttpFirewall implements HttpFirewall { }; } + private void rejectNonPrintableAsciiCharactersInFieldName(String toCheck, String propertyName) { + if (!containsOnlyPrintableAsciiCharacters(toCheck)) { + throw new RequestRejectedException( + String.format("The %s was rejected because it can only contain printable ASCII characters.", propertyName)); + } + } + private void rejectForbiddenHttpMethod(HttpServletRequest request) { if (this.allowedHttpMethods == ALLOW_ANY_HTTP_METHOD) { return; @@ -434,6 +446,9 @@ public class StrictHttpFirewall implements HttpFirewall { } private static boolean containsOnlyPrintableAsciiCharacters(String uri) { + if (uri == null) { + return true; + } int length = uri.length(); for (int i = 0; i < length; i++) { char c = uri.charAt(i); diff --git a/web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java b/web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java index 8695ea7fa6..d91818af2a 100644 --- a/web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java +++ b/web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java @@ -18,6 +18,7 @@ package org.springframework.security.web.firewall; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.fail; import java.util.Arrays; @@ -379,6 +380,34 @@ public class StrictHttpFirewallTests { // --- from DefaultHttpFirewallTests --- + @Test + public void getFirewalledRequestWhenContainsLineFeedThenException() { + this.request.setRequestURI("/something\n/"); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + + @Test + public void getFirewalledRequestWhenServletPathContainsLineFeedThenException() { + this.request.setServletPath("/something\n/"); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + + @Test + public void getFirewalledRequestWhenContainsCarriageReturnThenException() { + this.request.setRequestURI("/something\r/"); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + + @Test + public void getFirewalledRequestWhenServletPathContainsCarriageReturnThenException() { + this.request.setServletPath("/something\r/"); + 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