Bug 1586926 - Add necessary caller access checks for cross-process Location navigations. r=nika
authorKris Maglione <maglione.k@gmail.com>
Thu, 10 Oct 2019 19:36:23 +0000
changeset 497298 a3d7e24bd090c093fc9822a83402f7c3502a4d5b
parent 497297 f9209979c9bfe7c97d13f80fd507cd779af26fd8
child 497299 d3f5f69f38a829af852593d2202976e67598d883
push id36682
push userncsoregi@mozilla.com
push dateSat, 12 Oct 2019 09:52:03 +0000
treeherdermozilla-central@06ea2371f897 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1586926
milestone71.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 1586926 - Add necessary caller access checks for cross-process Location navigations. r=nika We attempt to enforce the same (approximate) access checks to Location-based navigation that we use for loads that use named targeting (e.g., via window.open), so that a frame that can't be navigated via, e.g., window.open, also can't be navigated via, e.g., window.parent[1].location = url. For the in-process case, this is handled by a somewhat hidden call to CheckLoadingPermissions() in nsDocShell::InternalLoad, where the former checks whether the principal of whatever JS context happens to be on the stack subsumes the principal of the target DocShell or any of its ancestors, and blocks the load if it doesn't. Since there is no JS context on the stack when we call into the DocShell loading code in the cross-process case, the check is simply ignored. So we need to instead do the check in BrowsingContext::LoadURI, where we already have an explicit accessor, and can simply use the standard access checks that we use elsewhere. Differential Revision: https://phabricator.services.mozilla.com/D48443
docshell/base/BrowsingContext.cpp
docshell/test/navigation/mochitest.ini
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -855,16 +855,20 @@ nsresult BrowsingContext::LoadURI(Browsi
 
   if (!aAccessor && XRE_IsParentProcess()) {
     Unused << Canonical()->GetCurrentWindowGlobal()->SendLoadURIInChild(
         aLoadState, aSetNavigating);
   } else {
     MOZ_DIAGNOSTIC_ASSERT(aAccessor);
     MOZ_DIAGNOSTIC_ASSERT(aAccessor->Group() == Group());
 
+    if (!aAccessor->CanAccess(this)) {
+      return NS_ERROR_DOM_PROP_ACCESS_DENIED;
+    }
+
     nsCOMPtr<nsPIDOMWindowOuter> win(aAccessor->GetDOMWindow());
     MOZ_DIAGNOSTIC_ASSERT(win);
     if (WindowGlobalChild* wgc =
             win->GetCurrentInnerWindow()->GetWindowGlobalChild()) {
       wgc->SendLoadURI(this, aLoadState, aSetNavigating);
     }
   }
   return NS_OK;
--- a/docshell/test/navigation/mochitest.ini
+++ b/docshell/test/navigation/mochitest.ini
@@ -87,14 +87,13 @@ skip-if =
   fission # Times out.
 [test_performance_navigation.html]
 [test_sessionhistory.html]
 skip-if = toolkit == 'android' # RANDOM on android
 support-files = file_bug1379762-1.html
 [test_sibling-matching-parent.html]
 skip-if = fission # Times out.
 [test_sibling-off-domain.html]
-fail-if = fission
 [test_triggeringprincipal_frame_nav.html]
 [test_triggeringprincipal_window_open.html]
 [test_triggeringprincipal_parent_iframe_window_open.html]
 [test_triggeringprincipal_iframe_iframe_window_open.html]
 [test_contentpolicy_block_window.html]