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,8 +242,9 @@ public List<Header> formatCookies(final List<Cookie> cookies) {
                 buffer.append(PARAM_DELIMITER);
                 buffer.append(' ');
             }
-            buffer.append(cookie.getName());
-            final String s = cookie.getValue();
+            final String name = replaceControlChars(cookie.getName());
+            buffer.append(name);
+            final String s = replaceControlChars(cookie.getValue());
             if (s != null) {
                 buffer.append(EQUAL_CHAR);
                 if (containsSpecialChar(s)) {
@@ -278,4 +285,51 @@ 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++) {
+            final char ch = s.charAt(i);
+            if (ch < 0x20 || ch == 0x7f) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Replaces every control character ({@code %x00-1F} or {@code %x7F}) in the given cookie
+     * name or value 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.
+     */
+    static String replaceControlChars(final CharSequence s) {
+        if (s == null) {
+            return null;
+        }
+        StringBuilder buf = null;
+        for (int i = 0; i < s.length(); i++) {
+            final char ch = s.charAt(i);
+            if (ch < 0x20 || ch == 0x7f) {
+                if (buf == null) {
+                    buf = new StringBuilder(s.length());
+                    buf.append(s, 0, i);
+                }
+                buf.append('?');
+            } else if (buf != null) {
+                buf.append(ch);
+            }
+        }
+        return buf != null ? buf.toString() : s.toString();
+    }
+
 }

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