diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java index 590cc5a319..16f71dced0 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java @@ -199,6 +199,12 @@ public final void validate(final Cookie cookie, final CookieOrigin origin) throws MalformedCookieException { Args.notNull(cookie, "Cookie"); Args.notNull(origin, "Cookie origin"); + if (containsControlChar(cookie.getName())) { + throw new MalformedCookieException("Cookie name contains control characters"); + } + if (containsControlChar(cookie.getValue())) { + throw new MalformedCookieException("Cookie value contains control characters"); + } for (final CookieAttributeHandler handler: this.attribHandlers) { handler.validate(cookie, origin); } @@ -236,7 +242,7 @@ public List
formatCookies(final List cookies) { buffer.append(PARAM_DELIMITER); buffer.append(' '); } - buffer.append(cookie.getName()); + appendReplacingControlChars(buffer, cookie.getName()); final String s = cookie.getValue(); if (s != null) { buffer.append(EQUAL_CHAR); @@ -247,11 +253,11 @@ public List
formatCookies(final List cookies) { if (ch == DQUOTE_CHAR || ch == ESCAPE_CHAR) { buffer.append(ESCAPE_CHAR); } - buffer.append(ch); + buffer.append(isControlChar(ch) ? '?' : ch); } buffer.append(DQUOTE_CHAR); } else { - buffer.append(s); + appendReplacingControlChars(buffer, s); } } } @@ -278,4 +284,44 @@ boolean containsChars(final CharSequence s, final Tokenizer.Delimiter chars) { return false; } + /** + * Tests whether the given cookie name or value contains a control character + * ({@code %x00-1F} or {@code %x7F}). RFC 6265 permits neither in a cookie-name (a token) + * nor in a cookie-value (cookie-octet), and such characters must never be copied into + * a {@code Cookie} request header. + */ + static boolean containsControlChar(final CharSequence s) { + if (s == null) { + return false; + } + for (int i = 0; i < s.length(); i++) { + if (isControlChar(s.charAt(i))) { + return true; + } + } + return false; + } + + /** + * Appends the given cookie name or value to the buffer, replacing every control character + * ({@code %x00-1F} or {@code %x7F}) with {@code '?'}, mirroring the substitution the session + * output buffer already performs, so such characters can never be copied verbatim into + * a {@code Cookie} request header. Cookies carrying control characters are rejected by + * {@link #validate} before they reach the cookie store; this is a defensive fallback for + * a value that bypassed validation. + */ + private static void appendReplacingControlChars(final CharArrayBuffer buffer, final CharSequence s) { + if (s == null) { + return; + } + for (int i = 0; i < s.length(); i++) { + final char ch = s.charAt(i); + buffer.append(isControlChar(ch) ? '?' : ch); + } + } + + private static boolean isControlChar(final char ch) { + return ch < 0x20 || ch == 0x7f; + } + } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java index 56897c9b81..823cc32c31 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java @@ -35,6 +35,7 @@ import org.apache.hc.client5.http.cookie.Cookie; import org.apache.hc.client5.http.cookie.CookieOrigin; import org.apache.hc.client5.http.cookie.MalformedCookieException; +import org.apache.hc.core5.http.FormattedHeader; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.message.BasicHeader; import org.junit.jupiter.api.Assertions; @@ -222,6 +223,30 @@ void testValidateCookieBasics() throws Exception { Mockito.verify(h2).validate(cookie, origin); } + @Test + void testValidateCookieRejectsControlCharsInName() { + final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec(); + final CookieOrigin origin = new CookieOrigin("host", 80, "/path/", true); + final BasicClientCookie cookie = new BasicClientCookie("sid\r\nInjected: evil", "value"); + Assertions.assertThrows(MalformedCookieException.class, () -> + cookiespec.validate(cookie, origin)); + } + + @Test + void testValidateCookieRejectsControlCharsInValue() throws Exception { + final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec(); + final CookieOrigin origin = new CookieOrigin("host", 80, "/path/", true); + // NUL is neither whitespace nor a delimiter, so it survives parsing and + // must be caught by validation before the cookie reaches the store. + final Header header = new BasicHeader("Set-Cookie", "sid=abc\u0000def"); + final List cookies = cookiespec.parse(header, origin); + Assertions.assertEquals(1, cookies.size()); + final Cookie cookie = cookies.get(0); + Assertions.assertEquals("abc\u0000def", cookie.getValue()); + Assertions.assertThrows(MalformedCookieException.class, () -> + cookiespec.validate(cookie, origin)); + } + @Test void testMatchCookie() { final CommonCookieAttributeHandler h1 = Mockito.mock(CommonCookieAttributeHandler.class); @@ -291,6 +316,28 @@ void testFormatCookiesIllegalCharsInValue() { Assertions.assertEquals("name1=value; name2=\"some value\"; name3=\"\\\"\\\\\\\"\"", header.getValue()); } + @Test + void testFormatCookiesReplacesControlCharsInName() { + final Cookie cookie = new BasicClientCookie("sid\r\nInjected: evil", "value"); + final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec(); + final List
headers = cookiespec.formatCookies(Collections.singletonList(cookie)); + Assertions.assertEquals(1, headers.size()); + final String wire = ((FormattedHeader) headers.get(0)).getBuffer().toString(); + Assertions.assertFalse(wire.indexOf('\r') >= 0 || wire.indexOf('\n') >= 0); + Assertions.assertEquals("Cookie: sid??Injected: evil=value", wire); + } + + @Test + void testFormatCookiesReplacesControlCharsInValue() { + final Cookie cookie = new BasicClientCookie("sid", "abc\r\nInjected: evil"); + final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec(); + final List
headers = cookiespec.formatCookies(Collections.singletonList(cookie)); + Assertions.assertEquals(1, headers.size()); + final String wire = ((FormattedHeader) headers.get(0)).getBuffer().toString(); + Assertions.assertFalse(wire.indexOf('\r') >= 0 || wire.indexOf('\n') >= 0); + Assertions.assertEquals("Cookie: sid=\"abc??Injected: evil\"", wire); + } + @Test void testParseCookieMultipleAttributes() throws Exception { final CommonCookieAttributeHandler h1 = Mockito.mock(CommonCookieAttributeHandler.class);