Bug 63434 - Multiple Cookie headers combined to one comma-separated header line
Summary: Multiple Cookie headers combined to one comma-separated header line
Status: NEW
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: Core (show other bugs)
Version: 2.4.39
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2019-05-15 08:42 UTC by Armin Abfalterer
Modified: 2019-05-29 04:27 UTC (History)
0 users



Attachments
patch that turns multiple Cookie headers into single header (2.71 KB, patch)
2019-05-27 07:34 UTC, Armin Abfalterer
Details | Diff
patch file with initialization fix (2.72 KB, patch)
2019-05-27 09:43 UTC, Armin Abfalterer
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Armin Abfalterer 2019-05-15 08:42:19 UTC
RFC 6265, 5.4. The Cookie Header says

"When the user agent generates an HTTP request, the user agent MUST NOT attach more than one Cookie header field."

However, httpd combines multiple Cookie headers into on header line; e.g.

Cookie: foo1=bar1
Cookie: foo2=bar2
Cookie: foo3=bar3

becomes "Cookie: foo1=bar1, foo2=bar2, foo3=bar3" (which in turns violates syntax definition in RFC 6265, 4.2.1. Syntax).

The call of apr_table_compress() in in server/protocol.c:ap_get_mime_headers_core() leads to this misbehaviour

https://github.com/apache/httpd/blob/trunk/server/protocol.c#L1274

Cheers, Armin
Comment 1 Yann Ylavic 2019-05-15 09:05:15 UTC
(In reply to Armin Abfalterer from comment #0)
> RFC 6265, 5.4. The Cookie Header says
> 
> "When the user agent generates an HTTP request, the user agent MUST NOT
> attach more than one Cookie header field."

This says "there must not be multiple Cookie headers" (thus "multiple cookies must be in the same header"), which you translate as "there must not be multiple cookies in the same header"?

It seems to me that apr_table_compress() does the right thing, including for Cookie headers..
Comment 2 Armin Abfalterer 2019-05-15 10:43:23 UTC
(In reply to Yann Ylavic from comment #1)

> This says "there must not be multiple Cookie headers" (thus "multiple
> cookies must be in the same header"), which you translate as "there must not
> be multiple cookies in the same header"?

No, multiple cookies are fine in the same header... but separated by semicolon, not comma.
Comment 3 Yann Ylavic 2019-05-15 11:18:19 UTC
So, since comma in a header is equivalent to multiple headers, do you propose that httpd rejects (with status 4xx) any request with either multiple Cookie header or a single one containing comma(s)?

Because turning multiple Cookie headers into a single one with semicolon(s) is not the same HTTP request (while the comma preserves semantics), the only possible action would be to reject.

Also, it seems to me that Cookie is an application thingy, not an HTTP one, so why would httpd reject it if the HTTP header is valid?
With comma separated cookies, the application can detect and reject, not if httpd changes the semantics..
Comment 4 Armin Abfalterer 2019-05-15 12:19:04 UTC
(In reply to Yann Ylavic from comment #3)
> So, since comma in a header is equivalent to multiple headers, do you
> propose that httpd rejects (with status 4xx) any request with either
> multiple Cookie header or a single one containing comma(s)?
> 
> Because turning multiple Cookie headers into a single one with semicolon(s)
> is not the same HTTP request (while the comma preserves semantics), the only
> possible action would be to reject.

I'd propose either to reject a request with multiple Cookie headers or to turn multiple Cookie headers into one where each cookie-pair is separated by semicolon.

In any case I'd propose to reject a request with comma separated cookie-pairs in a Cookie header.

> Also, it seems to me that Cookie is an application thingy, not an HTTP one,
> so why would httpd reject it if the HTTP header is valid?
> With comma separated cookies, the application can detect and reject, not if
> httpd changes the semantics..

In my opinion separated cookie pairs are a HTTP protocol violation so httpd should not allow this at all; e.g. such request should not hit backend servers when mod_proxy is in use
Comment 5 Armin Abfalterer 2019-05-27 07:34:15 UTC
Created attachment 36600 [details]
patch that turns multiple Cookie headers into single header
Comment 6 Armin Abfalterer 2019-05-27 09:43:57 UTC
Created attachment 36601 [details]
patch file with initialization fix

(In reply to Armin Abfalterer from comment #5)
> Created attachment 36600 [details]
> patch that turns multiple Cookie headers into single header

there is a error in the patch, a wrong initialization

cookie_hdrs_state state = {r->pool, NULL, 0};

see new patch file