The App Install Prompt isn't modal to an app - which allows for a webpage to forcibly annoy a user across the phone until an app is installed

VERIFIED FIXED in Firefox 21

Status

Core Graveyard
DOM: Apps
P3
normal
VERIFIED FIXED
5 years ago
29 days ago

People

(Reporter: jsmith, Assigned: fabrice)

Tracking

({sec-high})

unspecified
B2G C4 (2jan on)
ARM
Gonk (Firefox OS)
sec-high

Firefox Tracking Flags

(blocking-basecamp:+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, firefox-esr17 unaffected, b2g18 fixed)

Details

(Whiteboard: [adv-main21-] berlinww,, URL)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 698299 [details]
Proof of Concept Test Case

Steps:

1. Load http://jds2501.github.com/webapi-permissions-tests/repeatedinstall.html in the browser
2. Cancel the install
3. Exit to the homescreen

Expected:

The app install prompt shouldn't appear on top of the homescreen - it should remain modal to the browser.

Actual:

The app install prompt appears on top of the homescreen or generally, any part of the phone every time you try to cancel install.

This is a security problem - an evil web developer could essentially force a user into installing their evil app if they setup a test page like what's seen attached to repeatedly annoy the user across the phone until they install the app.

This is the app install prompt equivalent of bug 826023. The more I see these type of bugs, the more I think we need to audit our system prompts to make sure they are modal.
(Reporter)

Updated

5 years ago
Blocks: 802574
blocking-basecamp: --- → ?
(Reporter)

Updated

5 years ago
Summary: The App Install Prompt isn't modal - which allows for a webpage to forcibly annoy a user until an app is installed → The App Install Prompt isn't modal - which allows for a webpage to forcibly annoy a user across the phone until an app is installed
(Reporter)

Updated

5 years ago
Summary: The App Install Prompt isn't modal - which allows for a webpage to forcibly annoy a user across the phone until an app is installed → The App Install Prompt isn't modal to an app - which allows for a webpage to forcibly annoy a user across the phone until an app is installed
(Reporter)

Comment 1

5 years ago
Actually this doesn't need to be security sensitive given that the other equivalent bug isn't either.
Group: core-security
Assignee: nobody → etienne
blocking-basecamp: ? → +
Priority: -- → P3
Target Milestone: --- → B2G C4 (2jan on)
(Reporter)

Comment 2

5 years ago
Actually let's stick this back under security - it's sec-high because it's forcing the user to do an install per talking with Paul.
Group: core-security
Keywords: sec-high
Talked with Fabrice, his opinion is: this is more of a global issue (same for starting web activities and popups) that shouldn't block.
blocking-basecamp: + → ?
(Reporter)

Comment 4

5 years ago
(In reply to Etienne Segonzac (:etienne) from comment #3)
> Talked with Fabrice, his opinion is: this is more of a global issue (same
> for starting web activities and popups) that shouldn't block.

Why? If it's a global issue that would be more likely to imply that it blocks.

The fact that you can force a user into installing apps diminishes a user's choice onto how they control their device. Which is why it was marked sec-high.

I can definitely through together a more dangerous PoC pretty easily that could probably start an endless loop of installing apps I want on the user's device until I take control of all of the storage on the device.
(Reporter)

Comment 5

5 years ago
I should note that this does *not* happen with alerts/confirms because we made sure those were modal.
(In reply to Jason Smith [:jsmith] from comment #5)
> I should note that this does *not* happen with alerts/confirms because we
> made sure those were modal.

True.
Sadly the installation prompts come from a mozChromeEvent (as opposed to a mozbrowser event on the iframe calling it) so we don't know with app/browser tab to attach it to.

This should be triaged in platform triage.
Assignee: etienne → nobody
Component: Gaia::System → General
Could the work on Bug 826023 fix this bug too ? Or am I completely out of here ?

Comment 8

5 years ago
Not blocking v.1 on this bug.

We probably will want UX input on this bug so that we know how to correct address this problem (maybe it is just making this dialog modal to the browser app)
blocking-basecamp: ? → -
tracking-b2g18: --- → +
Whiteboard: berlinww,
(Reporter)

Comment 9

5 years ago
(In reply to Doug Turner (:dougt) from comment #8)
> Not blocking v.1 on this bug.
> 
> We probably will want UX input on this bug so that we know how to correct
> address this problem (maybe it is just making this dialog modal to the
> browser app)

The fact that you can forcibly get app installations on a user's device in a repeated chain with uncontrolled user consent doesn't sound like something we could ship with. I still think this blocks.

And I have a feeling I could pull off this bug in an app as well.

Renom.
blocking-basecamp: - → ?
Flags: needinfo?(ladamski)
(Assignee)

Comment 10

5 years ago
(In reply to Jason Smith [:jsmith] from comment #9)

> The fact that you can forcibly get app installations on a user's device in a
> repeated chain with uncontrolled user consent doesn't sound like something
> we could ship with. I still think this blocks.

You don't get the app installed, but only the install dialog to show up, right?
(Reporter)

Comment 11

5 years ago
(In reply to Fabrice Desré [:fabrice] from comment #10)
> (In reply to Jason Smith [:jsmith] from comment #9)
> 
> > The fact that you can forcibly get app installations on a user's device in a
> > repeated chain with uncontrolled user consent doesn't sound like something
> > we could ship with. I still think this blocks.
> 
> You don't get the app installed, but only the install dialog to show up,
> right?

The install dialog, right. It doesn't automatically install the app. The user would still have to hit install. In the case of the PoC I'm describing, if I hit cancel, I'll just prompt to install again across any part of the phone.

If I was to do a repeated chain, I would just start another app install immediately after the first app is install. So the evil web developer once a user visits their target page can control what the user installs onto their device with uncontrolled user consent (i.e. cancel always reprompts).

I'll experiment to see what work-arounds are available. If I'm able to kill the phone easily when this happens, then I think it's worth reanalyzing to see if this blocks (in other words, how easily can you hit the eject button if you end up in this state).
(Reporter)

Comment 12

5 years ago
Actually the eject button does exist here - if the user entered this state and wanted to hit eject, they could hold the power button down and power down or restart the phone.
(Reporter)

Comment 13

5 years ago
(In reply to Jason Smith [:jsmith] from comment #12)
> Actually the eject button does exist here - if the user entered this state
> and wanted to hit eject, they could hold the power button down and power
> down or restart the phone.

Talked with Lucas.

There's two eject buttons here - power down or restart the phone or kill the app process through the task manager.
I tested this on my phone and its pretty obnoxious.  The task manager is the easiest workaround but its clearly not obvious.  I think the gecko part of the fix for bug 826023 will solve most of the problem, and then we just need to implement the getVisible() check in Gaia.  So I'm going to renominate this as a Gaia bug and recommend basecamp-blocking, dependent on 826023 sticking and Etienne's feedback to confirm/deny my theory.
Component: General → Gaia::System
Flags: needinfo?(ladamski)
Flags: needinfo?(etienne)
(In reply to Lucas Adamski from comment #14)
> I tested this on my phone and its pretty obnoxious.  The task manager is the
> easiest workaround but its clearly not obvious.  I think the gecko part of
> the fix for bug 826023 will solve most of the problem, and then we just need
> to implement the getVisible() check in Gaia.  So I'm going to renominate
> this as a Gaia bug and recommend basecamp-blocking, dependent on 826023
> sticking and Etienne's feedback to confirm/deny my theory.

Not prompting for install if the frame is hidden should be pretty straightforward (and I'm happy to do it), but I think it'll still need to live in dom/app (not gaia).

Fabrice will confirm.
Flags: needinfo?(etienne)
blocking-basecamp: ? → +
tracking-b2g18: + → ---
Just discussed with Fabrice and this is likely dom/apps.
Component: Gaia::System → DOM: Apps
Product: Boot2Gecko → Core
(Reporter)

Updated

5 years ago
(Assignee)

Comment 17

5 years ago
Created attachment 699186 [details] [diff] [review]
patch

This patch prevents install() and installPackage() to work on background pages.
Assignee: nobody → fabrice
Attachment #699186 - Flags: review?(21)
Comment on attachment 699186 [details] [diff] [review]
patch

r+ if you rename the isForeground -> to ensureForeground (otherwise the method is lying)
Attachment #699186 - Flags: review?(21) → review+
(Assignee)

Comment 19

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/65517a277d6b
(Assignee)

Comment 20

5 years ago
https://hg.mozilla.org/releases/mozilla-b2g18/rev/26b810ee919d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Hasn't hit m-c yet.
Status: RESOLVED → REOPENED
status-b2g18: --- → fixed
status-firefox19: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → fixed
Resolution: FIXED → ---
(Reporter)

Comment 22

5 years ago
(In reply to Ryan VanderMeulen from comment #21)
> Hasn't hit m-c yet.

For b2g work week we are closing bugs when they hit inbound + b2g18. Reclosing.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(In reply to Jason Smith [:jsmith] from comment #22)
> (In reply to Ryan VanderMeulen from comment #21)
> > Hasn't hit m-c yet.
> 
> For b2g work week we are closing bugs when they hit inbound + b2g18.
> Reclosing.

FWIW, I don't know what sense this makes. What does resolving the bug do (and by extension throwing out conventions that have been around for years) that status flags can't accomplish? We have status-b2g18:fixed specifically for tracking branch uplifts.

https://hg.mozilla.org/mozilla-central/rev/65517a277d6b
(Reporter)

Comment 24

5 years ago
(In reply to Ryan VanderMeulen from comment #23)
> (In reply to Jason Smith [:jsmith] from comment #22)
> > (In reply to Ryan VanderMeulen from comment #21)
> > > Hasn't hit m-c yet.
> > 
> > For b2g work week we are closing bugs when they hit inbound + b2g18.
> > Reclosing.
> 
> FWIW, I don't know what sense this makes. What does resolving the bug do
> (and by extension throwing out conventions that have been around for years)
> that status flags can't accomplish? We have status-b2g18:fixed specifically
> for tracking branch uplifts.
> 
> https://hg.mozilla.org/mozilla-central/rev/65517a277d6b

It's some work week special case for tracking - they wanted bugs closed if they hit b2g18 (the branch that we ship with). I agree this doesn't work on normal circumstances though.
I agree with Ryan here; focusing on numbers is so bad. Well well.
(In reply to Jason Smith [:jsmith] from comment #24)
> It's some work week special case for tracking - they wanted bugs closed if
> they hit b2g18 (the branch that we ship with). I agree this doesn't work on
> normal circumstances though.

I guess what I'm failing to understand is what this is even trying to accomplish. Presumably they're trying to track their remaining blockers. Presumably they're using bug queries for that. A bug query can just as easily use the status flag as the bug resolution. I can understand the need to streamline the landing & uplift process under the tight deadlines posed here, but I fail to see how changing this specific practice does anything to aid that cause.
(Reporter)

Updated

5 years ago
Keywords: verifyme
QA Contact: jsmith
(Reporter)

Comment 27

5 years ago
Verified on today's build. Looks good!
Status: RESOLVED → VERIFIED
Keywords: verifyme
status-firefox-esr17: --- → unaffected
Whiteboard: berlinww, → [adv-main21+] berlinww,
Whiteboard: [adv-main21+] berlinww, → [adv-main21-] berlinww,
Group: core-security

Updated

29 days ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.