Can't modify response headers from a chrome.webRequest.onHeadersReceived listener when invalid header fields already present
Reported by
rh...@raymondhill.net,
Jun 15 2016
|
|||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/50.0.2661.102 Chrome/50.0.2661.102 Safari/537.36 Steps to reproduce the problem: The issue occurs with chrome extension uMatrix, or any extension which modifies response headers. 1. Install proof-of-concept minimal extension attached as zipped file below 2. This extension will add a 'Set-Cookie' header which value is 'foo=bar' for the sites used below, through chrome.webRequest.onHeaderReceived API. 3. Visit https://news.ycombinator.com/ 4. Observe that cookie 'foo=bar' is set for the page 5. Visit http://git.musl-libc.org/cgit/musl/ 6. Observe that cookie 'foo=bar' is NOT set for the page What is the expected behavior? Step 6 should be: 6. Observe that cookie 'foo=bar' is set for http://git.musl-libc.org/cgit/musl/ What went wrong? The web page at http://git.musl-libc.org/cgit/musl/ contains invalid header fields (see attached PNG image). When the extension returns the modified response headers, it discards these modified response headers because of the pre-existing invalid header fields -- as if the extension is blamed for these invalid fields. The following error is reported in the extension's dev console: extensions::lastError:133 Unchecked runtime.lastError while running webRequestInternal.eventHandled: Invalid header name. reportIfUnchecked @ extensions::lastError:133 handleResponse @ extensions::sendRequest:78 WebStore page: https://chrome.google.com/webstore/detail/umatrix/ogfcmafjalglgifnmanfmnieipoejdcf Did this work before? N/A Chrome version: 50.0.2661.102 Channel: stable OS Version: Linux Mint 17.2 Flash Version: Though the proof of concept extension may give the impression this is not a serious issue, it is a serious issue for the uMatrix extension, as the bug here prevents the extension from correctly preventing javascript to execute -- which is one of its purpose, because the injected Content-Security-Policy response header is discarded by the chrome.webRequest API.
,
Jun 15 2016
> ... the injected Content-Security-Policy response header is discarded by the chrome.webRequest API. To be more accurate, I should have written: ... the injected Content-Security-Policy response header is discarded by the chrome.webRequest API when a server returns invalid header fields. This means any web site could bypass any Chrome extension modifying response header fields by merely using just one invalid header fields in its response headers.
,
Jun 24 2016
I'll take a look.
,
Jun 27 2016
Looking into this, it seems the IsValidHeaderName is causing this in web_request_api.cc where we go through each header that was specified. Rob seems to have made a bunch of changes here and would be more knowledgeable than I am, I'm assigning the bug him to see what we can do here. Comment #2 about bypassing seem concerning to me. Rob, would you able to take a look?
,
Jun 28 2016
This is an issue in the network layer.
The parsed header names should be valid, but they are not.
When I use netcat and serve the following response to Firefox 47,
HTTP/1.1 200 OK
[x] /: Value
Content-length: 0
When I perform a same-origin request to a URL that responds with the above data,
x=new XMLHttpRequest;x.open('get',document.URL);x.send();x.onload = ()=>console.log(x.getAllResponseHeaders())
then Firefox 47 shows this (GOOD):
Content-Length: 0
And Chrome 51 shows this (BAD):
Content-length: 0
[x] /: Value
Patch: https://codereview.chromium.org/2101963002/
,
Jun 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/78f29ac0214e16bdcc6eaa17fa4a5a84882bb7a8 commit 78f29ac0214e16bdcc6eaa17fa4a5a84882bb7a8 Author: rob <rob@robwu.nl> Date: Tue Jun 28 17:33:43 2016 Skip header lines with invalid header name The current header parsing algorithm is too lax and results in the creation of a header data structure with invalid headers. This patch makes the parser more strict, by rejecting header names that are not complying with RFC 7230 (field-name must be a token). Enforcing this standard should be web-compatible since Firefox (47) also follows this behavior. BUG= 620365 Review-Url: https://codereview.chromium.org/2101963002 Cr-Commit-Position: refs/heads/master@{#402498} [modify] https://crrev.com/78f29ac0214e16bdcc6eaa17fa4a5a84882bb7a8/net/http/http_util.cc [modify] https://crrev.com/78f29ac0214e16bdcc6eaa17fa4a5a84882bb7a8/net/http/http_util_unittest.cc
,
Jun 29 2016
,
Jul 6
,
Jul 6
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by rh...@raymondhill.net
, Jun 15 2016984 bytes
984 bytes Download