Last Comment Bug 733035 - need a way to postMessage from sandboxes (without source window object)
: need a way to postMessage from sandboxes (without source window object)
Status: RESOLVED FIXED
: dev-doc-needed
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Gabor Krizsanits [:krizsa :gabor]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 729994 762530
  Show dependency treegraph
 
Reported: 2012-03-05 10:26 PST by Gabor Krizsanits [:krizsa :gabor]
Modified: 2013-04-04 13:53 PDT (History)
6 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
first draft with test (9.83 KB, patch)
2012-03-08 06:20 PST, Gabor Krizsanits [:krizsa :gabor]
khuey: review+
Details | Diff | Splinter Review
ready to go (9.57 KB, patch)
2012-04-04 09:00 PDT, Gabor Krizsanits [:krizsa :gabor]
gkrizsanits: review+
Details | Diff | Splinter Review
ready to go (9.74 KB, patch)
2012-04-04 09:10 PDT, Gabor Krizsanits [:krizsa :gabor]
gkrizsanits: review+
Details | Diff | Splinter Review

Description Gabor Krizsanits [:krizsa :gabor] 2012-03-05 10:26:26 PST
From a sandbox currently postMessage to a window object does not work (silent failure). The reason is that the function expects that the global is a window that can be used as a source window. For sandboxes that is not the case, but probably the principal of the sandbox can be used for the security checks.
Comment 1 Gabor Krizsanits [:krizsa :gabor] 2012-03-08 06:20:27 PST
Created attachment 604030 [details] [diff] [review]
first draft with test

So I'm not sure that this is safe enough, and I don't know who is the best person to ask for a review on this. What do you think khuey? So there are two cases, either the sandbox have system principal, then I'm not really affraid of security implications. And when it has a URI based principal, then there is a check before cloning the data, but I'm not sure if it's enough. I would also love to validate somehow if the global or it's principal belongs to a sandbox, and the callerInnerWin isn't null for some other reasons.
Comment 2 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-04-03 08:49:59 PDT
Comment on attachment 604030 [details] [diff] [review]
first draft with test

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

::: dom/base/nsGlobalWindow.cpp
@@ +6347,5 @@
>      // if the principal has a URI, use that to generate the origin
>      nsContentUtils::GetUTFOrigin(callerPrin, origin);
>    }
>    else {
> +    if (callerInnerWin) {

just use

else if (callerInnerWin) {
...
}
else {
}

No need for another level of indenting.

@@ +6349,5 @@
>    }
>    else {
> +    if (callerInnerWin) {
> +      // otherwise use the URI of the document to generate origin
> +      nsCOMPtr<nsIDocument> doc = do_QueryInterface(callerInnerWin->mDocument);

callerInnerWin->GetExtantDocument()
Comment 3 Gabor Krizsanits [:krizsa :gabor] 2012-04-04 09:00:50 PDT
Created attachment 612215 [details] [diff] [review]
ready to go

https://tbpl.mozilla.org/?tree=Try&rev=7b684de69a0d
Comment 4 Gabor Krizsanits [:krizsa :gabor] 2012-04-04 09:10:43 PDT
Created attachment 612218 [details] [diff] [review]
ready to go

The previous version of the patch did not contain the Mercurial Queue headers, this one does.
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-04-05 15:35:09 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f03b8e2cc5a
Comment 6 Matt Brubeck (:mbrubeck) 2012-04-06 11:39:00 PDT
https://hg.mozilla.org/mozilla-central/rev/8f03b8e2cc5a

Note You need to log in before you can comment on or make changes to this bug.