[tbb-commits] [tor-browser/tor-browser-68.1.0esr-9.0-2] Bug 1573276 - Always allow localization in error pages r=johannh, peterv

gk at torproject.org gk at torproject.org
Thu Sep 26 11:32:41 UTC 2019


commit 9c1877056070c4de048b18e201fcb3dfcb1e1a02
Author: Alex Catarineu <acat at torproject.org>
Date:   Wed Sep 25 10:39:45 2019 +0000

    Bug 1573276 - Always allow localization in error pages r=johannh,peterv
    
    Differential Revision: https://phabricator.services.mozilla.com/D43216
    
    --HG--
    extra : moz-landing-system : lando
---
 .../content/test/about/browser_aboutCertError.js   | 23 ++++++++++++++++++++
 browser/base/content/test/about/head.js            | 13 ++++++-----
 dom/base/Document.cpp                              |  8 ++++---
 dom/base/nsContentUtils.cpp                        | 25 +++++++++++++++++++++-
 dom/base/nsContentUtils.h                          |  9 ++++----
 dom/security/nsContentSecurityManager.cpp          |  6 +++++-
 parser/htmlparser/nsExpatDriver.cpp                | 22 +++++++++++--------
 7 files changed, 83 insertions(+), 23 deletions(-)

diff --git a/browser/base/content/test/about/browser_aboutCertError.js b/browser/base/content/test/about/browser_aboutCertError.js
index 1544bab15f99..3e9a13dd99f3 100644
--- a/browser/base/content/test/about/browser_aboutCertError.js
+++ b/browser/base/content/test/about/browser_aboutCertError.js
@@ -459,3 +459,26 @@ add_task(async function checkBadStsCertHeadline() {
     BrowserTestUtils.removeTab(gBrowser.selectedTab);
   }
 });
+
+add_task(async function checkSandboxedIframe() {
+  info(
+    "Loading a bad sts cert error in a sandboxed iframe and check that the correct headline is shown"
+  );
+  let useFrame = true;
+  let sandboxed = true;
+  let tab = await openErrorPage(BAD_CERT, useFrame, sandboxed);
+  let browser = tab.linkedBrowser;
+
+  let titleContent = await ContentTask.spawn(browser, {}, async function() {
+    // Cannot test for error in the Advanced section since it's currently not present
+    // in a sandboxed iframe.
+    let doc = content.document.querySelector("iframe").contentDocument;
+    let titleText = doc.querySelector(".title-text");
+    return titleText.textContent;
+  });
+  ok(
+    titleContent.endsWith("Security Issue"),
+    "Did Not Connect: Potential Security Issue"
+  );
+  BrowserTestUtils.removeTab(gBrowser.selectedTab);
+});
diff --git a/browser/base/content/test/about/head.js b/browser/base/content/test/about/head.js
index 82fa5dcd540d..74f961dcefae 100644
--- a/browser/base/content/test/about/head.js
+++ b/browser/base/content/test/about/head.js
@@ -44,17 +44,20 @@ function getPEMString(cert) {
   );
 }
 
-function injectErrorPageFrame(tab, src) {
+function injectErrorPageFrame(tab, src, sandboxed) {
   return ContentTask.spawn(
     tab.linkedBrowser,
-    { frameSrc: src },
-    async function({ frameSrc }) {
+    { frameSrc: src, frameSandboxed: sandboxed },
+    async function({ frameSrc, frameSandboxed }) {
       let loaded = ContentTaskUtils.waitForEvent(
         content.wrappedJSObject,
         "DOMFrameContentLoaded"
       );
       let iframe = content.document.createElement("iframe");
       iframe.src = frameSrc;
+      if (frameSandboxed) {
+        iframe.setAttribute("sandbox", "allow-scripts");
+      }
       content.document.body.appendChild(iframe);
       await loaded;
       // We will have race conditions when accessing the frame content after setting a src,
@@ -67,7 +70,7 @@ function injectErrorPageFrame(tab, src) {
   );
 }
 
-async function openErrorPage(src, useFrame) {
+async function openErrorPage(src, useFrame, sandboxed) {
   let dummyPage =
     getRootDirectory(gTestPath).replace(
       "chrome://mochitests/content",
@@ -78,7 +81,7 @@ async function openErrorPage(src, useFrame) {
   if (useFrame) {
     info("Loading cert error page in an iframe");
     tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, dummyPage);
-    await injectErrorPageFrame(tab, src);
+    await injectErrorPageFrame(tab, src, sandboxed);
   } else {
     let certErrorLoaded;
     tab = await BrowserTestUtils.openNewForegroundTab(
diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp
index eebeada1c63c..cdeabc4dc7b9 100644
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -3225,11 +3225,13 @@ DocumentL10n* Document::GetL10n() { return mDocumentL10n; }
 bool Document::DocumentSupportsL10n(JSContext* aCx, JSObject* aObject) {
   nsCOMPtr<nsIPrincipal> callerPrincipal =
       nsContentUtils::SubjectPrincipal(aCx);
-  return nsContentUtils::PrincipalAllowsL10n(callerPrincipal);
+  nsGlobalWindowInner* win = xpc::WindowOrNull(aObject);
+  return nsContentUtils::PrincipalAllowsL10n(
+      callerPrincipal, win ? win->GetDocumentURI() : nullptr);
 }
 
 void Document::LocalizationLinkAdded(Element* aLinkElement) {
-  if (!nsContentUtils::PrincipalAllowsL10n(NodePrincipal())) {
+  if (!nsContentUtils::PrincipalAllowsL10n(NodePrincipal(), GetDocumentURI())) {
     return;
   }
 
@@ -3260,7 +3262,7 @@ void Document::LocalizationLinkAdded(Element* aLinkElement) {
 }
 
 void Document::LocalizationLinkRemoved(Element* aLinkElement) {
-  if (!nsContentUtils::PrincipalAllowsL10n(NodePrincipal())) {
+  if (!nsContentUtils::PrincipalAllowsL10n(NodePrincipal(), GetDocumentURI())) {
     return;
   }
 
diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp
index 66134cdb691f..cfae3ef224b3 100644
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -250,6 +250,7 @@
 #include "nsThreadManager.h"
 #include "nsIBidiKeyboard.h"
 #include "ReferrerInfo.h"
+#include "nsAboutProtocolUtils.h"
 
 #if defined(XP_WIN)
 // Undefine LoadImage to prevent naming conflict with Windows.
@@ -1676,8 +1677,30 @@ bool nsContentUtils::OfflineAppAllowed(nsIPrincipal* aPrincipal) {
   return NS_SUCCEEDED(rv) && allowed;
 }
 
+static bool IsErrorPage(nsIURI* aURI) {
+  if (!aURI) {
+    return false;
+  }
+
+  if (!aURI->SchemeIs("about")) {
+    return false;
+  }
+
+  nsAutoCString name;
+  nsresult rv = NS_GetAboutModuleName(aURI, name);
+  NS_ENSURE_SUCCESS(rv, false);
+
+  return name.EqualsLiteral("certerror") || name.EqualsLiteral("neterror") ||
+         name.EqualsLiteral("blocked");
+}
+
 /* static */
-bool nsContentUtils::PrincipalAllowsL10n(nsIPrincipal* aPrincipal) {
+bool nsContentUtils::PrincipalAllowsL10n(nsIPrincipal* aPrincipal,
+                                         nsIURI* aDocumentURI) {
+  if (IsErrorPage(aDocumentURI)) {
+    return true;
+  }
+
   // The system principal is always allowed.
   if (IsSystemPrincipal(aPrincipal)) {
     return true;
diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h
index 46818cc43a1b..2bf27250e68b 100644
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -1984,11 +1984,12 @@ class nsContentUtils {
   static bool OfflineAppAllowed(nsIPrincipal* aPrincipal);
 
   /**
-   * Determine whether the principal is allowed access to the localization
-   * system. We don't want the web to ever see this but all our UI including in
-   * content pages should pass this test.
+   * Determine whether the principal or document is allowed access to the
+   * localization system. We don't want the web to ever see this but all our UI
+   * including in content pages should pass this test.
    */
-  static bool PrincipalAllowsL10n(nsIPrincipal* aPrincipal);
+  static bool PrincipalAllowsL10n(nsIPrincipal* aPrincipal,
+                                  nsIURI* aDocumentURI);
 
   /**
    * If offline-apps.allow_by_default is true, we set offline-app permission
diff --git a/dom/security/nsContentSecurityManager.cpp b/dom/security/nsContentSecurityManager.cpp
index e7a0a9d1a72b..10ee46858654 100644
--- a/dom/security/nsContentSecurityManager.cpp
+++ b/dom/security/nsContentSecurityManager.cpp
@@ -333,7 +333,11 @@ static nsresult DoCheckLoadURIChecks(nsIURI* aURI, nsILoadInfo* aLoadInfo) {
   // same principal check as Fluent.
   if (aLoadInfo->InternalContentPolicyType() ==
       nsIContentPolicy::TYPE_INTERNAL_DTD) {
-    return nsContentUtils::PrincipalAllowsL10n(aLoadInfo->TriggeringPrincipal())
+    RefPtr<Document> doc;
+    aLoadInfo->GetLoadingDocument(getter_AddRefs(doc));
+    return nsContentUtils::PrincipalAllowsL10n(
+               aLoadInfo->TriggeringPrincipal(),
+               doc ? doc->GetDocumentURI() : nullptr)
                ? NS_OK
                : NS_ERROR_DOM_BAD_URI;
   }
diff --git a/parser/htmlparser/nsExpatDriver.cpp b/parser/htmlparser/nsExpatDriver.cpp
index 9f2321cd2831..73a0e65328f0 100644
--- a/parser/htmlparser/nsExpatDriver.cpp
+++ b/parser/htmlparser/nsExpatDriver.cpp
@@ -628,33 +628,37 @@ nsresult nsExpatDriver::OpenInputStreamFromExternalDTD(const char16_t* aFPIStr,
                        nsContentUtils::GetSystemPrincipal(),
                        nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL,
                        nsIContentPolicy::TYPE_DTD);
+    NS_ENSURE_SUCCESS(rv, rv);
   } else {
     NS_ASSERTION(
         mSink == nsCOMPtr<nsIExpatSink>(do_QueryInterface(mOriginalSink)),
         "In nsExpatDriver::OpenInputStreamFromExternalDTD: "
         "mOriginalSink not the same object as mSink?");
     nsContentPolicyType policyType = nsIContentPolicy::TYPE_INTERNAL_DTD;
-    nsCOMPtr<nsIPrincipal> loadingPrincipal;
     if (mOriginalSink) {
       nsCOMPtr<Document> doc;
       doc = do_QueryInterface(mOriginalSink->GetTarget());
       if (doc) {
-        loadingPrincipal = doc->NodePrincipal();
         if (doc->SkipDTDSecurityChecks()) {
           policyType = nsIContentPolicy::TYPE_INTERNAL_FORCE_ALLOWED_DTD;
         }
+        rv = NS_NewChannel(getter_AddRefs(channel), uri, doc,
+                           nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS |
+                               nsILoadInfo::SEC_ALLOW_CHROME,
+                           policyType);
+        NS_ENSURE_SUCCESS(rv, rv);
       }
     }
-    if (!loadingPrincipal) {
-      loadingPrincipal =
+    if (!channel) {
+      nsCOMPtr<nsIPrincipal> nullPrincipal =
           mozilla::NullPrincipal::CreateWithoutOriginAttributes();
+      rv = NS_NewChannel(getter_AddRefs(channel), uri, nullPrincipal,
+                         nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS |
+                             nsILoadInfo::SEC_ALLOW_CHROME,
+                         policyType);
+      NS_ENSURE_SUCCESS(rv, rv);
     }
-    rv = NS_NewChannel(getter_AddRefs(channel), uri, loadingPrincipal,
-                       nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS |
-                           nsILoadInfo::SEC_ALLOW_CHROME,
-                       policyType);
   }
-  NS_ENSURE_SUCCESS(rv, rv);
 
   nsAutoCString absURL;
   rv = uri->GetSpec(absURL);



More information about the tbb-commits mailing list