Bug 774633 - Remove "is chrome window" condition for inner window reuse. r=jst

WouldReuseInnerWindow also returns true if the new window is same-origin with
the old one about:blank document.

This condition exists in order to handle some sloppiness with respect to the
principals on initial about:blank documents. Chrome callers sometimes parent
chrome windows (with XUL document) to content windows. But this parenting causes
us to push the cx of the content window during window creation, meaning that
the subsequent load of chrome://foo.xul blows away the old inner window and any
expandos on it. We can handle this case more precisely by skipping the cx push
for type="chrome" windows.

Furthermore, this was also necessary to prevent the inner window from being
blown away in the call to SetOpenerScriptPrincipal once nsWindowWatcher gets
the window back from the window creator (and after it's already told consumers
about the window via "domwindowcreated"). But we fixed this nastiness in the
previous patches.

So we can remove this case. By doing so, we can prevent inner windows from ever
changing origins, which is very important for compartment security invariants.
This commit is contained in:
Bobby Holley 2012-08-23 16:44:53 -07:00
parent bf36b7f605
commit 38274fd965
2 changed files with 17 additions and 19 deletions

View File

@ -1464,8 +1464,6 @@ nsGlobalWindow::WouldReuseInnerWindow(nsIDocument *aNewDocument)
// We reuse the inner window when:
// a. We are currently at our original document.
// b. At least one of the following conditions are true:
// -- We are not currently a content window (i.e., we're currently a chrome
// window).
// -- The new document is the same as the old document. This means that we're
// getting called from document.open().
// -- The new document has the same origin as what we have loaded right now.
@ -1480,9 +1478,10 @@ nsGlobalWindow::WouldReuseInnerWindow(nsIDocument *aNewDocument)
NS_ASSERTION(NS_IsAboutBlank(mDoc->GetDocumentURI()),
"How'd this happen?");
// Great, we're the original document, check for one of the other
// conditions.
if (mDoc == aNewDocument) {
return true;
}
@ -1495,17 +1494,6 @@ nsGlobalWindow::WouldReuseInnerWindow(nsIDocument *aNewDocument)
return true;
}
nsCOMPtr<nsIDocShellTreeItem> treeItem(do_QueryInterface(mDocShell));
if (treeItem) {
int32_t itemType = nsIDocShellTreeItem::typeContent;
treeItem->GetItemType(&itemType);
// If we're a chrome window, then we want to reuse the inner window.
return itemType == nsIDocShellTreeItem::typeChrome;
}
// No treeItem: don't reuse the current inner window.
return false;
}

View File

@ -586,11 +586,21 @@ nsWindowWatcher::OpenWindowInternal(nsIDOMWindow *aParent,
JSContext *cx = GetJSContextFromWindow(aParent);
if (isCallerChrome && !hasChromeParent && cx) {
// open() is called from chrome on a non-chrome window, push
// the context of the callee onto the context stack to
// prevent the caller's priveleges from leaking into code
// that runs while opening the new window.
bool windowTypeIsChrome = chromeFlags & nsIWebBrowserChrome::CHROME_OPENAS_CHROME;
if (isCallerChrome && !hasChromeParent && !windowTypeIsChrome && cx) {
// open() is called from chrome on a non-chrome window, push the context of the
// callee onto the context stack to prevent the caller's priveleges from leaking
// into code that runs while opening the new window.
//
// The reasoning for this is in bug 289204. Basically, chrome sometimes does
// someContentWindow.open(untrustedURL), and wants to be insulated from nasty
// javascript: URLs and such. But there are also cases where we create a
// window parented to a content window (such as a download dialog), usually
// directly with nsIWindowWatcher. In those cases, we want the principal of
// the initial about:blank document to be system, so that the subsequent XUL
// load can reuse the inner window and avoid blowing away expandos. As such,
// we decide whether to load with the principal of the caller or of the parent
// based on whether the docshell type is chrome or content.
callerContextGuard.Push(cx);
}