strip control characters when writing the cookie header

httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java

@@ -236,8 +236,8 @@ public List<Header> formatCookies(final List<Cookie> cookies) {
                 buffer.append(PARAM_DELIMITER);
                 buffer.append(' ');
             }
-            buffer.append(cookie.getName());
-            final String s = cookie.getValue();
+            buffer.append(sanitize(cookie.getName()));
+            final String s = sanitize(cookie.getValue());
             if (s != null) {
                 buffer.append(EQUAL_CHAR);
                 if (containsSpecialChar(s)) {
@@ -278,4 +278,51 @@ boolean containsChars(final CharSequence s, final Tokenizer.Delimiter chars) {
         return false;
     }
 
+    /**
+     * Neutralises characters that must never appear in a cookie name or value when the
+     * {@code Cookie} header is written out. RFC 6265 excludes control characters from both the
+     * cookie-name (a token) and the cookie-value (cookie-octet), yet {@link #formatCookies} used to
+     * copy them through verbatim. A bare CR or LF in a cookie set from attacker-influenced data
+     * would then be written straight into the request header on the classic transport, splitting it
+     * and letting further headers be injected. A maximal run of CR, LF and NUL is collapsed to a
+     * single space so a folded {@code CRLF} leaves one space rather than a run of them, matching the
+     * substitution {@code BufferedHeader#getValue()} already applies when the value is read back.
+     * This closes the injection vector; it is not meant to fully validate the name against the
+     * token grammar or the value against cookie-octet.
+     */
+    private static String sanitize(final String s) {
+        if (s == null) {
+            return null;
+        }
+        int i = 0;
+        for (; i < s.length(); i++) {
+            final char ch = s.charAt(i);
+            if (ch == '\r' || ch == '\n' || ch == '\0') {
+                break;
+            }
+        }
+        if (i == s.length()) {
+            return s;
+        }
+        final StringBuilder buf = new StringBuilder(s.length());
+        buf.append(s, 0, i);
+        boolean pending = false;
+        for (; i < s.length(); i++) {
+            final char ch = s.charAt(i);
+            if (ch == '\r' || ch == '\n' || ch == '\0') {
+                pending = true;
+            } else {
+                if (pending) {
+                    buf.append(' ');
+                    pending = false;
+                }
+                buf.append(ch);
+            }
+        }
+        if (pending) {
+            buf.append(' ');
+        }
+        return buf.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;
@@ -291,6 +292,29 @@ void testFormatCookiesIllegalCharsInValue() {
         Assertions.assertEquals("name1=value; name2=\"some value\"; name3=\"\\\"\\\\\\\"\"", header.getValue());
     }
 
+    @Test
+    void testFormatCookiesStripsControlCharsFromValue() {
+        final Cookie cookie = new BasicClientCookie("sid", "abc\r\nInjected: evil");
+        final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec();
+        final List<Header> headers = cookiespec.formatCookies(Collections.singletonList(cookie));
+        // The classic transport writes the raw buffer of a FormattedHeader, not the sanitised
+        // getValue(), so the buffer itself must be free of CR/LF.
+        final String wire = ((FormattedHeader) headers.get(0)).getBuffer().toString();
+        Assertions.assertFalse(wire.indexOf('\r') >= 0 || wire.indexOf('\n') >= 0,
+                "CR/LF leaked into the Cookie header buffer");
+        Assertions.assertEquals("Cookie: sid=\"abc Injected: evil\"", wire);
+    }
+
+    @Test
+    void testFormatCookiesStripsControlCharsFromName() {
+        final Cookie cookie = new BasicClientCookie("sid\r\nInjected: evil", "v");
+        final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec();
+        final List<Header> headers = cookiespec.formatCookies(Collections.singletonList(cookie));
+        final String wire = ((FormattedHeader) headers.get(0)).getBuffer().toString();
+        Assertions.assertFalse(wire.indexOf('\r') >= 0 || wire.indexOf('\n') >= 0,
+                "CR/LF leaked into the Cookie header buffer via the cookie name");
+    }
+
     @Test
     void testParseCookieMultipleAttributes() throws Exception {
         final CommonCookieAttributeHandler h1 = Mockito.mock(CommonCookieAttributeHandler.class);