824420 - Browser 'view' activity handler doesn't clear background correctly for data: or javascript: URIs

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Browser, defect, P2)

Description

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

THe browser handles the view activity when the type is URL. If the supplied URL parameter (ie the URL to be opened) is a data: or javascript: uri then the resulting content is overlayed over the page which called the mozActivity. This might lead to spoofing cases, but I can think of anything particularly compelling at the moment.
For example, the call might be:

new MozActivity({
      name : 'view',
      data : {
        type : 'url',
        url : "data:text/html;,<button onclick=alert(1)>test</button>"
      }
    });

This creates a button overlayed on the calling page.  Not really a high priority issue I think, but still a bug.

Comment 1

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

z

Comment 2

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

Attachment: Patch

Let's use the new regexp filter that will be introduced by bug 816730.

Comment 3

[David Flanagan [:djf]]

Comment on attachment 695996 [details] [diff] [review]
Patch

With this patch, invoking the system would look for a property named 'regexp' in the activity request.

I think the correct syntax will be something like:

"type": "url",
"url": { "required":true, "regexp":"/^https?:/" }

I don't have a m-i tree to test this against, though, so I'm not sure.

Also: is http and https enough or do we need to whitelist the app:// protocol here?  I don't know at all whether that is necessary.

Comment 4

[David Flanagan [:djf]]

See https://hg.mozilla.org/integration/mozilla-inbound/file/f9c5b23292ca/dom/activities/tests/unit/test_activityFilters.js for examples of the new filter syntax

Comment 5

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

(In reply to David Flanagan [:djf] from comment #3)
> Comment on attachment 695996 [details] [diff] [review]
> Patch
> 
> With this patch, invoking the system would look for a property named
> 'regexp' in the activity request.
> 
> I think the correct syntax will be something like:
> 
> "type": "url",
> "url": { "required":true, "regexp":"/^https?:/" }
> 
> I don't have a m-i tree to test this against, though, so I'm not sure.
> 

I'm rebuilding now.

> Also: is http and https enough or do we need to whitelist the app://
> protocol here?  I don't know at all whether that is necessary.

I don't see why someone wants to open something using the app:// protocol with the browser.

Comment 6

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

Attachment: Patch v2

Comment 7

[David Flanagan [:djf]]

Comment on attachment 696070 [details] [diff] [review]
Patch v2

This looks good to me, but as noted, I'm not able to test it myself.

If it works in your testing against m-i, then r+, but coordinate the landing in gaia with the landing of the gecko patch. I don't know how things will break with this new filter in a gecko that doesn't support the syntax.

If this filter does not work against m-i, then you should ask Andrea or Mounir, I think.

Comment 8

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

https://github.com/mozilla-b2g/gaia/commit/afbf700e73f87eeb002ff055a4710680266cb335