response header merging
Reported by
m...@michaelruddy.com,
Nov 30 2016
|
|||||||||||
Issue descriptionVULNERABILITY DETAILS Response headers are incorrectly merged across line breaks when a valid header line is followed by a line that begins with a null character. This can be used to do things such as change the scope or expiration time of session cookies, or generally to alter the meaning/values of previous response header lines. The attached POC shows how this could affect sites that return user controlled header data. I noticed this while reviewing the AssembleRawHeaders method at https://cs.chromium.org/chromium/src/net/http/http_util.cc?cl=GROK&l=679 I believe that the bug is due to searches for the null character finding the automatically appended null terminating character of the HTTP_LWS constant: https://cs.chromium.org/chromium/src/net/http/http_util.cc?cl=GROK&gsn=IsLWS&l=429 https://cs.chromium.org/chromium/src/net/http/http_util.h?cl=GROK&gsn=IsLWS&rcl=1480493940&l=25 A fix may be to check for the null character prior to running strchr: bool HttpUtil::IsLWS(char c) { return c != '\0' && strchr(HTTP_LWS, c) != NULL; } VERSION Chrome Version: 54.0.2840.100 + stable Operating System: Ubuntu 16.04.1 LTS REPRODUCTION CASE 1) Point example.com to your localhost by making /etc/hosts point example.com to 127.0.0.1. 2) Setup a TCP listener to return the attached crafted response: sudo nc -l 80 < crafted-response.hex 3) Start chrome and navigate to example.com 4) Open dev tools and examine merged response headers and cookies. Make a second request and see that the cookie was set and sent on the second request successfully. I've included screenshots of the dev tools for two requests (the second shows that the cookie was set successfully and resent), a wireshark packet capture, a net-internals capture, and an example crafted HTTP response that demonstrates changing a random header and the expiration of a session cookie.
,
Nov 30 2016
"The strchr function locates the first occurrence of c (converted to a char) in the string pointed to by s. The terminating null character is considered to be part of the string." The impact of this bug is limited to a server that allows only *limited* attacker control of a header value; if the attacker can inject either space or tab before a header name they can accomplish the same thing within the rules of RFC7230. obs-fold = CRLF 1*( SP / HTAB )
,
Nov 30 2016
,
Nov 30 2016
Seems like we should just get rid of the "raw" header format. Also may be a good idea to at least reject some obviously bad characters when they're in headers, though doing that may break too much, actually, at least for anything but null.
,
Nov 30 2016
Tagging with current canary milestone, feel free to change if needed.
,
Nov 30 2016
Thanks for the report! Confirmed that HttpUtil::IsLWS() is buggy, and this can lead to incorrectly treating embedded nulls as header line continuations.
,
Dec 6 2016
Marking this as Pri-1, as I think we should fix it before M-57 branches, though not so urgent it warrants a merge. Eric, you are planning on getting to this before the break? If not, I'm happy to take it.
,
Dec 6 2016
Yeah I can work on this today.
,
Dec 6 2016
,
Dec 6 2016
,
Dec 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/acf834a5675c89f8b4c3b44a4dfeed3fb4be4375 commit acf834a5675c89f8b4c3b44a4dfeed3fb4be4375 Author: eroman <eroman@chromium.org> Date: Tue Dec 06 21:13:27 2016 Don't consider '\0' as whitespace when parsing HTTP headers. BUG= 669932 Review-Url: https://codereview.chromium.org/2555813002 Cr-Commit-Position: refs/heads/master@{#436724} [modify] https://crrev.com/acf834a5675c89f8b4c3b44a4dfeed3fb4be4375/net/http/http_util.cc [modify] https://crrev.com/acf834a5675c89f8b4c3b44a4dfeed3fb4be4375/net/http/http_util_unittest.cc
,
Feb 17 2017
Has this not been marked for bug bounty consideration, or did it go through that process and get declined?
,
Feb 17 2017
@elawrence, can you answer comment #12?
,
Feb 17 2017
Oddly, this doesn't seem to have Type-Security, despite the issue being filed with the Vulnerability template, so it's possible that awhalley@ never saw it. That said, I believe that this issue would've been triaged as Low Severity and thus most likely out of scope for the Vulnerability Rewards program (https://www.google.com/about/appsecurity/chrome-rewards/). As noted in Comment #2, the scope of this vulnerability is very small. A vulnerable server would have to have a bug whereby it could be induced to emit an attacker controlled null byte at a particular location in its HTTP response headers, and to make Chrome's bug useful, that server would have to correctly forbid emission of an attacker controlled SPACE or TAB byte in the same position. The set of real-world servers meeting both conditions seems likely to be extremely small (and again, the vector should primarily be considered a server bug).
,
Feb 17 2017
OK, thanks for the update and explanation.
,
Jul 6
,
Jul 6
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by elawrence@chromium.org
, Nov 30 2016