BroadcastChannel API bypasses private browsing mode

RESOLVED FIXED in Firefox 38

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: sdna.muneaki.nishimura, Assigned: baku)

Tracking

({privacy, sec-low})

37 Branch
Firefox 40
x86
Windows 8
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty -
in-testsuite +

Firefox Tracking Flags

(firefox37 unaffected, firefox38 fixed, firefox39 fixed, firefox40 fixed)

Details

(Whiteboard: [adv-main38-])

Attachments

(1 attachment, 1 obsolete attachment)

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.
Flags: sec-bounty?
Component: Untriaged → Private Browsing
Flags: needinfo?(ehsan)
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?
Flags: needinfo?(ehsan) → needinfo?(amarchesini)
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Posted patch bc.patch (obsolete) — Splinter Review
Attachment #8585602 - Flags: review?(ehsan)
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?
Attachment #8585602 - Flags: review?(ehsan) → feedback+
> > +  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?
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Attachment #8585602 - Flags: review?(ehsan)
Posted patch bc.patchSplinter Review
test included
Attachment #8585602 - Attachment is obsolete: true
Attachment #8585602 - Flags: review?(ehsan)
Attachment #8586171 - Flags: review?(ehsan)
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?
Attachment #8586171 - Flags: review?(ehsan) → review+
Duplicate of this bug: 1147778
I fixed the oth failure and here the push:

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

Do we want this on Aurora/Beta?
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(amarchesini)
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
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
Flags: needinfo?(amarchesini)
Attachment #8586171 - Flags: approval-mozilla-beta?
Attachment #8586171 - Flags: approval-mozilla-aurora?
Comment on attachment 8586171 [details] [diff] [review]
bc.patch

should be in 38b2
Attachment #8586171 - Flags: approval-mozilla-beta?
Attachment #8586171 - Flags: approval-mozilla-beta+
Attachment #8586171 - Flags: approval-mozilla-aurora?
Attachment #8586171 - Flags: approval-mozilla-aurora+
Minusing this for bounty because it is a low rated security issue.
Group: core-security
Flags: sec-bounty? → sec-bounty-
Whiteboard: [adv-main38+]
If bug 966439 is the regressor, this is Firefox 38 and higher only, isn't it?
Flags: needinfo?(amarchesini)
Flags: needinfo?(sdna.muneaki.nishimura)
As in, we never shipped this publicly in Firefox 37...
(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.
Flags: needinfo?(sdna.muneaki.nishimura)
Thanks.
Whiteboard: [adv-main38+] → [adv-main38-]
Flags: needinfo?(amarchesini)
Depends on: 1276328
You need to log in before you can comment on or make changes to this bug.