Bug 1883860 - nsDocShell::HandleSameDocumentNavigation() cause crash in SetDocumentURI, r=smaug

Differential Revision: https://phabricator.services.mozilla.com/D204611

caps/nsScriptSecurityManager.cpp

@@ -225,6 +225,18 @@ uint32_t nsScriptSecurityManager::SecurityHashURI(nsIURI* aURI) {
   return NS_SecurityHashURI(aURI);
 }
 
+bool nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(nsIURI* aUriA,
+                                                          nsIURI* aUriB) {
+  if (!aUriA || (!net::SchemeIsHTTP(aUriA) && !net::SchemeIsHTTPS(aUriA)) ||
+      !aUriB || (!net::SchemeIsHTTP(aUriB) && !net::SchemeIsHTTPS(aUriB))) {
+    return false;
+  }
+  if (!SecurityCompareURIs(aUriA, aUriB)) {
+    return true;
+  }
+  return false;
+}
+
 /*
  * GetChannelResultPrincipal will return the principal that the resource
  * returned by this channel will use.  For example, if the resource is in

caps/nsScriptSecurityManager.h

@@ -70,6 +70,7 @@ class nsScriptSecurityManager final : public nsIScriptSecurityManager {
    */
   static bool SecurityCompareURIs(nsIURI* aSourceURI, nsIURI* aTargetURI);
   static uint32_t SecurityHashURI(nsIURI* aURI);
+  static bool IsHttpOrHttpsAndCrossOrigin(nsIURI* aUriA, nsIURI* aUriB);
 
   static nsresult ReportError(const char* aMessageTag, nsIURI* aSource,
                               nsIURI* aTarget, bool aFromPrivateWindow,

caps/tests/gtest/TestScriptSecurityManager.cpp

@@ -0,0 +1,50 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+#include "gtest/gtest.h"
+
+#include "nsCOMPtr.h"
+#include "nsIURI.h"
+#include "nsNetUtil.h"
+#include "nsScriptSecurityManager.h"
+
+namespace mozilla {
+
+TEST(ScriptSecurityManager, IsHttpOrHttpsAndCrossOrigin)
+{
+  nsCOMPtr<nsIURI> uriA;
+  NS_NewURI(getter_AddRefs(uriA), "https://apple.com");
+  nsCOMPtr<nsIURI> uriB;
+  NS_NewURI(getter_AddRefs(uriB), "https://google.com");
+  nsCOMPtr<nsIURI> uriB_http;
+  NS_NewURI(getter_AddRefs(uriB_http), "http://google.com");
+  nsCOMPtr<nsIURI> aboutBlank;
+  NS_NewURI(getter_AddRefs(aboutBlank), "about:blank");
+  nsCOMPtr<nsIURI> aboutConfig;
+  NS_NewURI(getter_AddRefs(aboutConfig), "about:config");
+  nsCOMPtr<nsIURI> example_com;
+  NS_NewURI(getter_AddRefs(example_com), "https://example.com");
+  nsCOMPtr<nsIURI> example_com_with_path;
+  NS_NewURI(getter_AddRefs(example_com_with_path),
+            "https://example.com/test/1/2/3");
+  nsCOMPtr<nsIURI> nullURI = nullptr;
+
+  ASSERT_TRUE(nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(uriA, uriB));
+  ASSERT_TRUE(
+      nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(uriB, uriB_http));
+
+  ASSERT_FALSE(nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(
+      aboutBlank, aboutConfig));
+  ASSERT_FALSE(nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(
+      aboutBlank, aboutBlank));
+  ASSERT_FALSE(
+      nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(uriB, aboutConfig));
+  ASSERT_FALSE(nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(
+      example_com, example_com_with_path));
+  ASSERT_FALSE(
+      nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(uriB_http, nullURI));
+  ASSERT_FALSE(
+      nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(nullURI, uriB_http));
+}
+
+}  // namespace mozilla

caps/tests/gtest/moz.build

@@ -11,6 +11,7 @@ UNIFIED_SOURCES += [
     "TestPrincipalAttributes.cpp",
     "TestPrincipalSerialization.cpp",
     "TestRedirectChainURITruncation.cpp",
+    "TestScriptSecurityManager.cpp",
 ]
 
 include("/ipc/chromium/chromium-config.mozbuild")

docshell/base/nsDocShell.cpp

@@ -144,6 +144,7 @@
 #include "nsIScriptChannel.h"
 #include "nsIScriptObjectPrincipal.h"
 #include "nsIScriptSecurityManager.h"
+#include "nsScriptSecurityManager.h"
 #include "nsIScrollableFrame.h"
 #include "nsIScrollObserver.h"
 #include "nsISupportsPrimitives.h"
@@ -8689,24 +8690,18 @@ nsresult nsDocShell::HandleSameDocumentNavigation(
       }
     }
 
-    auto isLoadableViaInternet = [](nsIURI* uri) {
-      return (uri && (net::SchemeIsHTTP(uri) || net::SchemeIsHTTPS(uri)));
-    };
-
-    if (isLoadableViaInternet(principalURI) &&
-        isLoadableViaInternet(mCurrentURI) && isLoadableViaInternet(newURI)) {
-      nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager();
-      if (!NS_SUCCEEDED(
-              ssm->CheckSameOriginURI(newURI, principalURI, false, false)) ||
-          !NS_SUCCEEDED(ssm->CheckSameOriginURI(mCurrentURI, principalURI,
-                                                false, false))) {
-        MOZ_LOG(gSHLog, LogLevel::Debug,
-                ("nsDocShell[%p]: possible violation of the same origin policy "
-                 "during same document navigation",
-                 this));
-        aSameDocument = false;
-        return NS_OK;
-      }
+    if (nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(principalURI,
+                                                             newURI) ||
+        nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(principalURI,
+                                                             mCurrentURI) ||
+        nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(mCurrentURI,
+                                                             newURI)) {
+      aSameDocument = false;
+      MOZ_LOG(gSHLog, LogLevel::Debug,
+              ("nsDocShell[%p]: possible violation of the same origin policy "
+               "during same document navigation",
+               this));
+      return NS_OK;
     }
   }
 

dom/ipc/WindowGlobalChild.cpp

@@ -35,7 +35,7 @@
 #include "nsQueryObject.h"
 #include "nsSerializationHelper.h"
 #include "nsFrameLoader.h"
-#include "nsIScriptSecurityManager.h"
+#include "nsScriptSecurityManager.h"
 
 #include "mozilla/dom/JSWindowActorBinding.h"
 #include "mozilla/dom/JSWindowActorChild.h"
@@ -588,30 +588,19 @@ void WindowGlobalChild::SetDocumentURI(nsIURI* aDocumentURI) {
       embedderInnerWindowID, BrowsingContext()->UsePrivateBrowsing());
 
   if (StaticPrefs::dom_security_setdocumenturi()) {
-    auto isLoadableViaInternet = [](nsIURI* uri) {
-      return (uri && (net::SchemeIsHTTP(uri) || net::SchemeIsHTTPS(uri)));
-    };
-    if (isLoadableViaInternet(aDocumentURI)) {
-      nsCOMPtr<nsIURI> principalURI = mDocumentPrincipal->GetURI();
-      if (mDocumentPrincipal->GetIsNullPrincipal()) {
-        nsCOMPtr<nsIPrincipal> precursor =
-            mDocumentPrincipal->GetPrecursorPrincipal();
-        if (precursor) {
-          principalURI = precursor->GetURI();
-        }
-      }
-
-      if (isLoadableViaInternet(principalURI)) {
-        nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager();
-
-        if (!NS_SUCCEEDED(ssm->CheckSameOriginURI(principalURI, aDocumentURI,
-                                                  false, false))) {
-          MOZ_DIAGNOSTIC_ASSERT(false,
-                                "Setting DocumentURI with a different origin "
-                                "than principal URI");
-        }
+    nsCOMPtr<nsIURI> principalURI = mDocumentPrincipal->GetURI();
+    if (mDocumentPrincipal->GetIsNullPrincipal()) {
+      nsCOMPtr<nsIPrincipal> precursor =
+          mDocumentPrincipal->GetPrecursorPrincipal();
+      if (precursor) {
+        principalURI = precursor->GetURI();
       }
     }
+
+    MOZ_DIAGNOSTIC_ASSERT(!nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(
+                              principalURI, aDocumentURI),
+                          "Setting DocumentURI with a different origin "
+                          "than principal URI");
   }
 
   mDocumentURI = aDocumentURI;

dom/ipc/WindowGlobalParent.cpp

@@ -399,26 +399,20 @@ IPCResult WindowGlobalParent::RecvUpdateDocumentURI(NotNull<nsIURI*> aURI) {
       return IPC_FAIL(this, "Setting DocumentURI with unknown protocol.");
     }
 
-    auto isLoadableViaInternet = [](nsIURI* uri) {
-      return (uri && (net::SchemeIsHTTP(uri) || net::SchemeIsHTTPS(uri)));
-    };
-
-    if (isLoadableViaInternet(aURI)) {
-      nsCOMPtr<nsIURI> principalURI = mDocumentPrincipal->GetURI();
-      if (mDocumentPrincipal->GetIsNullPrincipal()) {
-        nsCOMPtr<nsIPrincipal> precursor =
-            mDocumentPrincipal->GetPrecursorPrincipal();
-        if (precursor) {
-          principalURI = precursor->GetURI();
-        }
+    nsCOMPtr<nsIURI> principalURI = mDocumentPrincipal->GetURI();
+    if (mDocumentPrincipal->GetIsNullPrincipal()) {
+      nsCOMPtr<nsIPrincipal> precursor =
+          mDocumentPrincipal->GetPrecursorPrincipal();
+      if (precursor) {
+        principalURI = precursor->GetURI();
       }
+    }
 
-      if (isLoadableViaInternet(principalURI) &&
-          !nsScriptSecurityManager::SecurityCompareURIs(principalURI, aURI)) {
-        return IPC_FAIL(this,
-                        "Setting DocumentURI with a different Origin than "
-                        "principal URI");
-      }
+    if (nsScriptSecurityManager::IsHttpOrHttpsAndCrossOrigin(principalURI,
+                                                             aURI)) {
+      return IPC_FAIL(this,
+                      "Setting DocumentURI with a different Origin than "
+                      "principal URI");
     }
   }