Bug 1570207: Part 1 - Move CanAccess logic from DocShell to BrowsingContext. r=nika
authorKris Maglione <maglione.k@gmail.com>
Wed, 31 Jul 2019 11:40:39 -0700
changeset 485612 7bfc8b1c626dcf3cf146fafc606fbe0cf8064d1a
parent 485611 065a5198a0dc0cbe9db889a64da7808ee15a447b
child 485613 783c0e44c713d31bdf14dd84bdded192171343d1
push id113816
push usermaglione.k@gmail.com
push dateThu, 01 Aug 2019 20:19:59 +0000
treeherdermozilla-inbound@783c0e44c713 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1570207
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 1570207: Part 1 - Move CanAccess logic from DocShell to BrowsingContext. r=nika This change is necessary in order to support named targeting of remote BrowsingContexts, since the current arrangement only supports in-process contexts. It also considerably simplifies the logic, since named targetting is now restricted to the same TabGroup/BrowsingContextGroup, which in and of itself guarantees that origin attributes will always match in the cases that we care about. Differential Revision: https://phabricator.services.mozilla.com/D39991
docshell/base/BrowsingContext.cpp
docshell/base/BrowsingContext.h
docshell/base/nsDocShell.cpp
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -524,21 +524,57 @@ BrowsingContext* BrowsingContext::FindWi
             child->FindWithNameInSubtree(aName, aRequestingContext)) {
       return found;
     }
   }
 
   return nullptr;
 }
 
-bool BrowsingContext::CanAccess(BrowsingContext* aContext) {
-  // TODO(farre): Bouncing this to nsDocShell::CanAccessItem is
-  // temporary, we should implement a replacement for this in
-  // BrowsingContext. See Bug 151590.
-  return aContext && nsDocShell::CanAccessItem(aContext->mDocShell, mDocShell);
+// For historical context, see:
+//
+// Bug 13871:   Prevent frameset spoofing
+// Bug 103638:  Targets with same name in different windows open in wrong
+//              window with javascript
+// Bug 408052:  Adopt "ancestor" frame navigation policy
+// Bug 1570207: Refactor logic to rely on BrowsingContextGroups to enforce
+//              origin attribute isolation.
+bool BrowsingContext::CanAccess(BrowsingContext* aTarget,
+                                bool aConsiderOpener) {
+  MOZ_ASSERT(
+      mDocShell,
+      "CanAccess() may only be called in the process of the accessing window");
+  MOZ_ASSERT(aTarget, "Must have a target");
+
+  MOZ_DIAGNOSTIC_ASSERT(
+      Group() == aTarget->Group(),
+      "A BrowsingContext should never see a context from a different group");
+
+  // A frame can navigate itself and its own root.
+  if (aTarget == this || aTarget == Top()) {
+    return true;
+  }
+
+  // A frame can navigate any frame with a same-origin ancestor.
+  for (BrowsingContext* bc = aTarget; bc; bc = bc->GetParent()) {
+    if (bc->mDocShell &&
+        nsDocShell::ValidateOrigin(mDocShell, bc->mDocShell)) {
+      return true;
+    }
+  }
+
+  // If the target is a top-level document, a frame can navigate it if it can
+  // navigate its opener.
+  if (aConsiderOpener && !aTarget->GetParent()) {
+    if (RefPtr<BrowsingContext> opener = aTarget->GetOpener()) {
+      return CanAccess(opener, false);
+    }
+  }
+
+  return false;
 }
 
 BrowsingContext::~BrowsingContext() {
   MOZ_DIAGNOSTIC_ASSERT(!mParent || !mParent->mChildren.Contains(this));
   MOZ_DIAGNOSTIC_ASSERT(!mGroup || !mGroup->Toplevels().Contains(this));
   MOZ_DIAGNOSTIC_ASSERT(!mGroup || !mGroup->IsContextCached(this));
 
   if (sBrowsingContexts) {
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -376,16 +376,19 @@ class BrowsingContext : public nsWrapper
   // Create an IPCInitializer object for this BrowsingContext.
   IPCInitializer GetIPCInitializer();
 
   // Create a BrowsingContext object from over IPC.
   static already_AddRefed<BrowsingContext> CreateFromIPC(
       IPCInitializer&& aInitializer, BrowsingContextGroup* aGroup,
       ContentParent* aOriginProcess);
 
+  // Performs access control to check that 'this' can access 'aTarget'.
+  bool CanAccess(BrowsingContext* aTarget, bool aConsiderOpener = true);
+
  protected:
   virtual ~BrowsingContext();
   BrowsingContext(BrowsingContext* aParent, BrowsingContextGroup* aGroup,
                   uint64_t aBrowsingContextId, Type aType);
 
  private:
   // Find the special browsing context if aName is '_self', '_parent',
   // '_top', but not '_blank'. The latter is handled in FindWithName
@@ -393,19 +396,16 @@ class BrowsingContext : public nsWrapper
 
   // Find a browsing context in the subtree rooted at 'this' Doesn't
   // consider the special names, '_self', '_parent', '_top', or
   // '_blank'. Performs access control with regard to
   // 'aRequestingContext'.
   BrowsingContext* FindWithNameInSubtree(const nsAString& aName,
                                          BrowsingContext* aRequestingContext);
 
-  // Performs access control to check that 'this' can access 'aContext'.
-  bool CanAccess(BrowsingContext* aContext);
-
   // Removes the context from its group and sets mIsDetached to true.
   void Unregister();
 
   friend class ::nsOuterWindowProxy;
   friend class ::nsGlobalWindowOuter;
   // Update the window proxy object that corresponds to this browsing context.
   // This should be called from the window proxy object's objectMoved hook, if
   // the object mWindowProxy points to was moved by the JS GC.
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -251,21 +251,16 @@ static NS_DEFINE_CID(kAppShellCID, NS_AP
 static int32_t gNumberOfDocumentsLoading = 0;
 
 // Global count of existing docshells.
 static int32_t gDocShellCount = 0;
 
 // Global count of docshells with the private attribute set
 static uint32_t gNumberOfPrivateDocShells = 0;
 
-// True means we validate window targets to prevent frameset
-// spoofing. Initialize this to a non-bolean value so we know to check
-// the pref on the creation of the first docshell.
-static uint32_t gValidateOrigin = 0xffffffff;
-
 #ifdef DEBUG
 static mozilla::LazyLogModule gDocShellLog("nsDocShell");
 #endif
 static mozilla::LazyLogModule gDocShellLeakLog("nsDocShellLeak");
 extern mozilla::LazyLogModule gPageCacheLog;
 
 const char kBrandBundleURL[] = "chrome://branding/locale/brand.properties";
 const char kAppstringsBundleURL[] =
@@ -2842,154 +2837,31 @@ nsDocShell::GetSameTypeRootTreeItemIgnor
 }
 
 /* static */
 bool nsDocShell::CanAccessItem(nsIDocShellTreeItem* aTargetItem,
                                nsIDocShellTreeItem* aAccessingItem,
                                bool aConsiderOpener) {
   MOZ_ASSERT(aTargetItem, "Must have target item!");
 
-  if (!gValidateOrigin || !aAccessingItem) {
+  if (!aAccessingItem) {
     // Good to go
     return true;
   }
 
-  // XXXbz should we care if aAccessingItem or the document therein is
-  // chrome?  Should those get extra privileges?
-
-  // For historical context, see:
-  //
-  // Bug 13871:  Prevent frameset spoofing
-  // Bug 103638: Targets with same name in different windows open in wrong
-  //             window with javascript
-  // Bug 408052: Adopt "ancestor" frame navigation policy
-
-  // Now do a security check.
-  //
-  // Disallow navigation if the two frames are not part of the same app, or if
-  // they have different is-in-browser-element states.
-  //
-  // Allow navigation if
-  //  1) aAccessingItem can script aTargetItem or one of its ancestors in
-  //     the frame hierarchy or
-  //  2) aTargetItem is a top-level frame and aAccessingItem is its descendant
-  //  3) aTargetItem is a top-level frame and aAccessingItem can target
-  //     its opener per rule (1) or (2).
-
-  if (aTargetItem == aAccessingItem) {
-    // A frame is allowed to navigate itself.
-    return true;
-  }
-
   nsCOMPtr<nsIDocShell> targetDS = do_QueryInterface(aTargetItem);
   nsCOMPtr<nsIDocShell> accessingDS = do_QueryInterface(aAccessingItem);
   if (!targetDS || !accessingDS) {
     // We must be able to convert both to nsIDocShell.
     return false;
   }
 
-  nsCOMPtr<nsIDocShellTreeItem> accessingRoot;
-  aAccessingItem->GetInProcessSameTypeRootTreeItem(
-      getter_AddRefs(accessingRoot));
-  nsCOMPtr<nsIDocShell> accessingRootDS = do_QueryInterface(accessingRoot);
-
-  nsCOMPtr<nsIDocShellTreeItem> targetRoot;
-  aTargetItem->GetInProcessSameTypeRootTreeItem(getter_AddRefs(targetRoot));
-  nsCOMPtr<nsIDocShell> targetRootDS = do_QueryInterface(targetRoot);
-
-  OriginAttributes targetOA =
-      static_cast<nsDocShell*>(targetDS.get())->GetOriginAttributes();
-  OriginAttributes accessingOA =
-      static_cast<nsDocShell*>(accessingDS.get())->GetOriginAttributes();
-
-  // When the first party isolation is on, the top-level docShell may not have
-  // the firstPartyDomain in its originAttributes, but its document will have
-  // it. So we get the firstPartyDomain from the nodePrincipal of the document
-  // before we compare the originAttributes.
-  if (OriginAttributes::IsFirstPartyEnabled()) {
-    if (aAccessingItem->ItemType() == nsIDocShellTreeItem::typeContent &&
-        (accessingDS == accessingRootDS || accessingDS->GetIsMozBrowser())) {
-      RefPtr<Document> accessingDoc = aAccessingItem->GetDocument();
-
-      if (accessingDoc) {
-        nsCOMPtr<nsIPrincipal> accessingPrincipal =
-            accessingDoc->NodePrincipal();
-
-        accessingOA.mFirstPartyDomain =
-            accessingPrincipal->OriginAttributesRef().mFirstPartyDomain;
-      }
-    }
-
-    if (aTargetItem->ItemType() == nsIDocShellTreeItem::typeContent &&
-        (targetDS == targetRootDS || targetDS->GetIsMozBrowser())) {
-      RefPtr<Document> targetDoc = aAccessingItem->GetDocument();
-
-      if (targetDoc) {
-        nsCOMPtr<nsIPrincipal> targetPrincipal = targetDoc->NodePrincipal();
-
-        targetOA.mFirstPartyDomain =
-            targetPrincipal->OriginAttributesRef().mFirstPartyDomain;
-      }
-    }
-  }
-
-  if (targetOA != accessingOA) {
-    return false;
-  }
-
-  // A private document can't access a non-private one, and vice versa.
-  if (static_cast<nsDocShell*>(targetDS.get())->UsePrivateBrowsing() !=
-      static_cast<nsDocShell*>(accessingDS.get())->UsePrivateBrowsing()) {
-    return false;
-  }
-
-  if (aTargetItem == accessingRoot) {
-    // A frame can navigate its root.
-    return true;
-  }
-
-  // Check if aAccessingItem can navigate one of aTargetItem's ancestors.
-  nsCOMPtr<nsIDocShellTreeItem> target = aTargetItem;
-  do {
-    if (ValidateOrigin(aAccessingItem, target)) {
-      return true;
-    }
-
-    nsCOMPtr<nsIDocShellTreeItem> parent;
-    target->GetInProcessSameTypeParent(getter_AddRefs(parent));
-    parent.swap(target);
-  } while (target);
-
-  if (aTargetItem != targetRoot) {
-    // target is a subframe, not in accessor's frame hierarchy, and all its
-    // ancestors have origins different from that of the accessor. Don't
-    // allow access.
-    return false;
-  }
-
-  if (!aConsiderOpener) {
-    // All done here
-    return false;
-  }
-
-  nsCOMPtr<nsPIDOMWindowOuter> targetWindow = aTargetItem->GetWindow();
-  if (!targetWindow) {
-    NS_ERROR("This should not happen, really");
-    return false;
-  }
-
-  nsCOMPtr<mozIDOMWindowProxy> targetOpener = targetWindow->GetOpener();
-  nsCOMPtr<nsIWebNavigation> openerWebNav(do_GetInterface(targetOpener));
-  nsCOMPtr<nsIDocShellTreeItem> openerItem(do_QueryInterface(openerWebNav));
-
-  if (!openerItem) {
-    return false;
-  }
-
-  return CanAccessItem(openerItem, aAccessingItem, false);
+  return Cast(accessingDS)
+      ->mBrowsingContext->CanAccess(Cast(targetDS)->mBrowsingContext,
+                                    aConsiderOpener);
 }
 
 static bool ItemIsActive(nsIDocShellTreeItem* aItem) {
   if (nsCOMPtr<nsPIDOMWindowOuter> window = aItem->GetWindow()) {
     auto* win = nsGlobalWindowOuter::Cast(window);
     if (!win->GetClosedOuter()) {
       return true;
     }
@@ -4940,22 +4812,16 @@ nsDocShell::Create() {
   }
 
   NS_ASSERTION(mItemType == typeContent || mItemType == typeChrome,
                "Unexpected item type in docshell");
 
   NS_ENSURE_TRUE(Preferences::GetRootBranch(), NS_ERROR_FAILURE);
   mCreated = true;
 
-  if (gValidateOrigin == 0xffffffff) {
-    // Check pref to see if we should prevent frameset spoofing
-    gValidateOrigin =
-        Preferences::GetBool("browser.frame.validate_origin", true);
-  }
-
   mUseStrictSecurityChecks = Preferences::GetBool(
       "security.strict_security_checks.enabled", mUseStrictSecurityChecks);
 
   // Should we use XUL error pages instead of alerts if possible?
   mUseErrorPages = StaticPrefs::browser_xul_error_pages_enabled();
 
   mDisableMetaRefreshWhenInactive =
       Preferences::GetBool("browser.meta_refresh_when_inactive.disabled",
@@ -8615,20 +8481,18 @@ nsresult nsDocShell::CheckLoadingPermiss
   // this docshell. Even though we've done our best to hide windows
   // from code that doesn't have the right to access them, it's
   // still possible for an evil site to open a window and access
   // frames in the new window through window.frames[] (which is
   // allAccess for historic reasons), so we still need to do this
   // check on load.
   nsresult rv = NS_OK;
 
-  if (!gValidateOrigin || !IsFrame()) {
-    // Origin validation was turned off, or we're not a frame.
-    // Permit all loads.
-
+  if (!IsFrame()) {
+    // We're not a frame. Permit all loads.
     return rv;
   }
 
   // Note - The check for a current JSContext here isn't necessarily sensical.
   // It's just designed to preserve the old semantics during a mass-conversion
   // patch.
   if (!nsContentUtils::GetCurrentJSContext()) {
     return NS_OK;