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
This commit is contained in:
Eddú Meléndez 2016-06-19 17:02:47 +10:00 committed by Rob Winch
parent 13b0ddb7e6
commit 26fa4a4bf0
2 changed files with 49 additions and 8 deletions

View File

@ -22,8 +22,10 @@ import java.util.regex.Pattern;
/** /**
* @author Luke Taylor * @author Luke Taylor
* @author Eddú Meléndez
*/ */
class FirewalledResponse extends HttpServletResponseWrapper { class FirewalledResponse extends HttpServletResponseWrapper {
private static final Pattern CR_OR_LF = Pattern.compile("\\r|\\n"); private static final Pattern CR_OR_LF = Pattern.compile("\\r|\\n");
public FirewalledResponse(HttpServletResponse response) { public FirewalledResponse(HttpServletResponse response) {
@ -34,10 +36,27 @@ class FirewalledResponse extends HttpServletResponseWrapper {
public void sendRedirect(String location) throws IOException { public void sendRedirect(String location) throws IOException {
// TODO: implement pluggable validation, instead of simple blacklisting. // TODO: implement pluggable validation, instead of simple blacklisting.
// SEC-1790. Prevent redirects containing CRLF // SEC-1790. Prevent redirects containing CRLF
if (CR_OR_LF.matcher(location).find()) { validateCRLF("Location", location);
throw new IllegalArgumentException(
"Invalid characters (CR/LF) in redirect location");
}
super.sendRedirect(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 header " + name);
}
}
} }

View File

@ -15,19 +15,21 @@
*/ */
package org.springframework.security.web.firewall; package org.springframework.security.web.firewall;
import org.junit.Test;
import org.springframework.mock.web.MockHttpServletResponse;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail; import static org.assertj.core.api.Assertions.fail;
import org.junit.*;
import org.springframework.mock.web.MockHttpServletResponse;
/** /**
* @author Luke Taylor * @author Luke Taylor
* @author Eddú Meléndez
*/ */
public class FirewalledResponseTests { public class FirewalledResponseTests {
@Test @Test
public void rejectsRedirectLocationContaingCRLF() throws Exception { public void rejectsRedirectLocationContainingCRLF() throws Exception {
MockHttpServletResponse response = new MockHttpServletResponse(); MockHttpServletResponse response = new MockHttpServletResponse();
FirewalledResponse fwResponse = new FirewalledResponse(response); FirewalledResponse fwResponse = new FirewalledResponse(response);
@ -54,4 +56,24 @@ public class FirewalledResponseTests {
catch (IllegalArgumentException expected) { 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) {
}
}
} }