commit 9c1877056070c4de048b18e201fcb3dfcb1e1a02 Author: Alex Catarineu acat@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);