1367626 - browser.downloads.download() should allow setting Referer (and others)

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement, P5)

Description

[none11given]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36

Steps to reproduce:

I understand that Referer is defined as a forbidden header and all. It's fine to prevent altering it in page scripts using XMLHttpRequest.

But WebExtensions (specifically background scripts) are privileged code. There's no good reason for the downloads API of WebExtensions to be restricted from setting a Referer header, when WebExtensions can do much more obviously dangerous stuff. In fact, hooking into onBeforeSendHeaders in the webRequest API allows me to modify any header I want, including Referer!

We need one of two things: allow setting Referer in the headers property of the options object we pass to download(), or have the request made by download() be subject to webRequest hooks.

I would be overjoyed to submit the code changes to enable the first option.

Comment 1

[Andy McKay]

This has been added to the agenda for the May 30 WebExtensions Triage meeting at 10:30am PT.

Call in info: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Details_.26_How_to_Join
Agenda: https://docs.google.com/document/d/1hKKRpGFIaAaI3G_HfPX2Nk8pCchyhUIKJB9y5sIrVV4/edit

Comment 2

[Shane Caraveo (:mixedpuppy)]

none11given: You're asking for a feature without any use case.  Why do you need this?

Comment 3

[none11given]

The use case is the same as for why an onBeforeSendHeaders hook lets my set the referrer. I'm the one making the request and I ought to be able to configure it how I want.

If you want a specific one for me, I use it in my personal "single click image download" extension in order to download from websites that check your referrer as an anti-leech measure.

Comment 4

[Caitlin Neiman [:caitmuenster]]

Shane, circling back to you about this -- from the meeting notes, it looks like this was approved, P5. Does that still sound right?

Comment 7

[pikashi]

This is not a small problem. At least on this extension ( https://github.com/harytfw/GlitterDrag ), downloads.download()'s lack of referer settings cause big problems.
I think at least the referer should be setted the same as current page url in downloads.download() rather than a blank value.

Comment 8

[pikashi]

I think the referer must be setted, because the referer's importance is as same as cookies to recognize the user's on many websites.

Comment 9

[Firefox Bug Husbandry Bot]

Bulk move of bugs per https://bugzilla.mozilla.org/show_bug.cgi?id=1483958

Comment 10

[Tom S [:evilpie]]

This is something that would be useful to DownThemAll! (https://github.com/downthemall/downthemall/blob/master/TODO.md) and should be really simple to implement.

Comment 11

[Tom S [:evilpie]]

Echoing comment 8, if we allow Referer, we should also consider allowing Cookie, both are useful for downloading from pages that require login.
I see we copied the headers restrictions from Chrome, do we have an idea of the rational here? I would imagine if we allow Cookie you could trigger remote endpoints with the users' credentials, which might be unexpected for an extension that only has the download permission, but no hosts permission.

Comment 12

[Tom S [:evilpie]]

Oh, the users' cookie is automatically included anyway. In that case I see no problem with also allowing Referer. Even setting Cookie might be okay, because if we already provide the user's cookie, what could really be worse?

Comment 13

[Tom S [:evilpie]]

Attachment: Bug 1367626 - browser.downloads.download() should allow setting Referer. r?zombie!

Comment 14

[Tomislav Jovanovic :zombie]

I see we copied the headers restrictions from Chrome, do we have an idea of the rational here?

It might be from Chrome, but the list actually references the Fetch spec. It probably made sense to copy Chrome if we're going for compat, and It was a safe choice by default, without going into details of each header.

https://fetch.spec.whatwg.org/#forbidden-header-name

About Referer specifically, I can see two subtle issues:

• If your extension has a.com host permission, it can modify the download headers and insert b.com as Referer,
• but If it has b.com permission, there are ways to trigger a load from a.com with b.com as a Referer (ie embedding an image).

I'm not sure which one is more surprising, but since either workaround doesn't already require both permissions, I think we can allow the header for downloads without any host permissions.

I would imagine if we allow Cookie you could trigger remote endpoints with the users' credentials, which might be unexpected for an extension that only has the download permission, but no hosts permission.

Cookies already require host permissions, so we wouldn't want to relax that. Checking permissions would be possible, but not sure it's worth it without a good use case that isn't covered by just using the cookies api.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5b29024aa67
browser.downloads.download() should allow setting Referer. r=zombie

Comment 16

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/f5b29024aa67

Comment 17

[Alex Cornestean]

Hello,

Will this fix require manual validation? If yes, please provide some steps to reproduce in order to correctly test it and also, please set the "qe-verify+" flag. Otherwise, could the "qe-verify-" flag be added? Thanks!

Comment 18

[Tom S [:evilpie]]

No thank you. I am sure the automated test is enough.

Comment 19

[Alex Cornestean]

Thank you. Setting the "qe-verify-" flag as per the above comment.

Comment 20

[Richard Bloor]

Hi Tom, looking for some advice on the documentation changes needed. Is this as simple as adding:

referrer | optional
A string representing the URL of the document requesting the download. Use this parameter where the site you are downloading from expects requests from specific pages.

to downloads.download?

With the browser compatibility details updated to show support from Firefox 70?

Thanks!

Comment 21

[Tom S [:evilpie]]

No, Referer can be used inside with the headers field now. So the sentence

restricted to those allowed by XMLHttpRequest.

needs to be adjusted like

restricted to those allowed by XMLHttpRequest except for Referer.

Comment 22

[Nils Maier [:nmaier]]

Not "except" really... That, to me, implies Referer is not allowed.

So maybe something like this, to avoid ambiguity.

An array of objects representing extra HTTP headers to send with the request if the URL uses the HTTP[s] protocol. Each header is represented as a dictionary object containing the keys name and either value or binaryValue. Allowed are the same headers that are allowed by XMLHttpRequest. Firefox 70 and later additionally allows the Referer header. Any attempt to use any header that is not allowed will throw an error.

(Changes in italics)

And for the compat table, it should be noted that Referer is an allowed header since Firefox 70, but not allowed in other browsers (yet).

Comment 23

[Nils Maier [:nmaier]]

hmm... The XMLHttpRequest link should better be replaced by a link to https://developer.mozilla.org/en-US/docs/Glossary/Forbidden_header_name

And that is a list of blacklisted headers not a list of whitelisted ones.

So maybe

An array of objects representing extra HTTP headers to send with the request if the URL uses the HTTP[s] protocol. Each header is represented as a dictionary object containing the keys name and either value or binaryValue. Forbidden are the same headers that are forbidden by XMLHttpRequest and fetch, except Firefox 70 and later also allows the use of the Referer header. Any attempt to use any forbidden header will throw an error.

Comment 24

[Richard Bloor]

Many thanks for the feedback and suggestions. I've made the changes to downloads.download() as well as the browser compatibility table.