Referrer policy is not applied to requests initiated via location.reload()
Reported by
dmascare...@etouch.net,
Oct 24 2016
|
||||||||
Issue descriptionChrome Version:56.0.2899.0 (Official Build) bb7071018e1fd8bf223b8ffff660883b7d17278d-refs/heads/master@{#426989} 32/64-bit OS : Mac(10.10.5, 10.11.4, 10.11.5), Windows(7,8,8.1,10), Linux(14.04 LTS) Test url: 1. https://chrome.google.com/webstore/detail/google-hangouts/nckgahadagoaajjgafhacjanaoiihapd?hl=en Precondition: Install 'Hangout' extension using above url and then disable (uncheck the checkbox) above extension from 'chrome://extensions' What steps will reproduce the problem? 1. Launch chrome and navigate to test url 1. 2. Click on 'Enable this item' link and observe. Actual: Glimpse of error message is seen while enabling the disabled extension on Web store page. Expected: Glimpse of error message should not be seen. This is regression issue, broken in 'M 56' and below is manual bisect info: Good build:56.0.2886.0 Bad build:56.0.2888.0
,
Oct 24 2016
Able to reproduce this issue on both Windows 10 on latest chrome canary version 56.0.2899.0 Issue is broken in M56. Below are the bisect details for the same: Bisect Info: =========== Good build:56.0.2886.0, Revision Range(424099) Bad build: 56.0.2888.0, Revision Range(424625) After executing the bisect script, i got the following CL's between good and bad build versions =============================================================================================== https://chromium.googlesource.com/chromium/src/+log/4d7d69c3449e072639ecd4751b9f11c6b142c36d..0f5deae9b4797dfc2ce9ec685bc877c68982621b The suspecting Change Log is : ----------- https://chromium.googlesource.com/chromium/src/+/0f5deae9b4797dfc2ce9ec685bc877c68982621b From the above CL suspecting the below change Review URL: https://codereview.chromium.org/2393633006 estark@- Could you please look into this issue, if it's related to your change? if not could you please help us to reassign this issue to the right owner. Thanks.!
,
Oct 24 2016
Interesting -- https://codereview.chromium.org/2393633006 did not actually cause the bug here but instead revealed a pre-existing bug. Here's a minimal repro that works in stable (M54): 1. Visit http://example.com/foo 2. Using the DevTools console, add a Referrer Policy to the page: var m = document.createElement("meta"); m.name = "referrer"; m.content = "origin"; document.head.appendChild(m); 3. Run `location.reload()` in the console. 4. Observe the Referer header on the reload request in the Network tab is "http://example.com/foo", whereas it should be "http://example.com/". In M56 Canary (after https://codereview.chromium.org/2393633006), this repro flashes a net error page before reloading the page. The bug is that the referrer policy is being set on the request, but the referrer is not being generated according to that referrer policy. The net stack cancels requests for which the referrer policy does not match the actual referrer string. Up until https://codereview.chromium.org/2393633006, the net stack lumped several referrer policies into the same category and would not have noticed that the referrer policy didn't match. So before https://codereview.chromium.org/2393633006, the net stack did not have enough information to notice that the referrer policy did not match the referrer on the reload request, so the request was not cancelled. As of https://codereview.chromium.org/2393633006 and later, the net stack cancels the request because the referrer policy and the referrer do not match. That explains the error page. There must be some retry going on somewhere that does set the referrer correctly, because the error page only flashes and then is replaced by the properly reloaded page.
,
Oct 24 2016
,
Oct 24 2016
Oh, I should also note: normally disregarding the Referrer Policy could be a low-severity security bug, but since AFAICT this only applies to location.reload() I can't think of a real privacy threat that this poses.
,
Oct 25 2016
,
Oct 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1dad47f20ed56478257aad69c35141472cff4c18 commit 1dad47f20ed56478257aad69c35141472cff4c18 Author: estark <estark@chromium.org> Date: Wed Oct 26 16:34:42 2016 Apply document's referrer policy on client-initiated reload location.reload() has apparently never applied the document's referrer policy to the reload request. Before https://codereview.chromium.org/2393633006, this was never noticed, because the net stack's information about referrer policies was too coarse to trigger its sanity check that the referrer matches the referrer policy. So the request would go through fine, but with the wrong referrer (e.g. a referrer of http://example.com/foo even if the document's referrer policy was 'origin'). But https://codereview.chromium.org/2393633006 gave the net stack finer-grained information about referrer policies, so that the sanity check does catch reload requests where the referrer does not match the referrer policy. On a DCHECK build, location.reload() on a page with a referrer policy of, e.g., 'origin' hits a NOTREACHED(). On a non-DCHECK build, location.reload() flashes a net error page because the request gets cancelled. This CL fixes this by using SecurityPolicy::generateReferrer() to set the referrer on the reload request. BUG= 658707 Review-Url: https://codereview.chromium.org/2450533002 Cr-Commit-Position: refs/heads/master@{#427712} [add] https://crrev.com/1dad47f20ed56478257aad69c35141472cff4c18/third_party/WebKit/LayoutTests/http/tests/security/referrer-on-client-reload.html [add] https://crrev.com/1dad47f20ed56478257aad69c35141472cff4c18/third_party/WebKit/LayoutTests/http/tests/security/resources/post-referrer-on-reload.html [modify] https://crrev.com/1dad47f20ed56478257aad69c35141472cff4c18/third_party/WebKit/Source/core/loader/FrameLoader.cpp
,
Oct 26 2016
,
Oct 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c8610f6fa44528bbe66c6d18f9ef5544b7ca7f8 commit 1c8610f6fa44528bbe66c6d18f9ef5544b7ca7f8 Author: Emily Stark <estark@google.com> Date: Wed Oct 26 22:33:08 2016 Apply document's referrer policy on client-initiated reload location.reload() has apparently never applied the document's referrer policy to the reload request. Before https://codereview.chromium.org/2393633006, this was never noticed, because the net stack's information about referrer policies was too coarse to trigger its sanity check that the referrer matches the referrer policy. So the request would go through fine, but with the wrong referrer (e.g. a referrer of http://example.com/foo even if the document's referrer policy was 'origin'). But https://codereview.chromium.org/2393633006 gave the net stack finer-grained information about referrer policies, so that the sanity check does catch reload requests where the referrer does not match the referrer policy. On a DCHECK build, location.reload() on a page with a referrer policy of, e.g., 'origin' hits a NOTREACHED(). On a non-DCHECK build, location.reload() flashes a net error page because the request gets cancelled. This CL fixes this by using SecurityPolicy::generateReferrer() to set the referrer on the reload request. BUG= 658707 Review-Url: https://codereview.chromium.org/2450533002 Cr-Commit-Position: refs/heads/master@{#427712} (cherry picked from commit 1dad47f20ed56478257aad69c35141472cff4c18) Review URL: https://codereview.chromium.org/2450343002 . Cr-Commit-Position: refs/branch-heads/2901@{#5} Cr-Branched-From: ee1035a7bb2231ad2192002434b030bad7050bbb-refs/heads/master@{#427541} [add] https://crrev.com/1c8610f6fa44528bbe66c6d18f9ef5544b7ca7f8/third_party/WebKit/LayoutTests/http/tests/security/referrer-on-client-reload.html [add] https://crrev.com/1c8610f6fa44528bbe66c6d18f9ef5544b7ca7f8/third_party/WebKit/LayoutTests/http/tests/security/resources/post-referrer-on-reload.html [modify] https://crrev.com/1c8610f6fa44528bbe66c6d18f9ef5544b7ca7f8/third_party/WebKit/Source/core/loader/FrameLoader.cpp |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by tkonch...@chromium.org
, Oct 24 2016Status: Untriaged (was: Unconfirmed)