1306334 - [meta] Protection from sites that try to download multiple files automatically

Status: NEW

(Firefox :: File Handling, defect, P3)

Description

[:Paolo Amadini]

This meta-bug is about providing protection from sites that try to download multiple files automatically, and is a prerequisite for the removal of the handler choice dialog that partly fulfills this function.

Comment 1

[Ben Tian (inactive)]

Paolo,

Can you explain how to orchestrate the 4 dependent bugs to protect from sites downloading multiple files automatically?

I see these bugs compute 1) number of user triggered downloads and 2) number of downloads started per origin, but don't get their relationship with detection of sites downloading multiple files.

Comment 2

[:Paolo Amadini]

Sorry for the delay, last week I was able to do the breakdown but I didn't have the time to add all the details that would have been needed for this meta-bug.

Fundamentally, we should protect the user from automatic downloads that can be started by any web page using a test case like this:

setInterval(function() {
  window.location = "<resource with Content-Disposition: attachment>";
}, 5000);

At the same time, we don't want to show unnecessary prompts for legitimate uses of the "window.location" assignment pattern.

To achieve this, we can always allow the first download from a web page, and show a permission prompt only if the page tries to download more files. A simple way to do this is to count the number of downloads based on the origin of the page that started the download. This is bug 1306352, that can implement the permission and determine when to show it with this simple method.

The above solution, by itself, has the problem that downloads started by clicking on a link would be treated in the same way. For example, clicking on various files from a page that shows a directory listing would display the prompt at the second click. To solve this, we should make sure that downloads started by the user are always allowed, and are not added to the count of downloads started automatically by the origin. This is bug 1306351.

The two bugs can be done in parallel, because we can land the first bug 1306352 without the second bug 1306351 as long as we keep the feature disabled. We probably want to keep the new feature disabled anyways at first, until we can confirm that it works well with real-world sites.

To verify how the feature behaves once enabled, we can measure the interaction with the permission prompt. For example, if 100% of the users allow the prompt, it indicates that the prompt is not useful, and maybe we should change the limits or investigate other heuristics. We already have probes for the permission prompts, but they will expire, so there is bug 1306355 to ensure that we have the specific data we need. This can be implemented on top of the code in bug 1306351.

We can also check if we correctly detect downloads started by the user in the wild. Again, if a very high number of downloads is detected as automatic, there is probably something wrong with the heuristic. This is bug 1306354, and should be done after bug 1306351 because the former depends on how we implement the latter.

Comment 3

[Ben Tian (inactive)]

I see. It seems to me bug 1306352 and bug 1306351 implement the detection itself, and bug 1306355 and bug 1306354 collect data and verify the implementation.

As a reference, Chrome's approach [1] allows only first download from a web page (limit=1) and regards downloads as user-triggered once 'user clicks the mouse, presses enter, the space bar or navigates to another page.' We may start implementation by referring it.

[1] https://cs.chromium.org/chromium/src/chrome/browser/download/download_request_limiter.h?sq=package:chromium&dr=CSs&rcl=1475194374&l=33

Comment 4

[:Paolo Amadini]

(In reply to Ben Tian [:btian] from comment #3)
> As a reference, Chrome's approach [1] allows only first download from a web
> page (limit=1) and regards downloads as user-triggered once 'user clicks the
> mouse, presses enter, the space bar or navigates to another page.' We may
> start implementation by referring it.

Ah, that's very useful, thanks. It seems that Chrome associates the state to the current tab, and resets it when navigating to a different host. This has some similarities to how we're handling temporary permissions in bug 1206232, with the difference that any interaction with the page would also grant the site one other download, even if it happens later than when the interaction occurred. This probably means that the permission would be granted to the current tab, and not permanently to all tabs from the same domain.

In Firefox we also have EventStateManager::IsHandlingUserInput() that is used by permissions like Pointer Lock to determine if they should be granted automatically. For downloads, this would work by excluding downloads started in this way from the downloads that we are limiting. This would involve propagating this state through the subsequent network loads, which I'm not sure we currently do.

I'm fine with both approaches. Boris, do you know if we support something like a flag indicating if a load was started by a user click, rather than a timeout like in the test case in comment 2?

Also, do you see other pros and cons to the two different approaches for limiting downloads, that we may have missed?

Comment 5

[Boris Zbarsky [:bzbarsky]]

We don't have an existing flag for that.  Note that to some extent this is similar to bug 670328...

The discussion in that bug proposes logic similar to the popup blocker, which is not quite the same as IsHandlingUserInput(), because it propagates across timeouts to a limited extent.  Using a more stringent thing here is probably OK given that the user will have a way to opt in to the blocked download.

I'm not quite sure which specifc "two different approaches" you mean.  Are they both described in comment 4, or is one of the them the thing from comment 2?

Comment 6

[:Paolo Amadini]

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #5)
> I'm not quite sure which specifc "two different approaches" you mean.  Are
> they both described in comment 4, or is one of the them the thing from
> comment 2?

One approach is the one from comment 2, that is limiting downloads by origin and excluding those started when IsHandlingUserInput(). The other approach is the one described in the Chromium code comment pointed to by comment 3, that is limiting downloads by tab and resetting the count on any relevant user interaction or cross-origin navigation.

Comment 7

[:Paolo Amadini]

...with the "count" probably being 1 download, unless we find out it's too stringent.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Ah.  It seems to me that the "other approach" is slightly more open to abuse, but also less likely to fall into the "why doesn't this work?  Oh, it's because of this other tab I opened two weeks ago!" failure mode...

Comment 9

[:Paolo Amadini]

Yes, it seems to me that Chrome's approach that Ben reported in comment 3 is the clearest from a user perspective. Ben, let me know if you have comments on either approach.

Boris, I'm not sure how we could detect the interactions with the page to reset the "one download allowed" flag. Can we just set up some event listeners in JavaScript, or would it impact performance since we have to do it on every page and for all elements? Maybe there is a simpler way to reset a flag when IsHandlingUserInput becomes true?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Could you use the same thing we use for skipping beforeunload prompts?  That is, nsIDocument::UserHasInteracted() and the place where SetUserHasInteracted() is called?

You should figure out whether you want to propagate your thing up the document parent chain or not...

In general, doing a bunch of work here for every event would be a bit annoying.

Comment 11

[Ben Tian (inactive)]

For reference, UX spec for multiple download protection.
https://mozilla.invisionapp.com/share/4Y6ZZH1E8#/screens/188580116

Comment 12

[Ben Tian (inactive)]

(In reply to :Paolo Amadini from comment #4)
> This has some similarities to how we're handling temporary permissions in bug
> 1206232

Paolo, does this bug depend on bug 1206232 that supports 'Blocked Temporarily' state? Since comment 11 UX flow sets 'Blocked Temporarily' permission to block multiple downloads temporarily.

Comment 13

[:Paolo Amadini]

If we go with Chrome's approach and the technical solution that Boris suggested in comment 10, then the mechanism with which we keep the state would be very different than the one used by the "Blocked Temporarily" case, discussed in bug 1206232 comment 36.

So, bug 1206232 does not block the work here, but they may share the new strings for "Blocked Temporarily". Since there is no case of permanent block for downloads, we can also use the normal "Blocked" strings if we need to land the bugs here before bug 1206232 is ready.

Comment 14

[Alphan Chen [:alchen]]

(In reply to :Paolo Amadini from comment #13)
> If we go with Chrome's approach and the technical solution that Boris
> suggested in comment 10, then the mechanism with which we keep the state
> would be very different than the one used by the "Blocked Temporarily" case,
> discussed in bug 1206232 comment 36.
> 

Let us use this approach to doing an implementation first.
At the beginning, we need to find out a way to identify "user-triggered".

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

Paolo, is this still an active project?

Comment 17

[:Paolo Amadini]

This isn't active at this time.

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot-2017-12-12 5 9 Protection For Multiple Automatic Downloads.jpg

Comment 19

[Anwesh]

Hey I want to work on this. Where do I start? I am also thinking of applying to GSOC with this

Comment 20

[Johann Hofmann [:johannh]]

(In reply to Anwesh from comment #19)
> Hey I want to work on this. Where do I start? I am also thinking of applying
> to GSOC with this

Hi there, thanks for your interest! We'd like to reserve this work for the GSOC project. You should take a look at https://wiki.mozilla.org/Community:SummerOfCode18 to learn more about applying for GSOC.

Feel free to email one of us if you have any additional questions!

Comment 21

[omkar]

Hello everyone, I'm also interested to work on this bug as a GSoC student

Comment 22

[Sumedh Nimkarde]

Hello everyone, I am interested to work on the idea "Download Spam Protection for Firefox". Please guide me where should I start.

Comment 23

[Payal]

Konichiwa! , Can anyone tell me the IRC channel to discuss about this bug:)

Comment 24

[shreya singhai]

(In reply to Ben Tian (inactive) from comment #3)
>https://cs.chromium.org/chromium/src/chrome/browser/download/download_request_limiter.h?sq=package:chromium&dr=CSs&rcl=1475194374&l=33

This link does not work ! It says- Can't load 'chromium/src/chrome/browser/download/download_request_limiter.h': Could not resolve path src/chrome/browser/download/download_request_limiter.h.
  Is there any way I can open it ?
Also I'm interested in applying this project in GSoC 2018 , can anyone tell me how can i move further on this project ?

Comment 25

[:Paolo Amadini]

(In reply to shreya singhai from comment #24)
> Also I'm interested in applying this project in GSoC 2018 , can anyone tell
> me how can i move further on this project ?

Hi! If you're interested in this project and have questions, as a first step you can contact the mentor directly using the contact information in this page:

https://wiki.mozilla.org/Community:SummerOfCode18

Comment 27

[Varin Nair]

Hi, is this bug still open to non-GSoC programmers?

Comment 28

[Sagar Bharadwaj]

Hi Varin,

I'm working on this bug as a part of my GSoC project. See this bug : https://bugzilla.mozilla.org/show_bug.cgi?id=1463527