SEC-1790: Reject redirect locations containing CR or LF.

web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java

@@ -33,7 +33,7 @@ public class DefaultHttpFirewall implements HttpFirewall {
     }
 
     public HttpServletResponse getFirewalledResponse(HttpServletResponse response) {
-        return response;
+        return new FirewalledResponse(response);
     }
 
     /**

web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java

@@ -0,0 +1,27 @@
+package org.springframework.security.web.firewall;
+
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpServletResponseWrapper;
+import java.io.IOException;
+import java.util.regex.Pattern;
+
+/**
+ * @author Luke Taylor
+ */
+class FirewalledResponse extends HttpServletResponseWrapper {
+    Pattern CR_OR_LF = Pattern.compile("\\r|\\n");
+
+    public FirewalledResponse(HttpServletResponse response) {
+        super(response);
+    }
+
+    @Override
+    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()) {
+            throw new IllegalArgumentException("Invalid characters (CR/LF) in redirect location");
+        }
+        super.sendRedirect(location);
+    }
+}

web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java

@@ -0,0 +1,39 @@
+package org.springframework.security.web.firewall;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import org.junit.*;
+import org.springframework.mock.web.MockHttpServletResponse;
+
+/**
+ * @author Luke Taylor
+ */
+public class FirewalledResponseTests {
+
+    @Test
+    public void rejectsRedirectLocationContaingCRLF() throws Exception {
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        FirewalledResponse fwResponse  = new FirewalledResponse(response);
+
+        fwResponse.sendRedirect("/theURL");
+        assertEquals("/theURL", response.getRedirectedUrl());
+
+        try {
+            fwResponse.sendRedirect("/theURL\r\nsomething");
+            fail();
+        } catch (IllegalArgumentException expected) {
+        }
+        try {
+            fwResponse.sendRedirect("/theURL\rsomething");
+            fail();
+        } catch (IllegalArgumentException expected) {
+        }
+
+        try {
+            fwResponse.sendRedirect("/theURL\nsomething");
+            fail();
+        } catch (IllegalArgumentException expected) {
+        }
+    }
+}