[tor-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 tor-commits
mailing list