Bug 1562292: Part 1e - Use BrowsingContext as target in IsSandboxedFrom(). r=nika
authorKris Maglione <maglione.k@gmail.com>
Thu, 01 Aug 2019 16:36:32 -0700
changeset 488065 3bffe760e2db5ff009f0d798cbb63b83a68de51b
parent 488064 ba987e21b5329d07c3606a65ee0cb76e7bc99a74
child 488066 40b61344f0da14f9700a75a8a1c67458e440b68a
push id36435
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:46:49 +0000
treeherdermozilla-central@0db07ff50ab5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1562292
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1562292: Part 1e - Use BrowsingContext as target in IsSandboxedFrom(). r=nika This check always has access to an in-process DocShell which is attempting to perform a load, but its target may often be cross-process, and have no in-process DocShell. This patch changes the target checks to use a BrowsingContext in a Fission-compatible way. Differential Revision: https://phabricator.services.mozilla.com/D40496
docshell/base/nsDocShell.cpp
docshell/base/nsIDocShell.idl
toolkit/components/windowwatcher/nsWindowWatcher.cpp
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -3002,24 +3002,24 @@ nsresult nsDocShell::DoFindItemWithName(
   if (window && !aSkipTabGroup) {
     RefPtr<mozilla::dom::TabGroup> tabGroup = window->TabGroup();
     tabGroup->FindItemWithName(aName, this, aOriginalRequestor, aResult);
   }
 
   return NS_OK;
 }
 
-bool nsDocShell::IsSandboxedFrom(nsIDocShell* aTargetDocShell) {
+bool nsDocShell::IsSandboxedFrom(BrowsingContext* aTargetBC) {
   // If no target then not sandboxed.
-  if (!aTargetDocShell) {
+  if (!aTargetBC) {
     return false;
   }
 
   // We cannot be sandboxed from ourselves.
-  if (aTargetDocShell == this) {
+  if (aTargetBC == mBrowsingContext) {
     return false;
   }
 
   // Default the sandbox flags to our flags, so that if we can't retrieve the
   // active document, we will still enforce our own.
   uint32_t sandboxFlags = mSandboxFlags;
   if (mContentViewer) {
     RefPtr<Document> doc = mContentViewer->GetDocument();
@@ -3028,55 +3028,47 @@ bool nsDocShell::IsSandboxedFrom(nsIDocS
     }
   }
 
   // If no flags, we are not sandboxed at all.
   if (!sandboxFlags) {
     return false;
   }
 
-  // If aTargetDocShell has an ancestor, it is not top level.
-  nsCOMPtr<nsIDocShellTreeItem> ancestorOfTarget;
-  aTargetDocShell->GetInProcessSameTypeParent(getter_AddRefs(ancestorOfTarget));
+  // If aTargetBC has an ancestor, it is not top level.
+  RefPtr<BrowsingContext> ancestorOfTarget(aTargetBC->GetParent());
   if (ancestorOfTarget) {
     do {
       // We are not sandboxed if we are an ancestor of target.
-      if (ancestorOfTarget == this) {
+      if (ancestorOfTarget == mBrowsingContext) {
         return false;
       }
-      nsCOMPtr<nsIDocShellTreeItem> tempTreeItem;
-      ancestorOfTarget->GetInProcessSameTypeParent(
-          getter_AddRefs(tempTreeItem));
-      tempTreeItem.swap(ancestorOfTarget);
+      ancestorOfTarget = ancestorOfTarget->GetParent();
     } while (ancestorOfTarget);
 
-    // Otherwise, we are sandboxed from aTargetDocShell.
+    // Otherwise, we are sandboxed from aTargetBC.
     return true;
   }
 
-  // aTargetDocShell is top level, are we the "one permitted sandboxed
-  // navigator", i.e. did we open aTargetDocShell?
+  // aTargetBC is top level, are we the "one permitted sandboxed
+  // navigator", i.e. did we open aTargetBC?
   RefPtr<BrowsingContext> permittedNavigator(
-      aTargetDocShell->GetBrowsingContext()
-          ->GetOnePermittedSandboxedNavigator());
+      aTargetBC->GetOnePermittedSandboxedNavigator());
   if (permittedNavigator == mBrowsingContext) {
     return false;
   }
 
   // If SANDBOXED_TOPLEVEL_NAVIGATION flag is not on, we are not sandboxed
   // from our top.
-  if (!(sandboxFlags & SANDBOXED_TOPLEVEL_NAVIGATION)) {
-    nsCOMPtr<nsIDocShellTreeItem> rootTreeItem;
-    GetInProcessSameTypeRootTreeItem(getter_AddRefs(rootTreeItem));
-    if (SameCOMIdentity(aTargetDocShell, rootTreeItem)) {
-      return false;
-    }
-  }
-
-  // Otherwise, we are sandboxed from aTargetDocShell.
+  if (!(sandboxFlags & SANDBOXED_TOPLEVEL_NAVIGATION) &&
+      aTargetBC == mBrowsingContext->Top()) {
+    return false;
+  }
+
+  // Otherwise, we are sandboxed from aTargetBC.
   return true;
 }
 
 NS_IMETHODIMP
 nsDocShell::GetTreeOwner(nsIDocShellTreeOwner** aTreeOwner) {
   NS_ENSURE_ARG_POINTER(aTreeOwner);
 
   *aTreeOwner = mTreeOwner;
@@ -9265,17 +9257,17 @@ nsresult nsDocShell::InternalLoad(nsDocS
     if (handled) {
       return NS_OK;
     }
   }
 
   // If a source docshell has been passed, check to see if we are sandboxed
   // from it as the result of an iframe or CSP sandbox.
   if (aLoadState->SourceDocShell() &&
-      aLoadState->SourceDocShell()->IsSandboxedFrom(this)) {
+      aLoadState->SourceDocShell()->IsSandboxedFrom(mBrowsingContext)) {
     return NS_ERROR_DOM_INVALID_ACCESS_ERR;
   }
 
   NS_ENSURE_STATE(!HasUnloadedParent());
 
   rv = CheckLoadingPermissions();
   if (NS_FAILED(rv)) {
     return rv;
--- a/docshell/base/nsIDocShell.idl
+++ b/docshell/base/nsIDocShell.idl
@@ -783,17 +783,17 @@ interface nsIDocShell : nsIDocShellTreeI
    * See nsSandboxFlags.h for the possible flags.
    */
   attribute unsigned long sandboxFlags;
 
   /**
    * Returns true if we are sandboxed from aTargetDocShell.
    * aTargetDocShell - the browsing context we are attempting to navigate.
    */
-  [noscript,notxpcom,nostdcall] bool isSandboxedFrom(in nsIDocShell aTargetDocShell);
+  [noscript,notxpcom,nostdcall] bool isSandboxedFrom(in BrowsingContext aTargetBC);
 
   /**
    * This member variable determines whether a document has Mixed Active Content that
    * was initially blocked from loading, but the user has choosen to override the
    * block and allow the content to load. mMixedContentChannel is set to the document's
    * channel when the user allows mixed content. The nsMixedContentBlocker content policy
    * checks if the document's root channel matches the mMixedContentChannel.  If it matches,
    * then Mixed Content is loaded.  If it does match, mixed content is blocked.
--- a/toolkit/components/windowwatcher/nsWindowWatcher.cpp
+++ b/toolkit/components/windowwatcher/nsWindowWatcher.cpp
@@ -651,18 +651,18 @@ nsresult nsWindowWatcher::OpenWindowInte
   // The state of the window can change before this call and if we are blocked
   // because of sandboxing, we wouldn't want that to happen.
   nsCOMPtr<nsPIDOMWindowOuter> parentWindow =
       aParent ? nsPIDOMWindowOuter::From(aParent) : nullptr;
   nsCOMPtr<nsIDocShell> parentDocShell;
   if (parentWindow) {
     parentDocShell = parentWindow->GetDocShell();
     if (parentDocShell) {
-      nsCOMPtr<nsIDocShell> foundDocShell = do_QueryInterface(newDocShellItem);
-      if (parentDocShell->IsSandboxedFrom(foundDocShell)) {
+      if (parentDocShell->IsSandboxedFrom(
+              newDocShellItem->GetBrowsingContext())) {
         return NS_ERROR_DOM_INVALID_ACCESS_ERR;
       }
     }
   }
 
   // no extant window? make a new one.
 
   // If no parent, consider it chrome when running in the parent process.