1260907 - nsGlobalWindow needs to handle user context id correctly.

Status: RESOLVED WONTFIX

(Core :: DOM: Security, defect)

Description

[Dave Huseby [:huseby]]

In nsGlobalWindow.cpp there is a call to createCodebasePrincipal here:

https://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#8035

It appears that this code is doing the right thing.  It is passing the origin attributes from the top level windows into the principal that is being created.  We need to analyze how the newly created principal is used.  Is it being use to isolate browser state in any way?  If it is, do we want to isolate on user context id?

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Dave Huseby [:huseby] from comment #0)
> Is it being use to isolate browser state in any way?  If it is, do we want
> to isolate on user context id?

Bug 1249224 already sets the userContextId which got from subject priciapl on the new window.
Is this what you want?

Comment 2

[Jonathan Hao (inactive) [:jhao]]

(In reply to Dave Huseby [:huseby] from comment #0)
> In nsGlobalWindow.cpp there is a call to createCodebasePrincipal here:
> 
> https://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.
> cpp#8035
> 
> It appears that this code is doing the right thing.  It is passing the
> origin attributes from the top level windows into the principal that is
> being created.  We need to analyze how the newly created principal is used. 
> Is it being use to isolate browser state in any way?  If it is, do we want
> to isolate on user context id?

The newly created principal is used to create a PostMessageEvent[1]. It becomes that event's member mProvidedPrincipal and is only used once. This code[2] checks if mProvidedPrincipal matches the target window's principal. Since as Yoshi said, the userContextIds are set in subject principal, windows with different userContextId won't be able to communicate to each other by postMessage. I think that's exactly what we want, because if windows with different userContextIds can communicate by postMessage, the user's privacy will be compromised.

I'm thinking about closing this bug, but I'm not sure if we need a test to ensure the correct behavior.

Comment 3

[Jonathan Hao (inactive) [:jhao]]

(In reply to Jonathan Hao [:jhao] from comment #2)
> The newly created principal is used to create a PostMessageEvent[1]. It
> becomes that event's member mProvidedPrincipal and is only used once. This
> code[2] checks if mProvidedPrincipal matches the target window's principal.
> Since as Yoshi said, the userContextIds are set in subject principal,
> windows with different userContextId won't be able to communicate to each
> other by postMessage. I think that's exactly what we want, because if
> windows with different userContextIds can communicate by postMessage, the
> user's privacy will be compromised.
> 
> I'm thinking about closing this bug, but I'm not sure if we need a test to
> ensure the correct behavior.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp?from=nsGlobalWindow.cpp#8076
[2] https://dxr.mozilla.org/mozilla-central/source/dom/base/PostMessageEvent.cpp#107

Comment 4

[Jonathan Hao (inactive) [:jhao]]

I talked to baku. He said that you can only get another window's reference by either window.open() or if it's an iframe inside. In either case, the other window will be in the same user context, so there should never be a case when we can do window-to-window postMessage to another window in a different user context. So, there's no need to write a test for it.

I did write a test and almost finished but later realized I was testing BroadcastChannel's postMessage. So, I opened bug 1268803 and intend to put my patch there.

Comment 5

[Ethan Tseng [:ethan]]

(In reply to Jonathan Hao [:jhao] from comment #4)
> I did write a test and almost finished but later realized I was testing
> BroadcastChannel's postMessage. So, I opened bug 1268803 and intend to put
> my patch there.

Thanks for clarifying this. Good job!