1148032 - BroadcastChannel API bypasses private browsing mode

Status: RESOLVED FIXED

(Firefox :: Private Browsing, 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. Open Firefox Nightly and open following two pages on both normal browsing mode and private browsing mode.
    Sender: http://csrf.jp/bc/sender.html
    Receiver: http://csrf.jp/bc/receiver.html
2. Push 'Send Message' button of Sender on normal mode. Then, the message is sent to Receiver on private mode.
3. On the contrary, push 'Send Message' button of Sender on private mode. Then, the message is sent to Receiver on normal mode.



Actual results:

BroadcastChannel message is sent with bypassing private browsing mode.



Expected results:

The message sent from Browser app should not be delivered beyond private mode context.
Note that this bug was origiinally reported in [Issue 2] in Bug 1147778.

Comment 1

[(no longer active)]

I don't think this bug is security sensitive, but we should definitely fix it, by ensuring that broadcasts from private windows are sandboxed from non-private ones, and vice versa.

Andrea, do you have cycles for this?

Comment 2

[Andrea Marchesini [:baku]]

Attachment: bc.patch

Comment 3

[(no longer active)]

Comment on attachment 8585602 [details] [diff] [review]
bc.patch

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

This also needs tests.

::: dom/broadcastchannel/BroadcastChannelChild.h
@@ +36,5 @@
>      return mActorDestroyed;
>    }
>  
>  private:
> +  BroadcastChannelChild(const nsAString& aOrigin);

This looks mostly good, but I don't understand this part.  Why are you dropping the channel argument here?

Comment 4

[Andrea Marchesini [:baku]]

> > +  BroadcastChannelChild(const nsAString& aOrigin);
> 
> This looks mostly good, but I don't understand this part.  Why are you
> dropping the channel argument here?

because this object doesn't use it. So why have it as argument?

Comment 5

[Andrea Marchesini [:baku]]

Comment on attachment 8585602 [details] [diff] [review]
bc.patch

I promise I'll write a mochitest. I tried but I'm having problems to have 2 windows opened at the same time and keep 2 broadcastchannel objects alive at the same time.
The test will come in a separate patch.

Comment 6

[Andrea Marchesini [:baku]]

Attachment: bc.patch

test included

Comment 7

[(no longer active)]

Comment on attachment 8586171 [details] [diff] [review]
bc.patch

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

::: dom/broadcastchannel/tests/test_broadcastchannel_private_browsing.html
@@ +52,5 @@
> +
> +function check(msg, private) {
> +  is(msg, private ? "private" : "public", "Correct context!");
> +  gCounter++;
> +info(gCounter);

Stray debugging line?

Comment 8

[Andrea Marchesini [:baku]]

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

Comment 10

[Andrea Marchesini [:baku]]

I fixed the oth failure and here the push:

https://hg.mozilla.org/integration/mozilla-inbound/rev/976f64497fbc

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for static analysis bustage: https://hg.mozilla.org/integration/mozilla-inbound/rev/75f95cbc0530

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

Comment 12

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6b886fbb3e48

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

And out again for mochitest-chrome orange on b2g emulators: https://hg.mozilla.org/integration/mozilla-inbound/rev/522cf568a364

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

Comment 14

[Andrea Marchesini [:baku]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=249ddd79d9e2

https://hg.mozilla.org/integration/mozilla-inbound/rev/128b6d47fe8a

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/128b6d47fe8a

Do we want this on Aurora/Beta?

Comment 16

[Andrea Marchesini [:baku]]

Comment on attachment 8586171 [details] [diff] [review]
bc.patch

Approval Request Comment
[Feature/regressing bug #]: 966439
[User impact if declined]: BroadcastChannel messages bypass the private browsing sandbox.
[Describe test coverage new/current, TreeHerder]: treeharder tests are included.
[Risks and why]: the patch is quite simple.
[String/UUID change made/needed]: none

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8586171 [details] [diff] [review]
bc.patch

should be in 38b2

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9ededdba9f92

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/125623fcc804

Comment 20

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

Minusing this for bounty because it is a low rated security issue.

Comment 21

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

If bug 966439 is the regressor, this is Firefox 38 and higher only, isn't it?

Comment 22

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

As in, we never shipped this publicly in Firefox 37...

Comment 23

[Muneaki Nishimura]

(In reply to Al Billings [:abillings] from comment #21)
> If bug 966439 is the regressor, this is Firefox 38 and higher only, isn't it?

Yes, correct.
As far as I investigated, The BroadcastChannel API is available from Firefox 38+.
It cannot be activated in Firefox 37 and there seems not to be any flags for enabling it in about:config.

Comment 24

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

Thanks.