Bug 1607483: Disallow loading http(s) scripts into system privileged contexts. r=tjr

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

--HG--
extra : moz-landing-system : lando

dom/security/nsContentSecurityManager.cpp

@@ -761,32 +761,55 @@ static void DebugDoContentSecurityCheck(nsIChannel* aChannel,
 }
 
 /* static */
-nsresult nsContentSecurityManager::CheckSystemPrincipalLoads(
+nsresult nsContentSecurityManager::CheckAllowLoadInSystemPrivilegedContext(
     nsIChannel* aChannel) {
-  // Assert that we never use the SystemPrincipal to load remote documents
-  // i.e., HTTP, HTTPS, FTP URLs
+  // Check and assert that we never allow remote documents/scripts (http:,
+  // https:, ...) to load in system privileged contexts.
   nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
 
-  // bail out, if we're not loading with a SystemPrincipal
+  // nothing to do here if we are not loading a resource into a
+  // system prvileged context.
   if (!loadInfo->LoadingPrincipal() ||
       !loadInfo->LoadingPrincipal()->IsSystemPrincipal()) {
     return NS_OK;
   }
-  nsContentPolicyType contentPolicyType =
-      loadInfo->GetExternalContentPolicyType();
-  if ((contentPolicyType != nsIContentPolicy::TYPE_DOCUMENT) &&
-      (contentPolicyType != nsIContentPolicy::TYPE_SUBDOCUMENT)) {
-    return NS_OK;
-  }
+
   nsCOMPtr<nsIURI> finalURI;
   NS_GetFinalChannelURI(aChannel, getter_AddRefs(finalURI));
-  // bail out, if URL isn't pointing to remote resource
+
+  // nothing to do here if we are not loading a resource using http:, https:,
+  // etc.
   if (!nsContentUtils::SchemeIs(finalURI, "http") &&
       !nsContentUtils::SchemeIs(finalURI, "https") &&
       !nsContentUtils::SchemeIs(finalURI, "ftp")) {
     return NS_OK;
   }
 
+  nsContentPolicyType contentPolicyType =
+      loadInfo->GetExternalContentPolicyType();
+
+  // We distinguish between 2 cases:
+  // a) remote scripts
+  //    which should never be loaded into system privileged contexts
+  // b) remote documents/frames
+  //    which generally should also never be loaded into system
+  //    privileged contexts but with some exceptions, like e.g. the
+  //    discoverURL.
+  if (contentPolicyType == nsIContentPolicy::TYPE_SCRIPT) {
+    MOZ_LOG(sCSMLog, LogLevel::Warning,
+            ("Do not load remote scripts into system privileged contexts"));
+    MOZ_ASSERT(false,
+               "Do not load remote scripts into system privileged contexts");
+    // Bug 1607673: Do not only assert but cancel the channel and
+    // return NS_ERROR_CONTENT_BLOCKED.
+    return NS_OK;
+  }
+
+  if ((contentPolicyType != nsIContentPolicy::TYPE_DOCUMENT) &&
+      (contentPolicyType != nsIContentPolicy::TYPE_SUBDOCUMENT)) {
+    return NS_OK;
+  }
+
   // FIXME The discovery feature in about:addons uses the SystemPrincpal.
   // We should remove the exception for AMO with bug 1544011.
   // We should remove the exception for Firefox Accounts with bug 1561318.
@@ -831,10 +854,6 @@ nsresult nsContentSecurityManager::CheckSystemPrincipalLoads(
 #endif
   nsAutoCString requestedURL;
   finalURI->GetAsciiSpec(requestedURL);
-  MOZ_LOG(
-      sCSMLog, LogLevel::Verbose,
-      ("SystemPrincipal must not load remote documents. URL: %s", requestedURL)
-          .get());
   if (xpc::AreNonLocalConnectionsDisabled()) {
     bool disallowSystemPrincipalRemoteDocuments = Preferences::GetBool(
         "security.disallow_non_local_systemprincipal_in_tests");
@@ -847,6 +866,10 @@ nsresult nsContentSecurityManager::CheckSystemPrincipalLoads(
     // but other mochitest are exempt from this
     return NS_OK;
   }
+  MOZ_LOG(
+      sCSMLog, LogLevel::Warning,
+      ("SystemPrincipal must not load remote documents. URL: %s", requestedURL)
+          .get());
   MOZ_ASSERT(false, "SystemPrincipal must not load remote documents.");
   aChannel->Cancel(NS_ERROR_CONTENT_BLOCKED);
   return NS_ERROR_CONTENT_BLOCKED;
@@ -878,7 +901,7 @@ nsresult nsContentSecurityManager::doContentSecurityCheck(
     DebugDoContentSecurityCheck(aChannel, loadInfo);
   }
 
-  nsresult rv = CheckSystemPrincipalLoads(aChannel);
+  nsresult rv = CheckAllowLoadInSystemPrivilegedContext(aChannel);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // if dealing with a redirected channel then we have already installed

dom/security/nsContentSecurityManager.h

@@ -41,7 +41,7 @@ class nsContentSecurityManager : public nsIContentSecurityManager,
  private:
   static nsresult CheckChannel(nsIChannel* aChannel);
   static nsresult CheckFTPSubresourceLoad(nsIChannel* aChannel);
-  static nsresult CheckSystemPrincipalLoads(nsIChannel* aChannel);
+  static nsresult CheckAllowLoadInSystemPrivilegedContext(nsIChannel* aChannel);
 
   virtual ~nsContentSecurityManager() {}
 };