635776 - When Javascript stack space is limited, window.open() returns a ChromeWindow

Status: RESOLVED FIXED

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

Description

[Zach Hoffman [:zrhoffman]]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11

If enough stack space is exhausted when window.open() is called, the new window's initialization fails in fundamental ways (it has no visual components besides the bare window and is not loaded as a tab), though the function manages to return a ChromeWindow reference.  The opener window then has full read/write access to the ChromeWindow's document object, regardless of either's page location.



Reproducible: Always

Steps to Reproduce:
1. Open Firefox beta 11 (or possibly other beta versions)
2. Run either the testcase (attached), or the following code from the address bar:
(function g(){try{eval("g()")}catch(c){if(c!=2)throw c[0]?1:c+1;d=open('data:text/html,A page')}})()



After running the testcase, the error console contains new exceptions after subsequent pageloads, sometimes even after restarting the browser.

Comment 1

[Zach Hoffman [:zrhoffman]]

Attachment: Opens a ChromeWindow, injects code

Comment 2

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

I can't reproduce on 64bit Linux, but "Open a ChromeWindow" does cause a crash.

Comment 3

[Johnny Stenback (:jst)]

Blake, care to have a look? We should never return the outer chrome window from window.open()...

Comment 4

[Daniel Veditz [:dveditz]]

Tried injecting "try{alert(Components.stack)} catch(e){alert(e)}" and got a security exception, proving it's not Chrome-privileged despite being a "ChromeWindow".

Comment 6

[Lucas Adamski [:ladamski]]

When I try to inject "alert(Components.stack)" instead I get a security exception in the error console.  This implies the window does not actually have chrome privileges.  Still XSS though.

Comment 7

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

Is this a regression?
There were some changes to window handling for FF4.

Comment 8

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

Ok, at least on 64bit linux / debug build this is a regression from
1.9.2.

Comment 9

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

Both 1.9.2 and 1.9.1 give security error when trying to inject code.

Comment 10

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

Attachment: Try to get docshell tree item explicitly only if nsIWebBrowserChrome::CHROME_OPENAS_CHROME

Comment 11

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

I haven't run tests yet.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Hmm.  So why does JS stack space matter?  We fail to get newWindow because of OOM or something?

Comment 13

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

Well all sort of things start to fail if we're running out of
JS stack space when creating a new window. The js in the new window
just can't run properly.
But I can still debug some more to find the exact place which prevents
the created window to have primary content shell.

Comment 14

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

Actually, might be better to add a check to nsXULWindow::CreateNewContentWindow
that the primary content shell can be get from the result.

Comment 15

[Boris Zbarsky [:bzbarsky]]

That seems like a good sanity-check, yes.  I'm not that interested in where the window creation fails, as long as it fails.  ;)

Comment 16

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

Apparently comment 14 isn't enough to fix this bug. Debugging...

Comment 17

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

Attachment: patch

Idiot me, I was checking mPrimaryContentShell of wrong XULWindow :)
This should work.

Posted to tryserver.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 516280 [details] [diff] [review]
patch

r=me

Comment 19

[Johnny Stenback (:jst)]

Checked in:

http://hg.mozilla.org/mozilla-central/rev/e79b2b60d394