diff --git a/web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java b/web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java index a40fa435dc..5592e9d050 100644 --- a/web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java +++ b/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); } /** 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 new file mode 100644 index 0000000000..95364af808 --- /dev/null +++ b/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); + } +} 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 new file mode 100644 index 0000000000..66a2268273 --- /dev/null +++ b/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) { + } + } +}