Issue metadata
Sign in to add a comment
|
Security: bypass CORS check by returning 304 from URL that previously returned 308 during revalidation from MemoryCache |
||||||||||||||||||||||
Issue descriptionVULNERABILITY DETAILS Resource::revalidationSucceeded() overwrites headers when receiving 304 response. When revalidating a URL that previously returns 308 but now returns 304, the 304 response can overwrite headers of the response from redirect target. Example: (localhost:8019 is an evil server) 1. Request localhost:8019/redirect 2. localhost:8019/redirect returns response 308: redirect to foo.com/target 3. foo.com/target returns response 200, without Access-Control-Allow-Origin header. Here we get a Resource with request URL = MemoryCache key = 'localhost:8019/redirect' with response headers/content from foo.com/target. XHRs to this Resource would fail due to CORS check. 4. Revalidate localhost:8019/redirect 5. localhost:8019/redirect returns response 304 with Access-Control-Allow-Origin header. Here the Resource has content from foo.com/target AND some response headers from localhost:8019/redirect. XHRs to this Resource would succeeds due to injected Access-Control-Allow-Origin header. VERSION 50.0.2661.102 (Official Build) stable on Ubuntu 51.0.2704.54 (Official Build) beta on Ubuntu 51.0.2704.54 (Official Build) beta-m on Windows 7 52.0.2743.0 (Official Build) canary on Windows 7 53.0.2744.0 (Developer Build, r395047) on Ubuntu REPRODUCTION CASE 1. Download and run $ python exploit4.py 2. Access http://localhost:8019/ (Do not open DevTools) 3. You'll see 'NG: CORS bypassed.' alert dialog. 4. Open DevTools console. 5. You'll see the contents of 'https://www.google.co.jp/images/nav_logo242.png' which doesn't have CORS headers (and thus shouldn't be accessed cross-origin).
,
May 24 2016
Adding security triage labels. Just to be explicit, this is not a regression from stable, so it's not a release blocker.
,
May 24 2016
,
May 25 2016
,
May 26 2016
Candidate fix: Disallow Revalidate in determineRevalidationPolicy() if redirectChain() is not empty, return Reload instead. Implementing revalidation of Resource with redirect chain looks quite hard: When receiving 304, ResourceLoader::didReceiveResponse() is called, but if it is 304 to a redirect, we must switch to the willFollowRedirect() path, and thus probably we have to modify Resource, ResourceLoader, WebURLLoaderImpl etc., and switching receiveResponse() path to willFollowRedirect() path would look complicated.
,
May 26 2016
,
May 26 2016
,
May 26 2016
,
May 27 2016
(Adding some more people who might have opinion) #5- the proposed fix sounds reasonable to me, it might have some perf impact (which will unlikely be caught by bots) and we might want to revisit if important sites start to have regression, but it'd fix the immediate security threat in a manageable way.
,
May 27 2016
Draft CL: https://codereview.chromium.org/2011283002/, the candidate fix in Comment #5.
,
Jun 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/64ad58e1d8451857f6a763651727c9d39375ea3b commit 64ad58e1d8451857f6a763651727c9d39375ea3b Author: hiroshige <hiroshige@chromium.org> Date: Wed Jun 01 05:15:46 2016 Do not initiate revalidation of Resource with redirects from MemoryCache BUG= 613971 Review-Url: https://codereview.chromium.org/2011283002 Cr-Commit-Position: refs/heads/master@{#397064} [modify] https://crrev.com/64ad58e1d8451857f6a763651727c9d39375ea3b/third_party/WebKit/Source/core/fetch/Resource.cpp
,
Jun 3 2016
#11 landed on 53.0.2756.0.
,
Jun 3 2016
The test case (Comment #0) works correctly on 53.0.2756.0 canary on Windows 7. Closing as fixed. I'll request merge later.
,
Jun 3 2016
Adding Merge-Triage label for tracking purposes. Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Request-XX label, where XX is the Chrome milestone. When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com. - Your friendly ClusterFuzz
,
Jun 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d0753f72046d160bc34584a47eb3f3a3c56daff1 commit d0753f72046d160bc34584a47eb3f3a3c56daff1 Author: hiroshige <hiroshige@chromium.org> Date: Fri Jun 03 11:28:20 2016 [Loader] Add asserts about revalidation and redirects After https://codereview.chromium.org/2011283002/ and https://codereview.chromium.org/2017883002/, redirect chain must be empty when revalidation starts or during revalidation is ongoing. This CL adds asserts to verify that. BUG= 613971 , 614989 Review-Url: https://codereview.chromium.org/2020253002 Cr-Commit-Position: refs/heads/master@{#397683} [modify] https://crrev.com/d0753f72046d160bc34584a47eb3f3a3c56daff1/third_party/WebKit/Source/core/fetch/Resource.cpp
,
Jun 3 2016
,
Jun 6 2016
Requesting merge to M-52 beta.
,
Jun 6 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jun 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4dc64be09d13a17a7870bfb16a4f042ab3b33c95 commit 4dc64be09d13a17a7870bfb16a4f042ab3b33c95 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Mon Jun 06 14:23:58 2016 Do not initiate revalidation of Resource with redirects from MemoryCache BUG= 613971 Review-Url: https://codereview.chromium.org/2011283002 Cr-Commit-Position: refs/heads/master@{#397064} (cherry picked from commit 64ad58e1d8451857f6a763651727c9d39375ea3b) Review URL: https://codereview.chromium.org/2044483002 . Cr-Commit-Position: refs/branch-heads/2743@{#235} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/4dc64be09d13a17a7870bfb16a4f042ab3b33c95/third_party/WebKit/Source/core/fetch/Resource.cpp
,
Jun 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/762426dfb504e5b631ee0b48ffe9e3ba7f7ba6e3 commit 762426dfb504e5b631ee0b48ffe9e3ba7f7ba6e3 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Wed Jun 08 04:19:58 2016 [Loader] Add asserts about revalidation and redirects After https://codereview.chromium.org/2011283002/ and https://codereview.chromium.org/2017883002/, redirect chain must be empty when revalidation starts or during revalidation is ongoing. This CL adds asserts to verify that. BUG= 613971 , 614989 Review-Url: https://codereview.chromium.org/2020253002 Cr-Commit-Position: refs/heads/master@{#397683} (cherry picked from commit d0753f72046d160bc34584a47eb3f3a3c56daff1) Review URL: https://codereview.chromium.org/2046923002 . Cr-Commit-Position: refs/branch-heads/2743@{#275} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/762426dfb504e5b631ee0b48ffe9e3ba7f7ba6e3/third_party/WebKit/Source/core/fetch/Resource.cpp
,
Jun 13 2016
This missed last Wednesday's Beta. This would be ready for a Stable release after 20th June. (Fixed in same commit as issue 614989 )
,
Jun 13 2016
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/762426dfb504e5b631ee0b48ffe9e3ba7f7ba6e3 commit 762426dfb504e5b631ee0b48ffe9e3ba7f7ba6e3 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Wed Jun 08 04:19:58 2016 [Loader] Add asserts about revalidation and redirects After https://codereview.chromium.org/2011283002/ and https://codereview.chromium.org/2017883002/, redirect chain must be empty when revalidation starts or during revalidation is ongoing. This CL adds asserts to verify that. BUG= 613971 , 614989 Review-Url: https://codereview.chromium.org/2020253002 Cr-Commit-Position: refs/heads/master@{#397683} (cherry picked from commit d0753f72046d160bc34584a47eb3f3a3c56daff1) Review URL: https://codereview.chromium.org/2046923002 . Cr-Commit-Position: refs/branch-heads/2743@{#275} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/762426dfb504e5b631ee0b48ffe9e3ba7f7ba6e3/third_party/WebKit/Source/core/fetch/Resource.cpp
,
Jul 19 2016
,
Sep 9 2016
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
,
Oct 1 2016
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
,
Oct 2 2016
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
,
Oct 2 2016
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by hirosh...@chromium.org
, May 23 2016Owner: hirosh...@chromium.org
Status: Assigned (was: Unconfirmed)