465806 - document.open should use the principal of the document whose URI it uses

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

Right now we get the principal off the stack.  Instead, we should use the document principal of the document whose URI we'll use.  Since we enforce same-origin on principals, there can't be a privilege-escalation issue here.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Oh, and we already use the securityInfo of the document in question, so this will just make everything consistent.

Comment 2

[Ben Newman (:bnewman) (:benjamn)]

I'd be happy to take this, bz.

Comment 3

[Ben Newman (:bnewman) (:benjamn)]

Attachment: patch without tests

Not sure how to set up a mochitest for this.  Other security restrictions seem to intervene before this test becomes relevant.  For example, I tried open()ing the contentWindow.document of a different-origin iframe, but I couldn't access the .document at all because of the origin difference.  Ideas?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 349082 [details] [diff] [review]
patch without tests

callerDoc could be null here.

For what it's worth I do have a patch (written when I posted the bug); I just still need to run it through mochitest...  I'll do that once I don't need my computer for an hour or so.  ;)

As for testing, you could use expanded privileges to access cross-origin (do it from chrome, or grab UniversalXPConnect, e.g.)

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: What I had

This does pass tests.  Still no additional tests, though.  Ben, if you want to write some up (or for that matter drive this patch in), please feel free.

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 349123 [details] [diff] [review]
What I had

+  nsCOMPtr<nsISupports> securityInfo = callerDoc->GetSecurityInfo();;

Remove the extra ';'.

Looks good to me. r+sr=jst

Comment 7

[Boris Zbarsky [:bzbarsky]]

So that patch on its own regresses bug 428653, which depends on the fact that a safe object wrapper will give the write() call a subject principal that corresponds to the untrusted page...  I'll see what I can do about fixing that.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Oh, and the reason that bug regresses is that we're no longer using the subject principal, of course.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: With "view mathml source" updates to not use document.write

This also makes the viewsource test effectively test this bug.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/76ca5cd4892f.

The dev-doc-needed is because this makes document.write into an untrusted document from chrome not work in Gecko 1.9.2, even if trying to use the wrappedJSObject trick.

Comment 11

[Eric Shepherd [:sheppy]]

Docs updated: https://developer.mozilla.org/En/DOM/Document.open

Comment 12

[peina]

Is that means calling document.open()/document.write() from C++ code no longer works in 1.9? 
That's really bad news for embedders because we already used these method in our application.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

C++ could push the right context to stack? Some context from which a document can be get. Maybe the context from the document, which .open()/.write() is being called.

Comment 14

[Boris Zbarsky [:bzbarsky]]

The only C++ callers which are prevented with the new code are the ones who were potentially introducing a security hole with their call anyway.  But yes, such callers are no longer allowed.

Comment 15

[peina]

Hi Boris and Olli, thanks for your reply.

Currently we do something like this:
1.Get nsIWebBrowser object of current browser window.
2.Get a nsIDOMWindow object from nsIWebBrowser's contentDOMWindow attribute.
3.Get a nsIDOMHTMLDocument object from nsIDOMWindow object's 'document' attribute.
4.Call nsIDOMHTMLDocument object's open/write/close method to write something into the current browser window.

It works on 1.8 but fails on 1.9. What else should I do to make it work without introducing security hole?

Comment 16

[Boris Zbarsky [:bzbarsky]]

Do you have to do it via write()?  Is using a data: URI or loadStream on the docshell not an option?

If so, you probably want to get the window's nsIScriptContext, get its JSContext, and push it on xpconnect's nsIJSContextStack before calling write(), then pop it off after.

Comment 17

[peina]

Thanks. I will try it. We have ever tried loadStream , it seems to be a asynchronous operation but we need a synchronous one.

Comment 18

[Boris Zbarsky [:bzbarsky]]

If you need a syncronous one, you could consider just setting the innerHTML of the <html> node in the document...

Comment 19

[peina]

setting innerHTML of the <html> node doesn't work for about:blank...

Comment 20

[Boris Zbarsky [:bzbarsky]]

That's... odd.  Works fine over here.