Closed Bug 1161507 Opened 5 years ago Closed 5 years ago

BroadcastChannel should use origin+appId+IsInBrowserElement as key in b2g

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed
b2g-v2.2 --- ?
b2g-master --- fixed

People

(Reporter: baku, Assigned: baku)

References

Details

Attachments

(1 file)

Attached patch bc.patchSplinter Review
Jonas, do you mind to take a look at this patch?
Attachment #8601441 - Flags: review?(jonas)
Depends on: 1148033
Comment on attachment 8601441 [details] [diff] [review]
bc.patch

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

::: dom/broadcastchannel/BroadcastChannelParent.cpp
@@ +32,5 @@
>  {
>    AssertIsOnBackgroundThread();
>    mService->RegisterActor(this);
> +
> +  if (aPrincipalInfo.type() ==PrincipalInfo::TContentPrincipalInfo) {

You should verify that the app-id received from the child process is an appid that's actually running in the child. And not an attempt of the child to try to snoop what's happening in other apps.

The best way to do that is to convert the PrincipalInfo to an nsIPrincipal and then calling AssertAppPrincipal:

http://mxr.mozilla.org/mozilla-central/source/dom/ipc/AppProcessChecker.h#101

Once you do that, you can also get the origin from the nsIPrincipal, and there's no need to pass the aOrigin separately. That way you'll also get additional security checks once we start enforcing that certain origins will only run in certain child processes and check that in the AssertAppPrincipal function.
We don't do this check in the BroadcastChannelParent but here:

http://mxr.mozilla.org/mozilla-central/source/ipc/glue/BackgroundParentImpl.cpp#309
https://hg.mozilla.org/mozilla-central/rev/e7f7dc49cf08
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Proposing that we backport this along with 1148033.
status-b2g-v2.2: --- → ?
Comment on attachment 8601441 [details] [diff] [review]
bc.patch

[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
Attachment #8601441 - Flags: approval-mozilla-b2g37?
Attachment #8601441 - Flags: approval-mozilla-b2g37?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.