sendBeacon does not honor meta referrer policy

VERIFIED FIXED in Firefox 53

Status

()

P2
normal
VERIFIED FIXED
3 years ago
a year ago

People

(Reporter: patrick.toomey, Assigned: tnguyen)

Tracking

39 Branch
mozilla55
Points:
---

Firefox Tracking Flags

(firefox53 fixed, firefox54 fixed, firefox55 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.130 Safari/537.36

Steps to reproduce:

1) Go to a site that has a meta referrer policy like (be sure you visit a page that has a subpath to show that the subpath is sent in the referrer):
    
    <meta content="origin" name="referrer" />
  
2) open the dev console and paste something like the following:
  
   navigator.sendBeacon("https://www.google-analytics.com/collect", "foo=bar");




Actual results:

Go to the network tab and observe that the full referrer is sent despite the fact that the site has an "origin" referrer policy set.


Expected results:

sendBeacon should respect the referrer policy the same way that any other XHR request would.
(Reporter)

Comment 1

3 years ago
A quick proof of concept page:

http://biasedcoin.com/meta-referrer-tests/origin.html

var http = new XMLHttpRequest();
var url = "https://www.google-analytics.com/collect";
var params = "foo=bar";
http.open("POST", url, true);
http.send(params);

sends: Referer:"http://biasedcoin.com"

navigator.sendBeacon("https://www.google-analytics.com/collect", "foo=bar");

sends: Referer:"http://biasedcoin.com/meta-referrer-tests/origin.html"

Updated

3 years ago
Component: Untriaged → DOM
Product: Firefox → Core
(Assignee)

Updated

2 years ago
Priority: -- → P2
Thomas, do you have any time to look at this one?  Its pretty clear we violate the spec here and it makes sendBeacon() less useful.  It seems sites like github are having compat issues with this.

It seems like this could be a fairly easy fix.  We need to add a WPT test, though.
Flags: needinfo?(tnguyen)

Comment 3

2 years ago
The issue is manifesting itself in sending sensitive path parameters to our analytics provider.
(Assignee)

Updated

2 years ago
Assignee: nobody → tnguyen
Flags: needinfo?(tnguyen)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8846936 - Flags: review?(ehsan)
(Assignee)

Comment 5

a year ago
Hi Ehsan,
Could you please take a look at this?
AFAIK, sendBeacon is not working in worker at the moment, so just use document referrer's policy when sending beacon.
I added some wpt tests, but have to move some of them to /mozilla/. The reason is we have to set some prefs to disable mixed content block to allow request downgrade from https -> http. I'm not sure we should push a WPT test that requires non-standard prefs like that
Comment hidden (mozreview-request)

Comment 7

a year ago
mozreview-review
Comment on attachment 8846936 [details]
Bug 1192940 - Support referrer policy in sendBeacon

https://reviewboard.mozilla.org/r/120016/#review122072

Thanks for the fix!

I think this is probably worth backporting to branches, given that the fix should be low risk and it makes us honour the referrer policy which is important.  Can you please nominate this for an uplift?  Thanks!
Attachment #8846936 - Flags: review?(ehsan) → review+

Comment 8

a year ago
(In reply to Thomas Nguyen[:tnguyen] ni plz from comment #5)
> Hi Ehsan,
> Could you please take a look at this?
> AFAIK, sendBeacon is not working in worker at the moment, so just use
> document referrer's policy when sending beacon.
> I added some wpt tests, but have to move some of them to /mozilla/. The
> reason is we have to set some prefs to disable mixed content block to allow
> request downgrade from https -> http. I'm not sure we should push a WPT test
> that requires non-standard prefs like that

There are other similar tests already, for example: <http://searchfox.org/mozilla-central/rev/a5c2b278897272497e14a8481513fee34bbc7e2c/testing/web-platform/meta/service-workers/service-worker/fetch-event.https.html.ini#3>
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Keywords: checkin-needed
(Assignee)

Comment 11

a year ago
Created attachment 8848376 [details] [diff] [review]
Support referrer policy in sendBeacon - Aurora
(Assignee)

Comment 12

a year ago
Comment on attachment 8848376 [details] [diff] [review]
Support referrer policy in sendBeacon - Aurora

Approval Request Comment
[Feature/Bug causing the regression]: No
[User impact if declined]: Leak referrer when sendbeacon
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: See comment 0
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: Low risky
[Why is the change risky/not risky?]: User referrer policy of doc instead of default
[String changes made/needed]: No
Attachment #8848376 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

a year ago
Attachment #8848376 - Attachment description: Support referrer policy in sendBeacon → Support referrer policy in sendBeacon - Aurora
(Assignee)

Comment 13

a year ago
Created attachment 8848381 [details] [diff] [review]
Support referrer policy in sendBeacon-beta
(Assignee)

Comment 14

a year ago
Comment on attachment 8848381 [details] [diff] [review]
Support referrer policy in sendBeacon-beta

Approval Request Comment
[Feature/Bug causing the regression]: no
[User impact if declined]: leak referrer when sendbeacon
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: comment 0
[List of other uplifts needed for the feature/fix]: aurora
[Is the change risky?]: low risky
[Why is the change risky/not risky?]: Change correct referrer when sendbeacon
[String changes made/needed]: no
Attachment #8848381 - Attachment description: Support referrer policy in sendBeacon → Support referrer policy in sendBeacon-beta
Attachment #8848381 - Flags: approval-mozilla-beta?

Comment 15

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/eb872c42dc7f
Support referrer policy in sendBeacon r=Ehsan
Keywords: checkin-needed

Comment 16

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/eb872c42dc7f
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)
Doesn't seem to apply
grafting 386618:eb872c42dc7f "Bug 1192940 - Support referrer policy in sendBeacon r=Ehsan"
merging dom/base/Navigator.cpp
merging testing/web-platform/meta/MANIFEST.json
merging testing/web-platform/mozilla/meta/MANIFEST.json
warning: conflicts while merging dom/base/Navigator.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging testing/web-platform/meta/MANIFEST.json! (edit, then use 'hg resolve --mark')
warning: conflicts while merging testing/web-platform/mozilla/meta/MANIFEST.json! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(tnguyen)
status-firefox53: --- → affected
status-firefox54: --- → affected
Comment hidden (typo)
(Assignee)

Comment 20

a year ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #18)
> Doesn't seem to apply
> grafting 386618:eb872c42dc7f "Bug 1192940 - Support referrer policy in
> sendBeacon r=Ehsan"
> merging dom/base/Navigator.cpp
> merging testing/web-platform/meta/MANIFEST.json
> merging testing/web-platform/mozilla/meta/MANIFEST.json
> warning: conflicts while merging dom/base/Navigator.cpp! (edit, then use 'hg
> resolve --mark')
> warning: conflicts while merging testing/web-platform/meta/MANIFEST.json!
> (edit, then use 'hg resolve --mark')
> warning: conflicts while merging
> testing/web-platform/mozilla/meta/MANIFEST.json! (edit, then use 'hg resolve
> --mark')
> abort: unresolved conflicts, can't continue
> (use 'hg resolve' and 'hg graft --continue')

Which branch did you fail to merge? aurora, beta?
It seems ok to me,  comment 12 for aurora, and comment 14 for beta
I was able to reproduce the bug on a Nightly 54.0a1, Build ID: 20170210030206.

I can confirm this is fixed on latest Nightly build 55.0a1, Build ID 20170320030209, on Windows 10x64, Mac OS X 10.10 and Ubuntu 16.04. Verified on site
http://biasedcoin.com/meta-referrer-tests/origin.html

and navigator.sendBeacon("https://www.google-analytics.com/collect", "foo=bar");
 sends only: Referer:"http://biasedcoin.com".
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
Flags: needinfo?(brindusa.tot)
Comment on attachment 8848376 [details] [diff] [review]
Support referrer policy in sendBeacon - Aurora

Fix sendBeacon API and was verified. Aurora54+ & Beta53+.
Attachment #8848376 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8848381 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
sorry had to back this out from beta for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=85588158&repo=mozilla-beta
status-firefox53: fixed → affected
Flags: needinfo?(tnguyen)
My bad, we are testing a script, sorry!
Flags: needinfo?(sledru)
(Assignee)

Comment 27

a year ago
Created attachment 8850460 [details] [diff] [review]
Support referrer policy in sendBeacon-beta

MozReview-Commit-ID: K9szPNxXtrf
(Assignee)

Comment 28

a year ago
(In reply to Carsten Book [:Tomcat] from comment #25)
> sorry had to back this out from beta for failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=85588158&repo=mozilla-
> beta

That is wpt json conflict. Ran ./mach wpt-manifest-update and it should be ok now
Flags: needinfo?(tnguyen)
(Assignee)

Updated

a year ago
Attachment #8848381 - Attachment is obsolete: true
(Assignee)

Comment 29

a year ago
Comment on attachment 8850460 [details] [diff] [review]
Support referrer policy in sendBeacon-beta


Approval Request Comment
[Feature/Bug causing the regression]: no
[User impact if declined]: leak referrer when sendbeacon
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: comment 0
[List of other uplifts needed for the feature/fix]: aurora
[Is the change risky?]: low risky
[Why is the change risky/not risky?]: Change correct referrer when sendbeacon
[String changes made/needed]: no

Request beta uplift again
Attachment #8850460 - Attachment description: Support referrer policy in sendBeacon r=ehsan, a=gchang → Support referrer policy in sendBeacon-beta
Attachment #8850460 - Flags: approval-mozilla-beta?

Comment 30

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/2e62913dde4d
status-firefox53: affected → fixed
Comment on attachment 8850460 [details] [diff] [review]
Support referrer policy in sendBeacon-beta

Retroactive beta re-approval!
Attachment #8850460 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.