807557 - Prevent background apps from launching Web Activities

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect, P2)

Description

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Apps can currently initiate web activities from the background. No background app should be able to influence anything displayed (unless they have the attention_screen permission) to the user as this might lead to security problems. I think this will probably be mitigated by bug 794407 but wanted to call it out explicitly to make sure.

Comment 1

[(inactive after 6/18) Alive Kuo [:alive]]

All list menu based XXXX menus have the same problem.

Comment 2

[Mounir Lamouri (:mounir)]

Paul, what kind of "explicit fix" are you looking for? I feel like trying to fix something for "background apps" might be hard unless this is a hack because the platform doesn't really have such a concept (it's currently a Gaia hack).
Do you expect background apps to simply not call activities? I feel like bug 794407 would be enough to prevent that.
Alive, what fix do you have in mind?

Comment 3

[Paul Theriault [:pauljt] (no longer reading bugmail)]

So my understanding of the Window Manager in Gaia is that there is always one app which is currently the foreground app. I thought it was a security requirement that only the current foreground app can display anything on the screen, with the exception of the attention screen (or the system app I suppose). If a background app can show things to the user, or somehow interfere with the foreground app, this might lead to security issues. 

In this specific case, I can't think of an exploitation case off the top of my head, but I haven't really looked at web activities all that much yet. The main reason I lodged this bug was that I wasn't certain that  bug 794407 would effectively exclude this from happening, but if you are comfortable that it does, then I am happy to mark this as a dupe. 

Alive, can you elaborate on what you mean by 'all the list based xxxx menus have the same problem'?

Comment 4

[(inactive after 6/18) Alive Kuo [:alive]]

Sorry I know less about gecko but I don't see anywhere the patch deals with app visibility. Maybe it does.

For gaia, activity menu is based on list menu, which doesn't prevent background app to request a menu.
Context menu is also based on list menu in system app so it has the same problem. This should be fixed later, but maybe we are O.K. to ship with it. I have no idea if gaia could know who(app) is asking an activity.

Comment 5

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Ok I see what you mean. But as you say, at this point: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/activities.js#L27

You only have the detail object, and as far as I cant work it, it doesn't have the origin of the original caller.

What I was thinking of, was something like the check in navigator.pay (checking docshell.active): http://mxr.mozilla.org/mozilla-central/source/dom/payment/Payment.js#50

But I'll it up to you Mounir - TBH I dont completely understand how nsEventStateManager::IsHandlingUserInput() works, but if you are comfortable that this control will prevent background apps spawning activities I think that will be ok.

I assume there might still be an edge case like if an app fired an activity on close or something, but again, I can't see how an attacker could use that to their advantage. What I am more concerned about is background apps using activities to confuse the user as to what is the foreground app. (and given the app change animations I think this may be unlikely anyways)

Comment 6

[Jason Smith [:jsmith]]

Is this a security requirement for v1?

Comment 7

[Paul Theriault [:pauljt] (no longer reading bugmail)]

I think it should be, but decision isnt really up to me. This is a broader problem I think for Gaia since there are few ways that background apps can interfere with the foreground app. My understanding from previous bug discussion was that it was a requirement that an app that is not foreground cant display anything to the screen. This is the simplest from  a security perspective (i know the implementation isnt simple). Otherwise, if we allow it, we need to consider all of the edge cases where this behavior might be used for an attack and I don't think we will be able to do that comprehensively. I am trying to do some testing to figure out how extensive this problem is and how much work is needed to solve it.

Comment 8

[Jason Smith [:jsmith]]

I'll nom it just in case to get on this triage radar then.

Comment 9

[Alex Keybl [:akeybl]]

Milestoning for C2 (deadline of 12/10), as this meets the criteria of "known P2 bugs found before or during C1".

Comment 10

[Mounir Lamouri (:mounir)]

(In reply to Paul Theriault [:pauljt] from comment #5)
> But I'll it up to you Mounir - TBH I dont completely understand how
> nsEventStateManager::IsHandlingUserInput() works, but if you are comfortable
> that this control will prevent background apps spawning activities I think
> that will be ok.

IsHandlingUserInput() will return true if the call is made in an event input handler, roughly. It seems hard to abuse that in a background app.

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Mounir Lamouri (:mounir) from comment #10)
> (In reply to Paul Theriault [:pauljt] from comment #5)
> > But I'll it up to you Mounir - TBH I dont completely understand how
> > nsEventStateManager::IsHandlingUserInput() works, but if you are comfortable
> > that this control will prevent background apps spawning activities I think
> > that will be ok.
> 
> IsHandlingUserInput() will return true if the call is made in an event input
> handler, roughly. It seems hard to abuse that in a background app.


Exact. Because of this I would say this bug is now blocking-basecamp-. Renoming.

Comment 12

[Mounir Lamouri (:mounir)]

I would keep it as blocking because it requires bug 794407 to be fixed. As long as bug 794407 isn't fixed, background apps can launch activities. Given that the blocker of bug 794407 isn't really moving, I wouldn't be against some pressure to make bug 794407 resolved.

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Mounir Lamouri (:mounir) from comment #12)
> I would keep it as blocking because it requires bug 794407 to be fixed. As
> long as bug 794407 isn't fixed, background apps can launch activities. Given
> that the blocker of bug 794407 isn't really moving, I wouldn't be against
> some pressure to make bug 794407 resolved.

The underlying Gaia issue is waiting for review. I expect it to land this week.

Comment 14

[Doug Turner (:dougt)]

mounir, is this something you can work on?

Comment 15

[Mounir Lamouri (:mounir)]

AFAIK, there is nothing to do here. Bug 794407 makes Web Activities not launchable from outside an event handler so I do not see how a background app can start a Web Activity.

Feel free to reopen if that statement is wrong.