New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 658707 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Referrer policy is not applied to requests initiated via location.reload()

Reported by dmascare...@etouch.net, Oct 24 2016

Issue description

Chrome 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

 
Actual_webstore.mp4
872 KB View Download
Labels: ReleaseBlock-Stable Needs-Bisect
Status: Untriaged (was: Unconfirmed)
Adding RB label as this is a recent regression
Cc: kkaluri@chromium.org
Labels: -Needs-Bisect hasbisect-per-revision
Owner: est...@chromium.org
Status: Assigned (was: Untriaged)
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.!

Comment 3 by est...@chromium.org, Oct 24 2016

Cc: jochen@chromium.org
Components: -Platform>Extensions Blink>SecurityFeature
Labels: OS-Android OS-Chrome
Summary: Referrer (was: Regression: Glimpse of error message is seen while enabling the disabled extension on Web store page.)
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.

Comment 4 by est...@chromium.org, Oct 24 2016

Summary: Referrer policy is not applied to requests initiated via location.reload() (was: Referrer)

Comment 5 by est...@chromium.org, Oct 24 2016

Labels: Security_Impact-None
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.
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 25 2016

Labels: -ReleaseBlock-Stable
Project Member

Comment 7 by bugdroid1@chromium.org, 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

Comment 8 by est...@chromium.org, Oct 26 2016

Status: Fixed (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 26 2016

Labels: merge-merged-2901
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