Author: mgrigorov
Date: Wed Nov 9 13:43:46 2011
New Revision: 1199754
URL: http://svn.apache.org/viewvc?rev=1199754&view=rev
Log:
WICKET-4196 Accessing Wicket through AJP makes Wicket vulnerable to HTTP
Response Splitting Attack
Clean header name/value from \n and \r before writing them in the
HttpServletResponse
Modified:
wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebResponse.java
wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebResponseTest.java
Modified:
wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebResponse.java
URL:
http://svn.apache.org/viewvc/wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebResponse.java?rev=1199754&r1=1199753&r2=1199754&view=diff
==============================================================================
---
wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebResponse.java
(original)
+++
wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebResponse.java
Wed Nov 9 13:43:46 2011
@@ -76,26 +76,26 @@ public class ServletWebResponse extends
@Override
public void setContentType(String mimeType)
{
- httpServletResponse.setContentType(mimeType);
+ httpServletResponse.setContentType(sanitize(mimeType));
}
@Override
public void setDateHeader(String name, Time date)
{
Args.notNull(date, "date");
- httpServletResponse.setDateHeader(name, date.getMilliseconds());
+ httpServletResponse.setDateHeader(sanitize(name),
date.getMilliseconds());
}
@Override
public void setHeader(String name, String value)
{
- httpServletResponse.setHeader(name, value);
+ httpServletResponse.setHeader(sanitize(name), sanitize(value));
}
@Override
public void addHeader(String name, String value)
{
- httpServletResponse.addHeader(name, value);
+ httpServletResponse.addHeader(sanitize(name), sanitize(value));
}
@Override
@@ -155,7 +155,7 @@ public class ServletWebResponse extends
}
else
{
- httpServletResponse.sendError(sc, msg);
+ httpServletResponse.sendError(sc,
sanitize(msg));
}
}
catch (IOException e)
@@ -203,6 +203,7 @@ public class ServletWebResponse extends
try
{
redirect = true;
+ url = sanitize(url);
url = encodeRedirectURL(url);
// wicket redirects should never be cached
@@ -268,4 +269,17 @@ public class ServletWebResponse extends
{
return httpServletResponse;
}
+
+ /**
+ * Cleans the provided input (header name or value) from malicious
characters.
+ *
+ * @param input
+ * the string to sanitize
+ * @return the sanitized string
+ */
+ String sanitize(final String input)
+ {
+ String output = input.replace('\n', ' ').replace('\r', ' ');
+ return output;
+ }
}
Modified:
wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebResponseTest.java
URL:
http://svn.apache.org/viewvc/wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebResponseTest.java?rev=1199754&r1=1199753&r2=1199754&view=diff
==============================================================================
---
wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebResponseTest.java
(original)
+++
wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebResponseTest.java
Wed Nov 9 13:43:46 2011
@@ -27,6 +27,8 @@ import java.io.StringWriter;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.apache.wicket.protocol.http.mock.MockHttpServletResponse;
+import org.apache.wicket.util.time.Time;
import org.junit.Assert;
import org.junit.Test;
import org.mockito.Matchers;
@@ -99,4 +101,48 @@ public class ServletWebResponseTest exte
assertTrue(webResponse.isRedirect());
}
+
+ /**
+ * Verifies that response headers' name and/or values doesn't contain
malicious characters
+ *
+ * https://issues.apache.org/jira/browse/WICKET-4196
+ *
+ * @throws IOException
+ */
+ @Test
+ public void sanitizeHeaders() throws IOException
+ {
+ final String badInput = "something\n\rbad\n\r\n";
+ final String badUrl = "bad\n\rurl\r\n";
+
+ ServletWebRequest webRequest = mock(ServletWebRequest.class);
+ when(webRequest.isAjax()).thenReturn(Boolean.FALSE);
+
+ MockHttpServletResponse httpServletResponse = new
MockHttpServletResponse(null);
+
+ ServletWebResponse webResponse = new
ServletWebResponse(webRequest, httpServletResponse);
+
+ webResponse.addHeader(badInput, "someValue");
+ assertNull(httpServletResponse.getHeader(badInput));
+
assertEquals(httpServletResponse.getHeader(webResponse.sanitize(badInput)),
"someValue");
+
+ webResponse.addHeader("someName", badInput);
+ assertEquals(httpServletResponse.getHeader("someName"),
"something bad ");
+
+ webResponse.setHeader(badInput, badInput);
+ assertNull(httpServletResponse.getHeader(badInput));
+
assertEquals(httpServletResponse.getHeader(webResponse.sanitize(badInput)),
+ "something bad ");
+
+ Time now = Time.now();
+ webResponse.setDateHeader(badInput, now);
+ assertNull(httpServletResponse.getHeader(badInput));
+ String dateHeaderValue =
httpServletResponse.getHeader(webResponse.sanitize(badInput));
+ assertNotNull(dateHeaderValue);
+ assertEquals(-1, dateHeaderValue.indexOf('\n'));
+ assertEquals(-1, dateHeaderValue.indexOf('\r'));
+
+ webResponse.sendRedirect(badUrl);
+ assertEquals(httpServletResponse.getRedirectLocation(), "bad
url ");
+ }
}
