Extract rejectNonPrintableAsciiCharactersInFieldName

Closes gh-11234
spring-projects · May 16, 2022 · 4405cf1 · 4405cf1
1 parent 7eb8a58
commit 4405cf1
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 5 deletions.
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
@@ -431,14 +431,20 @@ public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws
 		if (!isNormalized(request)) {
 			throw new RequestRejectedException("The request was rejected because the URL was not normalized.");
 		}
-		String requestUri = request.getRequestURI();
-		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 StrictFirewalledRequest(request);
 	}
 
+	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;
@@ -526,6 +532,9 @@ private static boolean decodedUrlContains(HttpServletRequest request, String val
 	}
 
 	private static boolean containsOnlyPrintableAsciiCharacters(String uri) {
+		if (uri == null) {
+			return true;
+		}
 		int length = uri.length();
 		for (int i = 0; i < length; i++) {
 			char ch = 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
@@ -364,6 +364,34 @@ public void getFirewalledRequestWhenContainsEncodedNullThenException() {
 				.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
 	}
 
+	@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 while Spring MVC