author | Oana Pop Rus <opoprus@mozilla.com> |
Fri, 10 Jan 2020 16:04:27 +0200 | |
changeset 509731 | 7a3be2bbce032721ec01a9b75d88cc7c6b089825 |
parent 509730 | a75f79ec3e900031a2d4d19a4141d690ecf5b552 |
child 509732 | a16ed94b0571a0374c3188e86dd57d71fd92e0ae |
push id | 104810 |
push user | opoprus@mozilla.com |
push date | Fri, 10 Jan 2020 14:05:20 +0000 |
treeherder | autoland@7a3be2bbce03 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
bugs | 1596360 |
milestone | 74.0a1 |
backs out | b80ab0927b404f844b2adc88501275217e2e4078 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/browser/base/content/test/general/browser_testOpenNewRemoteTabsFromNonRemoteBrowsers.js +++ b/browser/base/content/test/general/browser_testOpenNewRemoteTabsFromNonRemoteBrowsers.js @@ -35,20 +35,16 @@ registerCleanupFunction(() => { Services.prefs.clearUserPref(OPEN_LOCATION_PREF); }); /** * Test that if we open a new tab from a link in a non-remote * browser in an e10s window, that the new tab will load properly. */ add_task(async function test_new_tab() { - await SpecialPowers.pushPrefEnv({ - set: [["domsecurity.skip_html_fragment_assertion", true]], - }); - let normalWindow = await BrowserTestUtils.openNewBrowserWindow({ remote: true, }); let privateWindow = await BrowserTestUtils.openNewBrowserWindow({ remote: true, private: true, }); @@ -83,20 +79,16 @@ add_task(async function test_new_tab() { }); /** * Test that if we open a new window from a link in a non-remote * browser in an e10s window, that the new window is not an e10s * window. Also tests with a private browsing window. */ add_task(async function test_new_window() { - await SpecialPowers.pushPrefEnv({ - set: [["domsecurity.skip_html_fragment_assertion", true]], - }); - let normalWindow = await BrowserTestUtils.openNewBrowserWindow( { remote: true, }, true ); let privateWindow = await BrowserTestUtils.openNewBrowserWindow( {
--- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -52,17 +52,16 @@ #include "mozilla/dom/ContentChild.h" #include "mozilla/dom/CustomElementRegistry.h" #include "mozilla/dom/Document.h" #include "mozilla/dom/DocumentInlines.h" #include "mozilla/dom/MessageBroadcaster.h" #include "mozilla/dom/DocumentFragment.h" #include "mozilla/dom/DOMException.h" #include "mozilla/dom/DOMExceptionBinding.h" -#include "mozilla/dom/DOMSecurityMonitor.h" #include "mozilla/dom/DOMTypes.h" #include "mozilla/dom/Element.h" #include "mozilla/dom/ElementInlines.h" #include "mozilla/dom/Event.h" #include "mozilla/dom/FileSystemSecurity.h" #include "mozilla/dom/FileBlobImpl.h" #include "mozilla/dom/FontTableURIProtocolHandler.h" #include "mozilla/dom/HTMLInputElement.h" @@ -4755,109 +4754,73 @@ already_AddRefed<DocumentFragment> nsCon } } content = content->GetParent(); } RefPtr<DocumentFragment> frag; aRv = ParseFragmentXML(aFragment, document, tagStack, aPreventScriptExecution, - -1, getter_AddRefs(frag)); + getter_AddRefs(frag)); return frag.forget(); } /* static */ void nsContentUtils::DropFragmentParsers() { NS_IF_RELEASE(sHTMLFragmentParser); NS_IF_RELEASE(sXMLFragmentParser); NS_IF_RELEASE(sXMLFragmentSink); } /* static */ void nsContentUtils::XPCOMShutdown() { nsContentUtils::DropFragmentParsers(); } -/* Helper function to compuate Sanitization Flags for ParseFramentHTML/XML */ -uint32_t computeSanitizationFlags(nsIPrincipal* aPrincipal, int32_t aFlags) { - uint32_t sanitizationFlags = 0; - if (aPrincipal->IsSystemPrincipal()) { - if (aFlags < 0) { - // if this is a chrome-privileged document and no explicit flags - // were passed, then use this sanitization flags. - sanitizationFlags = nsIParserUtils::SanitizerAllowStyle | - nsIParserUtils::SanitizerAllowComments | - nsIParserUtils::SanitizerDropForms | - nsIParserUtils::SanitizerLogRemovals; - } else { - // if the caller explicitly passes flags, then we use those - // flags but additionally drop forms. - sanitizationFlags = aFlags | nsIParserUtils::SanitizerDropForms; - } - } else if (aFlags >= 0) { - // aFlags by default is -1 and is only ever non equal to -1 if the - // caller of ParseFragmentHTML/ParseFragmentXML is - // ParserUtils::ParseFragment(). Only in that case we should use - // the sanitization flags passed within aFlags. - sanitizationFlags = aFlags; - } - return sanitizationFlags; -} - -/* static */ -nsresult nsContentUtils::ParseFragmentHTML( - const nsAString& aSourceBuffer, nsIContent* aTargetNode, - nsAtom* aContextLocalName, int32_t aContextNamespace, bool aQuirks, - bool aPreventScriptExecution, int32_t aFlags) { +/* static */ +nsresult nsContentUtils::ParseFragmentHTML(const nsAString& aSourceBuffer, + nsIContent* aTargetNode, + nsAtom* aContextLocalName, + int32_t aContextNamespace, + bool aQuirks, + bool aPreventScriptExecution) { AutoTimelineMarker m(aTargetNode->OwnerDoc()->GetDocShell(), "Parse HTML"); if (nsContentUtils::sFragmentParsingActive) { MOZ_ASSERT_UNREACHABLE("Re-entrant fragment parsing attempted."); return NS_ERROR_DOM_INVALID_STATE_ERR; } mozilla::AutoRestore<bool> guard(nsContentUtils::sFragmentParsingActive); nsContentUtils::sFragmentParsingActive = true; if (!sHTMLFragmentParser) { NS_ADDREF(sHTMLFragmentParser = new nsHtml5StringParser()); // Now sHTMLFragmentParser owns the object } - nsCOMPtr<nsIPrincipal> nodePrincipal = aTargetNode->NodePrincipal(); - -#ifdef DEBUG - // aFlags should always be -1 unless the caller of ParseFragmentHTML - // is ParserUtils::ParseFragment() which is the only caller that intends - // sanitization. For all other callers we need to ensure to call - // AuditParsingOfHTMLXMLFragments. - if (aFlags < 0) { - DOMSecurityMonitor::AuditParsingOfHTMLXMLFragments(nodePrincipal, - aSourceBuffer); - } -#endif - nsIContent* target = aTargetNode; + // If this is a chrome-privileged document, create a fragment first, and + // sanitize it before insertion. RefPtr<DocumentFragment> fragment; - // We sanitize if the fragment occurs in a system privileged - // context or if there are explicit sanitization flags. - bool shouldSanitize = nodePrincipal->IsSystemPrincipal() || aFlags >= 0; - if (shouldSanitize) { + if (aTargetNode->NodePrincipal()->IsSystemPrincipal()) { fragment = new DocumentFragment(aTargetNode->OwnerDoc()->NodeInfoManager()); target = fragment; } nsresult rv = sHTMLFragmentParser->ParseFragment( aSourceBuffer, target, aContextLocalName, aContextNamespace, aQuirks, aPreventScriptExecution); NS_ENSURE_SUCCESS(rv, rv); if (fragment) { - uint32_t sanitizationFlags = - computeSanitizationFlags(nodePrincipal, aFlags); // Don't fire mutation events for nodes removed by the sanitizer. nsAutoScriptBlockerSuppressNodeRemoved scriptBlocker; - nsTreeSanitizer sanitizer(sanitizationFlags); + + nsTreeSanitizer sanitizer(nsIParserUtils::SanitizerAllowStyle | + nsIParserUtils::SanitizerAllowComments | + nsIParserUtils::SanitizerDropForms | + nsIParserUtils::SanitizerLogRemovals); sanitizer.Sanitize(fragment); ErrorResult error; aTargetNode->AppendChild(*fragment, error); rv = error.StealNSResult(); } return rv; @@ -4884,17 +4847,16 @@ nsresult nsContentUtils::ParseDocumentHT return rv; } /* static */ nsresult nsContentUtils::ParseFragmentXML(const nsAString& aSourceBuffer, Document* aDocument, nsTArray<nsString>& aTagStack, bool aPreventScriptExecution, - int32_t aFlags, DocumentFragment** aReturn) { AutoTimelineMarker m(aDocument->GetDocShell(), "Parse XML"); if (nsContentUtils::sFragmentParsingActive) { MOZ_ASSERT_UNREACHABLE("Re-entrant fragment parsing attempted."); return NS_ERROR_DOM_INVALID_STATE_ERR; } mozilla::AutoRestore<bool> guard(nsContentUtils::sFragmentParsingActive); @@ -4923,39 +4885,26 @@ nsresult nsContentUtils::ParseFragmentXM return rv; } rv = sXMLFragmentSink->FinishFragmentParsing(aReturn); sXMLFragmentParser->Reset(); NS_ENSURE_SUCCESS(rv, rv); - nsCOMPtr<nsIPrincipal> nodePrincipal = aDocument->NodePrincipal(); - -#ifdef DEBUG - // aFlags should always be -1 unless the caller of ParseFragmentXML - // is ParserUtils::ParseFragment() which is the only caller that intends - // sanitization. For all other callers we need to ensure to call - // AuditParsingOfHTMLXMLFragments. - if (aFlags < 0) { - DOMSecurityMonitor::AuditParsingOfHTMLXMLFragments(nodePrincipal, - aSourceBuffer); - } -#endif - - // We sanitize if the fragment occurs in a system privileged - // context or if there are explicit sanitization flags. - bool shouldSanitize = nodePrincipal->IsSystemPrincipal() || aFlags >= 0; - - if (shouldSanitize) { - uint32_t sanitizationFlags = - computeSanitizationFlags(nodePrincipal, aFlags); + // If this is a chrome-privileged document, sanitize the fragment before + // returning. + if (aDocument->NodePrincipal()->IsSystemPrincipal()) { // Don't fire mutation events for nodes removed by the sanitizer. nsAutoScriptBlockerSuppressNodeRemoved scriptBlocker; - nsTreeSanitizer sanitizer(sanitizationFlags); + + nsTreeSanitizer sanitizer(nsIParserUtils::SanitizerAllowStyle | + nsIParserUtils::SanitizerAllowComments | + nsIParserUtils::SanitizerDropForms | + nsIParserUtils::SanitizerLogRemovals); sanitizer.Sanitize(*aReturn); } return rv; } /* static */ nsresult nsContentUtils::ConvertToPlainText(const nsAString& aSourceBuffer,
--- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -1728,55 +1728,42 @@ class nsContentUtils { * @param aContextNamespace namespace of context node * @param aQuirks true to make <table> not close <p> * @param aPreventScriptExecution true to prevent scripts from executing; * don't set to false when parsing into a target node that has been * bound to tree. * @return NS_ERROR_DOM_INVALID_STATE_ERR if a re-entrant attempt to parse * fragments is made, NS_ERROR_OUT_OF_MEMORY if aSourceBuffer is too * long and NS_OK otherwise. - * @param aFlags defaults to -1 indicating that ParseFragmentHTML will do - * default sanitization for system privileged calls to it. Only - * ParserUtils::ParseFragment() should ever pass explicit aFlags - * which will then used for sanitization of the fragment. - * To pass explicit aFlags use any of the sanitization flags - * listed in nsIParserUtils.idl. */ static nsresult ParseFragmentHTML(const nsAString& aSourceBuffer, nsIContent* aTargetNode, nsAtom* aContextLocalName, int32_t aContextNamespace, bool aQuirks, - bool aPreventScriptExecution, - int32_t aFlags = -1); + bool aPreventScriptExecution); /** * Invoke the fragment parsing algorithm (innerHTML) using the XML parser. * * Please note that for safety reasons, if the node principal of aDocument * is the system principal, this function will automatically sanitize its * input using nsTreeSanitizer. * * @param aSourceBuffer the string being set as innerHTML * @param aDocument the target document * @param aTagStack the namespace mapping context * @param aPreventExecution whether to mark scripts as already started - * @param aFlags, pass -1 and ParseFragmentXML will do default - * sanitization for system privileged calls to it. Only - * ParserUtils::ParseFragment() should ever pass explicit aFlags - * which will then used for sanitization of the fragment. - * To pass explicit aFlags use any of the sanitization flags - * listed in nsIParserUtils.idl. * @param aReturn the result fragment * @return NS_ERROR_DOM_INVALID_STATE_ERR if a re-entrant attempt to parse * fragments is made, a return code from the XML parser. */ static nsresult ParseFragmentXML(const nsAString& aSourceBuffer, Document* aDocument, nsTArray<nsString>& aTagStack, - bool aPreventScriptExecution, int32_t aFlags, + bool aPreventScriptExecution, mozilla::dom::DocumentFragment** aReturn); /** * Parse a string into a document using the HTML parser. * Script elements are marked unexecutable. * * @param aSourceBuffer the string to parse as an HTML document * @param aTargetDocument the document object to parse into. Must not have
deleted file mode 100644 --- a/dom/security/DOMSecurityMonitor.cpp +++ /dev/null @@ -1,95 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* 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 "DOMSecurityMonitor.h" -#include "nsContentUtils.h" - -#include "nsIPrincipal.h" -#include "nsIURI.h" - -/* static */ -void DOMSecurityMonitor::AuditParsingOfHTMLXMLFragments( - nsIPrincipal* aPrincipal, const nsAString& aFragment) { - // if the fragment parser (e.g. innerHTML()) is not called in chrome: code - // or any of our about: pages, then there is nothing to do here. - if (!aPrincipal->IsSystemPrincipal() && !aPrincipal->SchemeIs("about")) { - return; - } - - // check if the fragment is empty, if so, we can return early. - if (aFragment.IsEmpty()) { - return; - } - - // check if there is a JS caller, if not, then we can can return early here - // because we only care about calls to the fragment parser (e.g. innerHTML) - // originating from JS code. - nsAutoString filename; - uint32_t lineNum = 0; - uint32_t columnNum = 0; - JSContext* cx = nsContentUtils::GetCurrentJSContext(); - if (!nsJSUtils::GetCallingLocation(cx, filename, &lineNum, &columnNum)) { - return; - } - - // check if we should skip assertion. Please only ever set this pref to - // true if really needed for testing purposes. - if (Preferences::GetBool("domsecurity.skip_html_fragment_assertion")) { - return; - } - - /* - * WARNING: Do not add any new entries to the htmlFragmentAllowlist - * withiout proper review from a dom:security peer! - */ - static nsLiteralCString htmlFragmentAllowlist[] = { - NS_LITERAL_CSTRING("chrome://browser/content/aboutNetError.js"), - NS_LITERAL_CSTRING( - "chrome://devtools-startup/content/aboutdevtools/aboutdevtools.js"), - NS_LITERAL_CSTRING( - "resource://activity-stream/data/content/activity-stream.bundle.js"), - NS_LITERAL_CSTRING("resource://devtools/client/debugger/src/components/" - "Editor/Breakpoint.js"), - NS_LITERAL_CSTRING("resource://devtools/client/debugger/src/components/" - "Editor/ColumnBreakpoint.js"), - NS_LITERAL_CSTRING( - "resource://devtools/client/shared/vendor/fluent-react.js"), - NS_LITERAL_CSTRING( - "resource://devtools/client/shared/widgets/FilterWidget.js"), - NS_LITERAL_CSTRING( - "resource://devtools/client/shared/widgets/Spectrum.js"), - NS_LITERAL_CSTRING("resource://gre/modules/narrate/VoiceSelect.jsm"), - NS_LITERAL_CSTRING("resource://normandy-vendor/ReactDOM.js"), - // ------------------------------------------------------------------ - // test pages - // ------------------------------------------------------------------ - NS_LITERAL_CSTRING("chrome://mochikit/content/harness.xhtml"), - NS_LITERAL_CSTRING("chrome://mochikit/content/tests/"), - NS_LITERAL_CSTRING("chrome://mochitests/content/"), - NS_LITERAL_CSTRING("chrome://reftest/content/"), - }; - - for (const nsLiteralCString& allowlistEntry : htmlFragmentAllowlist) { - if (StringBeginsWith(NS_ConvertUTF16toUTF8(filename), allowlistEntry)) { - return; - } - } - - nsAutoCString uriSpec; - aPrincipal->GetAsciiSpec(uriSpec); - - // Ideally we should not call the fragment parser (e.g. innerHTML()) in - // chrome: code or any of our about: pages. If you hit that assertion, - // please do *not* add your filename to the allowlist above, but rather - // refactor your code. - fprintf(stderr, - "Do not call the fragment parser (e.g innerHTML()) in chrome code " - "or in about: pages, (uri: %s), (caller: %s, line: %d, col: %d), " - "(fragment: %s)", - uriSpec.get(), NS_ConvertUTF16toUTF8(filename).get(), lineNum, - columnNum, NS_ConvertUTF16toUTF8(aFragment).get()); - MOZ_ASSERT(false); -}
deleted file mode 100644 --- a/dom/security/DOMSecurityMonitor.h +++ /dev/null @@ -1,31 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* 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/. */ - -#ifndef mozilla_dom_DOMSecurityMonitor_h -#define mozilla_dom_DOMSecurityMonitor_h - -class nsIPrincipal; - -class DOMSecurityMonitor final { - public: - /* The fragment parser is triggered anytime JS calls innerHTML or similar - * JS functions which can generate HTML fragments. This generation of - * HTML might be dangerous, hence we should ensure that no new instances - * of innerHTML and similar functions are introduced in system privileged - * contexts, or also about: pages, in our codebase. - * - * If the auditor detects a new instance of innerHTML or similar - * function it will CRASH using a strong assertion. - */ - static void AuditParsingOfHTMLXMLFragments(nsIPrincipal* aPrincipal, - const nsAString& aFragment); - - private: - DOMSecurityMonitor() = default; - ~DOMSecurityMonitor() = default; -}; - -#endif /* mozilla_dom_DOMSecurityMonitor_h */
--- a/dom/security/moz.build +++ b/dom/security/moz.build @@ -9,17 +9,16 @@ with Files('*'): TEST_DIRS += ['test'] DIRS += [ 'featurepolicy' ] EXPORTS.mozilla.dom += [ 'CSPEvalChecker.h', 'DOMSecurityManager.h', - 'DOMSecurityMonitor.h', 'FramingChecker.h', 'nsContentSecurityManager.h', 'nsContentSecurityUtils.h', 'nsCSPContext.h', 'nsCSPService.h', 'nsCSPUtils.h', 'nsMixedContentBlocker.h', 'PolicyTokenizer.h', @@ -34,17 +33,16 @@ EXPORTS += [ 'nsContentSecurityUtils.h', 'nsMixedContentBlocker.h', 'ReferrerInfo.h', ] UNIFIED_SOURCES += [ 'CSPEvalChecker.cpp', 'DOMSecurityManager.cpp', - 'DOMSecurityMonitor.cpp', 'FramingChecker.cpp', 'nsContentSecurityManager.cpp', 'nsContentSecurityUtils.cpp', 'nsCSPContext.cpp', 'nsCSPParser.cpp', 'nsCSPService.cpp', 'nsCSPUtils.cpp', 'nsMixedContentBlocker.cpp',
--- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -2280,20 +2280,16 @@ pref("security.notification_enable_delay #if defined(DEBUG) // For testing purposes only: Flipping this pref to true allows // to skip the assertion that every about page ships with a CSP. pref("csp.skip_about_page_has_csp_assert", false); // For testing purposes only: Flipping this pref to true allows // to skip the allowlist for about: pages and do not ship with a // CSP and NS_ASSERT right away. pref("csp.skip_about_page_csp_allowlist_and_assert", false); - // For testing purposes only: Flipping this pref to true allows - // to skip the assertion that HTML fragments (e.g. innerHTML) can - // not be used within chrome code or about: pages. - pref("domsecurity.skip_html_fragment_assertion", false); #endif #ifdef EARLY_BETA_OR_EARLIER // Disallow web documents loaded with the SystemPrincipal pref("security.disallow_non_local_systemprincipal_in_tests", false); #endif // Mixed content blocking
--- a/parser/html/nsParserUtils.cpp +++ b/parser/html/nsParserUtils.cpp @@ -96,22 +96,25 @@ nsParserUtils::ParseFragment(const nsASt // the fragment. nsresult rv = NS_OK; AutoTArray<nsString, 2> tagStack; RefPtr<DocumentFragment> fragment; if (aIsXML) { // XHTML tagStack.AppendElement(NS_LITERAL_STRING(XHTML_DIV_TAG)); rv = nsContentUtils::ParseFragmentXML(aFragment, document, tagStack, true, - aFlags, getter_AddRefs(fragment)); + getter_AddRefs(fragment)); } else { fragment = new DocumentFragment(document->NodeInfoManager()); rv = nsContentUtils::ParseFragmentHTML(aFragment, fragment, nsGkAtoms::body, - kNameSpaceID_XHTML, false, true, - aFlags); + kNameSpaceID_XHTML, false, true); + } + if (fragment) { + nsTreeSanitizer sanitizer(aFlags); + sanitizer.Sanitize(fragment); } if (scripts_enabled) { loader->SetEnabled(true); } fragment.forget(aReturn); return rv;