1148033 - BroadcastChannel API bypasses Browser API sandbox on B2G

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Muneaki Nishimura]

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

Steps to reproduce:

1. Install attached packaged app 'victim.zip' and 'victim2.zip' on Firefox OS simulator 3.0.
2. Start Browser app and open 'http://csrf.jp/bc/receiver.html'.
3. Start victim app. The app loads message receiver page from http://csrf.jp on it's iframe.
4. Start the victim2 app. The app loads message sender page from http://csrf.jp on it's iframe[mozbrowser].
5. Push 'Send Message' button in the sender page of victim2.app, and then, BroadcastChannel message is sent.
6. Open the victim app again. Then, you can see the received message from victim2 app on the alert popup.
7. Open the Browser app agaon. Then, you can see the received message from victim2 app on the alert popup.


Actual results:

This message sent from <iframe mozbrowser> is delivered to other apps.



Expected results:

The message sent from Browser app should not be delivered to other apps outside of the Browser API sandbox in an app.
Note that this bug was origiinally reported in [Issue 3] in Bug 1147778.

Comment 1

[Muneaki Nishimura]

Attachment: victim.zip

Comment 2

[Muneaki Nishimura]

Attachment: victim2.zip

Comment 3

[Daniel Veditz [:dveditz]]

Not clear to me that broadcasts to a web page aren't supposed to apply to web pages loaded by different apps. It's still web pages in browsers. I guess this could be used to synchronize app cookie jars.

Comment 4

[Andrea Marchesini [:baku]]

Attachment: bc2.patch

Comment 6

[(no longer active)]

Comment on attachment 8586836 [details] [diff] [review]
bc2.patch

Review of attachment 8586836 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/broadcastchannel/tests/test_broadcastchannel_mozbrowser.html
@@ +83,5 @@
> +       { "type": "webapps-manage", "allow": 1, "context": document }], nextStep);
> +  },
> +
> +  function() {
> +    SpecialPowers.setBoolPref("network.disable.ipc.security", true);

Why not use pushPrefEnv for this too?

Comment 7

[Andrea Marchesini [:baku]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ed0aab6f82e

Comment 8

[Andrea Marchesini [:baku]]

dveditz can I land this patch immediately? I don't see a valid reason to have it sec-critical actually.

Comment 9

[Daniel Veditz [:dveditz]]

Yes, we can land this now.

Comment 10

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43c2bea152ce

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Backed out for test failures/crashes.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d43cf0816a1

https://treeherder.mozilla.org/logviewer.html#?job_id=8412496&repo=mozilla-inbound

Comment 12

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/29bf67773de8

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/29bf67773de8

Comment 14

[Al Billings [:abillings - ex-MoCo]]

Minusing for bounty because it is a sec-low rated issue.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

It looks like the patch here makes us use the manifestURL *rather* *than* the origin as part of the key when broadcasting messages. The other part of the key is of course the broadcast-channel-name.

Is this correct?

If so, doesn't that mean that all webpages, from all domains, in an app that use the same broadcast-channel-name? I.e. facebook.com and google.com would share broadcast channels?

If that's what we do, then that's not desired behavior.

What we should do is we should use the appid+browserflag+origin as key, plus the broadcast-channel-name. An easy way to do that is to use the nsIPrincipal.jarPrefix+origin as key.

Comment 16

[Andrea Marchesini [:baku]]

Jonas, I'm going to file a new bug for this. Thanks for find this issue!

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Only a sec-low, but something we might want to consider for v2.2 still?

Comment 18

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

Yes that sounds like a good idea, and assume that means including 1161507 too. (I don't have +'ing rights...)

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Andrea, please request b2g37 approval on this and bug 1161507 :)

Comment 20

[Andrea Marchesini [:baku]]

Comment on attachment 8586836 [details] [diff] [review]
bc2.patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 966439 / BroadcastChannel API
User impact if declined: in B2G, broadcastChannel can send messages to the wrong origin.
Testing completed: yes, a mochitest is included
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8586836 [details] [diff] [review]
bc2.patch

Whoops, BroadcastChannel landed on Gecko 38, so v2.2 is unaffected. Sorry for wasting your time, Andrea :(