From d59efa67632954bed242682f36838a582c94d06b Mon Sep 17 00:00:00 2001 From: "bzbarsky@mit.edu" Date: Tue, 17 Jul 2007 18:47:07 -0700 Subject: [PATCH] More consistent handling of principals for loads across docshell type boundaries. Bug 388121, r+sr=jst --- content/base/src/nsFrameLoader.cpp | 19 +------- docshell/base/nsDocShell.cpp | 48 ++++++++++++------- .../windowwatcher/src/nsWindowWatcher.cpp | 16 +++++-- 3 files changed, 45 insertions(+), 38 deletions(-) diff --git a/content/base/src/nsFrameLoader.cpp b/content/base/src/nsFrameLoader.cpp index 08fcb04ea69..e64cab4ea2e 100644 --- a/content/base/src/nsFrameLoader.cpp +++ b/content/base/src/nsFrameLoader.cpp @@ -187,23 +187,8 @@ nsFrameLoader::LoadURI(nsIURI* aURI) // We'll use our principal, not that of the document loaded inside us. This // is very important; needed to prevent XSS attacks on documents loaded in - // subframes! But only use our principal if our docshell's type is the same - // as the type of our ownerDocument's docshell. Note that we could try - // checking GetSameTypeParent() on mDocShell, but that might break if we ever - // support docshells loaded inside disconnected nodes... - nsCOMPtr container = doc->GetContainer(); - nsCOMPtr parentItem = do_QueryInterface(container); - nsCOMPtr ourItem = do_QueryInterface(mDocShell); - NS_ASSERTION(ourItem, "Must have item"); - if (parentItem) { - PRInt32 parentType; - rv = parentItem->GetItemType(&parentType); - PRInt32 ourType; - nsresult rv2 = ourItem->GetItemType(&ourType); - if (NS_SUCCEEDED(rv) && NS_SUCCEEDED(rv2) && ourType == parentType) { - loadInfo->SetOwner(principal); - } - } + // subframes! + loadInfo->SetOwner(principal); nsCOMPtr referrer; rv = principal->GetURI(getter_AddRefs(referrer)); diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index dfe0600a731..b3172fdcb33 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -819,30 +819,44 @@ nsDocShell::LoadURI(nsIURI * aURI, } // Perform the load... else { - // We need an owner (a referring principal). 3 possibilities: - // (1) If a principal was passed in, that's what we'll use. - // (2) If the caller has allowed inheriting from the current document, + // We need an owner (a referring principal). 4 possibilities: + // (1) If the system principal was passed in and we're a typeContent + // docshell, inherit the principal from the current document + // instead. + // (2) In all other cases when the principal passed in is not null, + // use that principal. + // (3) If the caller has allowed inheriting from the current document, // or if we're being called from system code (eg chrome JS or pure // C++) then inheritOwner should be true and InternalLoad will get // an owner from the current document. If none of these things are // true, then - // (3) we pass a null owner into the channel, and an owner will be - // created later from the URL. + // (4) we pass a null owner into the channel, and an owner will be + // created later from the channel's internal data. // - // NOTE: This all only works because the only thing the owner is used - // for in InternalLoad is data: and javascript: URIs. For other - // URIs this would all be dead wrong! + // NOTE: This all only works because the only thing the owner is used + // for in InternalLoad is data:, javascript:, and about:blank + // URIs. For other URIs this would all be dead wrong! + nsCOMPtr secMan = + do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv); + NS_ENSURE_SUCCESS(rv, rv); + + if (owner && mItemType != typeChrome) { + nsCOMPtr ownerPrincipal = do_QueryInterface(owner); + PRBool isSystem; + rv = secMan->IsSystemPrincipal(ownerPrincipal, &isSystem); + NS_ENSURE_SUCCESS(rv, rv); + + if (isSystem) { + owner = nsnull; + inheritOwner = PR_TRUE; + } + } if (!owner && !inheritOwner) { // See if there's system or chrome JS code running - nsCOMPtr secMan; - - secMan = do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv); - if (NS_SUCCEEDED(rv)) { - rv = secMan->SubjectPrincipalIsSystem(&inheritOwner); - if (NS_FAILED(rv)) { - // Set it back to false - inheritOwner = PR_FALSE; - } + rv = secMan->SubjectPrincipalIsSystem(&inheritOwner); + if (NS_FAILED(rv)) { + // Set it back to false + inheritOwner = PR_FALSE; } } diff --git a/embedding/components/windowwatcher/src/nsWindowWatcher.cpp b/embedding/components/windowwatcher/src/nsWindowWatcher.cpp index c78bb8d6f12..e9e61055945 100644 --- a/embedding/components/windowwatcher/src/nsWindowWatcher.cpp +++ b/embedding/components/windowwatcher/src/nsWindowWatcher.cpp @@ -656,10 +656,7 @@ nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow *aParent, // chrome is always allowed, so clear the flag if the opener is chrome if (popupConditions) { - PRBool isChrome = PR_FALSE; - if (sm) - sm->SubjectPrincipalIsSystem(&isChrome); - popupConditions = !isChrome; + popupConditions = !isCallerChrome; } if (popupConditions) @@ -823,6 +820,17 @@ nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow *aParent, } } + PRBool isSystem; + rv = sm->IsSystemPrincipal(newWindowPrincipal, &isSystem); + if (NS_FAILED(rv) || isSystem) { + // Don't pass this principal along to content windows + PRInt32 itemType; + rv = newDocShellItem->GetItemType(&itemType); + if (NS_FAILED(rv) || itemType != nsIDocShellTreeItem::typeChrome) { + newWindowPrincipal = nsnull; + } + } + nsCOMPtr newWindow = do_QueryInterface(*_retval); #ifdef DEBUG nsCOMPtr newDebugWindow = do_GetInterface(newDocShell);