206026 - bookmarklet can't document.write to about:blank twice

Status: RESOLVED FIXED

(Core :: Security, defect, P1)

Description

[Jesse Ruderman]

Steps:
1. Type this into the location bar or use it as a bookmarklet:
javascript:x = window.open(); x.document.write(1); x.document.write(2);

Result:
* A new window opens.  (Good so far.)
* Only "1" appears in the new window.
* chrome://browser/content/browser.xul appears in the location bar in the new
window.
* Nothing appears on the JavaScript console (bug 206025; if I use try..catch, I
get something like "Permission denied to call HTMLDocument.write").

Expected:
* A new window opens.
* "12" appears in the new window.

This bug only affects javascript: URLs loaded from the location bar or from
bookmarks.

If I replace document.write(1) with document.open(), I get a message (not an
error) on the JavaScript console:
Security Error: Content at [...] may not load data from about:blank.

This regression was introduced between Firebird 05/11:17 and Firebird 05/13:15.
 The fix for bug 202994 was checked in during that time.  This regression is
also present in Seamonkey 05/16:08.

This bug breaks many bookmarklets, including "view variables", "view scripts",
and "make numbered list [of links]".

Comment 1

[Jesse Ruderman]

From an e-mail I recieved from a bookmarklet user:

If you open an affected bookmarklet from the sidebar panel instead of from the
personal toolbar, the URL that appears in the address bar of the new window is 
chrome://browser/content/bookmarks/bookmarksPanel.xul instead of
chrome://browser/content/browser.xul.

Comment 2

[Johnny Stenback (:jst)]

I just *love* javascript: URL's! So this is a regression from bug 202994. The
changing of the context on which we evaluate javascript: URL's caused this, so
we'll need to revert that change. Fortunately that can be done w/o opening up
the security hole in bug 202994. Patch coming up.

Comment 3

[Johnny Stenback (:jst)]

Attachment: Revert the evaluation context change.

This gets us back to where we were before the fix for bug 202994 landed wrt
what context javascript: URL's run on.

Comment 4

[Brendan Eich [:brendan]]

Doesn't this patch regress bug 202994?

I believe I have a dup of this in bug 123696.

In the old MozillaClassic codebase, the origin of a page generated by a
javascript: URL with non-void result, or via a document.write (or via a write
from a jsurl!) was the generator's origin.  The wysiwyg: URLs used to cache
generated pages could be unraveled to find the generator's origin, which meant
you would never see a security alert or suppression event in cases such as this
bug and bug 123696.

jst, can't we keep the origin invariant across any number of (re-)generations by
any combination of javascript:- and document.write-generated pages?

/be

Comment 5

[Johnny Stenback (:jst)]

Brendan, this does *not* regress bug 202994.

Ideally, we wouldn't need to do this, but unfortunately we must since this
causes javascript: URL's that are activated by chrome (i.e. by clicking on a
bookmarklet) to run on the chrome context (though w/o chrome priveleges) when
the expectation is that they run on the context of the current page.

Ideally we wouldn't need to revert this change and in stead figure out who
relies on javascript: URL's being executed on the currently loaded page's
context and fix that code, but I don't think that's something we want to do for
1.4 final.

I say we take this for 1.4 final since it gets us back to where we were w/o
reexposing the security problem that was fixed by the patch that regressed this,
and it does fix this problem.

Comment 6

[Brendan Eich [:brendan]]

I'm leary of reopening some security bug with this path.

Does it fix bug 123696?  I'll update my build, but if you can test that bug's
testcase quickly, please report results here.

/be

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 123745 [details] [diff] [review]
Revert the evaluation context change.

Ok, jst and I have fried our brains figuring out why this is safe, and stuff.  

/be

Comment 8

[Johnny Stenback (:jst)]

The reason this does *not* regress bug 202994 is that the reason for that bug
was not that we were evaluating javascript: URL's on the wrong context (that was
just something we noticed while fixing that bug), the reason for the exploit in
the bug was the bug in the caps code that assumed that all capabilities are
enabled if there's no principal to be found on the JS function stack.

Independent of the context we execute javascript: URL's on, we always do give
the JS in the URL the correct principals (in nsJSProtocolHandler.cpp, and the
principals can never be javascript:, btw), so AFAIK, there are no exploits
exposed either way.

Comment 9

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 123745 [details] [diff] [review]
Revert the evaluation context change.

I'm convinced. r=mstoltz.

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 123745 [details] [diff] [review]
Revert the evaluation context change.

Requesting approval for 1.4 final. This is a pretty serious regression that was
introduced just last week.

Comment 11

[Johnny Stenback (:jst)]

Fix checked in.

Comment 12

[Johnny Stenback (:jst)]

*** Bug 206025 has been marked as a duplicate of this bug. ***