From 302dede75e8af5e920f637926a3283cf8be289bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Edd=C3=BA=20Mel=C3=A9ndez?= <eddu.melendez@gmail.com>
Date: Sun, 19 Jun 2016 17:02:47 +1000
Subject: [PATCH] Prevent HTTP response splitting

Evaluate if http header value contains CR/LF.

Reference: https://www.owasp.org/index.php/HTTP_Response_Splitting

Fixes gh-3910
---
 .../web/firewall/FirewalledResponse.java      | 25 ++++++++++++++--
 .../web/firewall/FirewalledResponseTests.java | 30 ++++++++++++++++---
 2 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java b/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java
index 028acc0556..640172012d 100644
--- a/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java
+++ b/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java
@@ -22,8 +22,10 @@
 
 /**
  * @author Luke Taylor
+ * @author Eddú Meléndez
  */
 class FirewalledResponse extends HttpServletResponseWrapper {
+
 	private static final Pattern CR_OR_LF = Pattern.compile("\\r|\\n");
 
 	public FirewalledResponse(HttpServletResponse response) {
@@ -34,10 +36,27 @@ public FirewalledResponse(HttpServletResponse response) {
 	public void sendRedirect(String location) throws IOException {
 		// TODO: implement pluggable validation, instead of simple blacklisting.
 		// SEC-1790. Prevent redirects containing CRLF
-		if (CR_OR_LF.matcher(location).find()) {
+		validateCRLF("Location", location);
+		super.sendRedirect(location);
+	}
+
+	@Override
+	public void setHeader(String name, String value) {
+		validateCRLF(name, value);
+		super.setHeader(name, value);
+	}
+
+	@Override
+	public void addHeader(String name, String value) {
+		validateCRLF(name, value);
+		super.addHeader(name, value);
+	}
+
+	private void validateCRLF(String name, String value) {
+		if (CR_OR_LF.matcher(value).find()) {
 			throw new IllegalArgumentException(
-					"Invalid characters (CR/LF) in redirect location");
+					"Invalid characters (CR/LF) in header " + name);
 		}
-		super.sendRedirect(location);
 	}
+
 }
diff --git a/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java b/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java
index 7181e839c1..c1af03ddc9 100644
--- a/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java
+++ b/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java
@@ -15,19 +15,21 @@
  */
 package org.springframework.security.web.firewall;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.fail;
+import org.junit.Test;
 
-import org.junit.*;
 import org.springframework.mock.web.MockHttpServletResponse;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
+
 /**
  * @author Luke Taylor
+ * @author Eddú Meléndez
  */
 public class FirewalledResponseTests {
 
 	@Test
-	public void rejectsRedirectLocationContaingCRLF() throws Exception {
+	public void rejectsRedirectLocationContainingCRLF() throws Exception {
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		FirewalledResponse fwResponse = new FirewalledResponse(response);
 
@@ -54,4 +56,24 @@ public void rejectsRedirectLocationContaingCRLF() throws Exception {
 		catch (IllegalArgumentException expected) {
 		}
 	}
+
+	@Test
+	public void rejectHeaderContainingCRLF() {
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		FirewalledResponse fwResponse = new FirewalledResponse(response);
+
+		try {
+			fwResponse.addHeader("foo", "abc\r\nContent-Length:100");
+			fail("IllegalArgumentException should have thrown");
+		}
+		catch (IllegalArgumentException expected) {
+		}
+		try {
+			fwResponse.setHeader("foo", "abc\r\nContent-Length:100");
+			fail("IllegalArgumentException should have thrown");
+		}
+		catch (IllegalArgumentException expected) {
+		}
+	}
+
 }