reject cookies with control characters as invalid

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<Header> formatCookies(final List<Cookie> 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<Header> formatCookies(final List<Cookie> 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;
+    }
+
 }

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<Cookie> 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<Header> 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<Header> 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);