More consistent handling of principals for loads across docshell type

boundaries.  Bug 388121, r+sr=jst

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<nsISupports> container = doc->GetContainer();
-  nsCOMPtr<nsIDocShellTreeItem> parentItem = do_QueryInterface(container);
-  nsCOMPtr<nsIDocShellTreeItem> 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<nsIURI> referrer;
   rv = principal->GetURI(getter_AddRefs(referrer));

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<nsIScriptSecurityManager> secMan =
+            do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv);
+        NS_ENSURE_SUCCESS(rv, rv);
+
+        if (owner && mItemType != typeChrome) {
+            nsCOMPtr<nsIPrincipal> 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<nsIScriptSecurityManager> 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;
             }
         }
 

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<nsPIDOMWindow> newWindow = do_QueryInterface(*_retval);
 #ifdef DEBUG
     nsCOMPtr<nsPIDOMWindow> newDebugWindow = do_GetInterface(newDocShell);