commit 8c3c7dcd7e71ae7ca9237fef555efb602ddc7bcc Author: Richard Pospesel <richard@torproject.org> Date: Thu Mar 1 14:12:08 2018 -0800 Bug 25147: Sanitize HTML fragments created for chrome-privileged documents Ported over firefox patch c2db4a50dc5c (Bug 1432966) --- .../tests/mochitest/events/test_mutation.html | 4 +- browser/base/content/browser-media.js | 59 ++++++------ .../customizableui/CustomizableWidgets.jsm | 2 +- browser/modules/webrtcUI.jsm | 21 +++-- .../client/responsive.html/components/browser.js | 7 ++ devtools/shared/gcli/source/lib/gcli/util/util.js | 6 +- .../tests/browser/browser_l10n_localizeMarkup.js | 4 +- dom/base/Element.cpp | 6 ++ dom/base/Element.h | 1 + dom/base/FragmentOrElement.cpp | 13 ++- dom/base/FragmentOrElement.h | 3 +- dom/base/nsContentUtils.cpp | 60 ++++++++++-- dom/base/nsContentUtils.h | 26 +++++- dom/base/nsDocument.cpp | 12 ++- dom/base/nsIDocument.h | 6 ++ dom/base/test/chrome.ini | 1 + dom/base/test/chrome/test_bug683852.xul | 4 +- dom/base/test/test_fragment_sanitization.xul | 101 +++++++++++++++++++++ dom/webidl/Document.webidl | 5 + dom/webidl/Element.webidl | 10 ++ layout/style/test/chrome/bug418986-2.js | 16 +++- mobile/android/chrome/content/config.js | 4 +- toolkit/content/tests/chrome/test_bug570192.xul | 4 +- toolkit/mozapps/extensions/content/extensions.js | 2 +- 24 files changed, 309 insertions(+), 68 deletions(-) diff --git a/accessible/tests/mochitest/events/test_mutation.html b/accessible/tests/mochitest/events/test_mutation.html index 232a0972777f..63dd74ca4775 100644 --- a/accessible/tests/mochitest/events/test_mutation.html +++ b/accessible/tests/mochitest/events/test_mutation.html @@ -348,8 +348,8 @@ this.invoke = function insertReferredElm_invoke() { - this.containerNode.innerHTML = - "<span id='insertReferredElms_span'></span><input aria-labelledby='insertReferredElms_span'>"; + this.containerNode.unsafeSetInnerHTML( + "<span id='insertReferredElms_span'></span><input aria-labelledby='insertReferredElms_span'>"); } this.getID = function insertReferredElm_getID() diff --git a/browser/base/content/browser-media.js b/browser/base/content/browser-media.js index 81e7faf17aad..c1ed8f9399fa 100644 --- a/browser/base/content/browser-media.js +++ b/browser/base/content/browser-media.js @@ -41,11 +41,23 @@ var gEMEHandler = { } return true; }, - getLearnMoreLink: function(msgId) { - let text = gNavigatorBundle.getString("emeNotifications." + msgId + ".learnMoreLabel"); + getEMEDisabledFragment(msgId) { + let mainMessage = gNavigatorBundle.getString("emeNotifications.drmContentDisabled.message"); + let [prefix, suffix] = mainMessage.split(/%(?:1\$)?S/).map(s => document.createTextNode(s)); + let text = gNavigatorBundle.getString("emeNotifications.drmContentDisabled.learnMoreLabel"); let baseURL = Services.urlFormatter.formatURLPref("app.support.baseURL"); - return "<label class='text-link' href='" + baseURL + "drm-content'>" + - text + "</label>"; + let link = document.createElement("label"); + link.className = "text-link"; + link.setAttribute("href", baseURL + "drm-content"); + link.textContent = text; + + let fragment = document.createDocumentFragment(); + [prefix, link, suffix].forEach(n => fragment.appendChild(n)); + return fragment; + }, + getMessageWithBrandName(notificationId) { + let msgId = "emeNotifications." + notificationId + ".message"; + return gNavigatorBundle.getFormattedString(msgId, [this._brandShortName]); }, receiveMessage: function({target: browser, data: data}) { let parsedData; @@ -63,7 +75,8 @@ var gEMEHandler = { let notificationId; let buttonCallback; - let params = []; + // Notification message can be either a string or a DOM fragment. + let notificationMessage; switch (status) { case "available": case "cdm-created": @@ -77,18 +90,18 @@ var gEMEHandler = { case "api-disabled": case "cdm-disabled": notificationId = "drmContentDisabled"; - buttonCallback = gEMEHandler.ensureEMEEnabled.bind(gEMEHandler, browser, keySystem) - params = [this.getLearnMoreLink(notificationId)]; + buttonCallback = gEMEHandler.ensureEMEEnabled.bind(gEMEHandler, browser, keySystem); + notificationMessage = this.getEMEDisabledFragment(); break; case "cdm-insufficient-version": notificationId = "drmContentCDMInsufficientVersion"; - params = [this._brandShortName]; + notificationMessage = this.getMessageWithBrandName(notificationId); break; case "cdm-not-installed": notificationId = "drmContentCDMInstalling"; - params = [this._brandShortName]; + notificationMessage = this.getMessageWithBrandName(notificationId); break; case "cdm-not-supported": @@ -100,43 +113,27 @@ var gEMEHandler = { return; } - this.showNotificationBar(browser, notificationId, keySystem, params, buttonCallback); - }, - showNotificationBar: function(browser, notificationId, keySystem, labelParams, callback) { + // Now actually create the notification + let box = gBrowser.getNotificationBox(browser); if (box.getNotificationWithValue(notificationId)) { return; } - let msgPrefix = "emeNotifications." + notificationId + "."; - let msgId = msgPrefix + "message"; - - let message = labelParams.length ? - gNavigatorBundle.getFormattedString(msgId, labelParams) : - gNavigatorBundle.getString(msgId); - let buttons = []; - if (callback) { + if (buttonCallback) { + let msgPrefix = "emeNotifications." + notificationId + "."; let btnLabelId = msgPrefix + "button.label"; let btnAccessKeyId = msgPrefix + "button.accesskey"; buttons.push({ label: gNavigatorBundle.getString(btnLabelId), accessKey: gNavigatorBundle.getString(btnAccessKeyId), - callback: callback + callback: buttonCallback, }); } let iconURL = "chrome://browser/skin/drm-icon.svg#chains-black"; - - // Do a little dance to get rich content into the notification: - let fragment = document.createDocumentFragment(); - let descriptionContainer = document.createElement("description"); - descriptionContainer.innerHTML = message; - while (descriptionContainer.childNodes.length) { - fragment.appendChild(descriptionContainer.childNodes[0]); - } - - box.appendNotification(fragment, notificationId, iconURL, box.PRIORITY_WARNING_MEDIUM, + box.appendNotification(notificationMessage, notificationId, iconURL, box.PRIORITY_WARNING_MEDIUM, buttons); }, showPopupNotificationForSuccess: function(browser, keySystem) { diff --git a/browser/components/customizableui/CustomizableWidgets.jsm b/browser/components/customizableui/CustomizableWidgets.jsm index 907e2e0f75b7..b18249a35811 100644 --- a/browser/components/customizableui/CustomizableWidgets.jsm +++ b/browser/components/customizableui/CustomizableWidgets.jsm @@ -327,7 +327,7 @@ const CustomizableWidgets = [ let promoParentElt = doc.getElementById("PanelUI-remotetabs-mobile-promo"); // Put it all together... let contents = bundle.getFormattedString("appMenuRemoteTabs.mobilePromo.text2", formatArgs); - promoParentElt.innerHTML = contents; + promoParentElt.unsafeSetInnerHTML(contents); // We manually manage the "click" event to open the promo links because // allowing the "text-link" widget handle it has 2 problems: (1) it only // supports button 0 and (2) it's tricky to intercept when it does the diff --git a/browser/modules/webrtcUI.jsm b/browser/modules/webrtcUI.jsm index b24135bfc959..ddcf09260f97 100644 --- a/browser/modules/webrtcUI.jsm +++ b/browser/modules/webrtcUI.jsm @@ -536,21 +536,26 @@ function prompt(aBrowser, aRequest) { bundle.getString("getUserMedia.shareScreen.learnMoreLabel"); let baseURL = Services.urlFormatter.formatURLPref("app.support.baseURL"); - let learnMore = - "<label class='text-link' href='" + baseURL + "screenshare-safety'>" + - learnMoreText + "</label>"; + + let learnMore = chromeWin.document.createElement("label"); + learnMore.className = "text-link"; + learnMore.setAttribute("href", baseURL + "screenshare-safety"); + learnMore.textContent = learnMoreText; if (type == "screen") { string = bundle.getFormattedString("getUserMedia.shareScreenWarning.message", - [learnMore]); - } - else { + ["<>"]); + } else { let brand = chromeDoc.getElementById("bundle_brand").getString("brandShortName"); string = bundle.getFormattedString("getUserMedia.shareFirefoxWarning.message", - [brand, learnMore]); + [brand, "<>"]); } - warning.innerHTML = string; + + let [pre, post] = string.split("<>"); + warning.textContent = pre; + warning.appendChild(learnMore); + warning.appendChild(chromeWin.document.createTextNode(post)); } let perms = Services.perms; diff --git a/devtools/client/responsive.html/components/browser.js b/devtools/client/responsive.html/components/browser.js index f2902905b522..91f709f0cb8a 100644 --- a/devtools/client/responsive.html/components/browser.js +++ b/devtools/client/responsive.html/components/browser.js @@ -16,6 +16,13 @@ const Types = require("../types"); const e10s = require("../utils/e10s"); const message = require("../utils/message"); +// Allow creation of HTML fragments without automatic sanitization, even +// though we're in a chrome-privileged document. +// This is, unfortunately, necessary in order to React to function +// correctly. + +document.allowUnsafeHTML = true; + module.exports = createClass({ /** diff --git a/devtools/shared/gcli/source/lib/gcli/util/util.js b/devtools/shared/gcli/source/lib/gcli/util/util.js index 065bf36c07db..cfc18062e1c5 100644 --- a/devtools/shared/gcli/source/lib/gcli/util/util.js +++ b/devtools/shared/gcli/source/lib/gcli/util/util.js @@ -498,7 +498,11 @@ exports.setContents = function(elem, contents) { return; } - if ('innerHTML' in elem) { + if ('unsafeSetInnerHTML' in elem) { + // FIXME: Stop relying on unsanitized HTML. + elem.unsafeSetInnerHTML(contents); + } + else if ('innerHTML' in elem) { elem.innerHTML = contents; } else { diff --git a/devtools/shared/tests/browser/browser_l10n_localizeMarkup.js b/devtools/shared/tests/browser/browser_l10n_localizeMarkup.js index f33a5a331c51..59bbb308ba44 100644 --- a/devtools/shared/tests/browser/browser_l10n_localizeMarkup.js +++ b/devtools/shared/tests/browser/browser_l10n_localizeMarkup.js @@ -18,7 +18,7 @@ add_task(function* () { info("Create the test markup"); let div = document.createElement("div"); - div.innerHTML = + div.unsafeSetInnerHTML( `<div data-localization-bundle="devtools/client/locales/startup.properties"> <div id="d0" data-localization="content=inspector.someInvalidKey"></div> <div id="d1" data-localization="content=inspector.label">Text will disappear</div> @@ -32,7 +32,7 @@ add_task(function* () { <div id="d5" data-localization="content=toolbox.defaultTitle"></div> </div> </div> - `; + `); info("Use localization helper to localize the test markup"); localizeMarkup(div); diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 09c6b39da574..daf5d6aa98bf 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -3564,6 +3564,12 @@ Element::SetInnerHTML(const nsAString& aInnerHTML, ErrorResult& aError) } void +Element::UnsafeSetInnerHTML(const nsAString& aInnerHTML, ErrorResult& aError) +{ + SetInnerHTMLInternal(aInnerHTML, aError, true); +} + +void Element::GetOuterHTML(nsAString& aOuterHTML) { GetMarkup(true, aOuterHTML); diff --git a/dom/base/Element.h b/dom/base/Element.h index 5d878df60668..94991b26a641 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -896,6 +896,7 @@ public: NS_IMETHOD GetInnerHTML(nsAString& aInnerHTML); virtual void SetInnerHTML(const nsAString& aInnerHTML, ErrorResult& aError); + void UnsafeSetInnerHTML(const nsAString& aInnerHTML, ErrorResult& aError); void GetOuterHTML(nsAString& aOuterHTML); void SetOuterHTML(const nsAString& aOuterHTML, ErrorResult& aError); void InsertAdjacentHTML(const nsAString& aPosition, const nsAString& aText, diff --git a/dom/base/FragmentOrElement.cpp b/dom/base/FragmentOrElement.cpp index 4ad10931a8d0..c83a76dc2603 100644 --- a/dom/base/FragmentOrElement.cpp +++ b/dom/base/FragmentOrElement.cpp @@ -2183,7 +2183,8 @@ ContainsMarkup(const nsAString& aStr) } void -FragmentOrElement::SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult& aError) +FragmentOrElement::SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult& aError, + bool aNeverSanitize) { FragmentOrElement* target = this; // Handle template case. @@ -2237,6 +2238,9 @@ FragmentOrElement::SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult contextNameSpaceID = shadowRoot->GetHost()->GetNameSpaceID(); } + auto sanitize = (aNeverSanitize ? nsContentUtils::NeverSanitize + : nsContentUtils::SanitizeSystemPrivileged); + if (doc->IsHTMLDocument()) { int32_t oldChildCount = target->GetChildCount(); aError = nsContentUtils::ParseFragmentHTML(aInnerHTML, @@ -2245,14 +2249,17 @@ FragmentOrElement::SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult contextNameSpaceID, doc->GetCompatibilityMode() == eCompatibility_NavQuirks, - true); + true, + sanitize); mb.NodesAdded(); // HTML5 parser has notified, but not fired mutation events. nsContentUtils::FireMutationEventsForDirectParsing(doc, target, oldChildCount); } else { RefPtr<DocumentFragment> df = - nsContentUtils::CreateContextualFragment(target, aInnerHTML, true, aError); + nsContentUtils::CreateContextualFragment(target, aInnerHTML, true, + sanitize, + aError); if (!aError.Failed()) { // Suppress assertion about node removal mutation events that can't have // listeners anyway, because no one has had the chance to register mutation diff --git a/dom/base/FragmentOrElement.h b/dom/base/FragmentOrElement.h index 3cb5575fe063..c6ed7ab85e41 100644 --- a/dom/base/FragmentOrElement.h +++ b/dom/base/FragmentOrElement.h @@ -357,7 +357,8 @@ public: protected: void GetMarkup(bool aIncludeSelf, nsAString& aMarkup); - void SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult& aError); + void SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult& aError, + bool aNeverSanitize = false); // Override from nsINode virtual nsINode::nsSlots* CreateSlots() override; diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index f93274280f7c..182a1686d5fe 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -155,6 +155,7 @@ #include "nsIObserverService.h" #include "nsIOfflineCacheUpdate.h" #include "nsIParser.h" +#include "nsIParserUtils.h" #include "nsIParserService.h" #include "nsIPermissionManager.h" #include "nsIPluginHost.h" @@ -195,6 +196,7 @@ #include "nsTextFragment.h" #include "nsTextNode.h" #include "nsThreadUtils.h" +#include "nsTreeSanitizer.h" #include "nsUnicharUtilCIID.h" #include "nsUnicodeProperties.h" #include "nsViewManager.h" @@ -4576,6 +4578,7 @@ already_AddRefed<DocumentFragment> nsContentUtils::CreateContextualFragment(nsINode* aContextNode, const nsAString& aFragment, bool aPreventScriptExecution, + SanitizeFragments aSanitize, ErrorResult& aRv) { if (!aContextNode) { @@ -4611,14 +4614,16 @@ nsContentUtils::CreateContextualFragment(nsINode* aContextNode, contextAsContent->GetNameSpaceID(), (document->GetCompatibilityMode() == eCompatibility_NavQuirks), - aPreventScriptExecution); + aPreventScriptExecution, + aSanitize); } else { aRv = ParseFragmentHTML(aFragment, frag, nsGkAtoms::body, kNameSpaceID_XHTML, (document->GetCompatibilityMode() == eCompatibility_NavQuirks), - aPreventScriptExecution); + aPreventScriptExecution, + aSanitize); } return frag.forget(); @@ -4682,7 +4687,8 @@ nsContentUtils::CreateContextualFragment(nsINode* aContextNode, nsCOMPtr<nsIDOMDocumentFragment> frag; aRv = ParseFragmentXML(aFragment, document, tagStack, - aPreventScriptExecution, getter_AddRefs(frag)); + aPreventScriptExecution, getter_AddRefs(frag), + aSanitize); return frag.forget().downcast<DocumentFragment>(); } @@ -4709,7 +4715,8 @@ nsContentUtils::ParseFragmentHTML(const nsAString& aSourceBuffer, nsIAtom* aContextLocalName, int32_t aContextNamespace, bool aQuirks, - bool aPreventScriptExecution) + bool aPreventScriptExecution, + SanitizeFragments aSanitize) { AutoTimelineMarker m(aTargetNode->OwnerDoc()->GetDocShell(), "Parse HTML"); @@ -4723,13 +4730,39 @@ nsContentUtils::ParseFragmentHTML(const nsAString& aSourceBuffer, NS_ADDREF(sHTMLFragmentParser = new nsHtml5StringParser()); // Now sHTMLFragmentParser owns the object } + + nsIContent* target = aTargetNode; + + // If this is a chrome-privileged document, create a fragment first, and + // sanitize it before insertion. + RefPtr<DocumentFragment> fragment; + if (aSanitize != NeverSanitize && !aTargetNode->OwnerDoc()->AllowUnsafeHTML()) { + fragment = new DocumentFragment(aTargetNode->OwnerDoc()->NodeInfoManager()); + target = fragment; + } + nsresult rv = sHTMLFragmentParser->ParseFragment(aSourceBuffer, - aTargetNode, + target, aContextLocalName, aContextNamespace, aQuirks, aPreventScriptExecution); + NS_ENSURE_SUCCESS(rv, rv); + + if (fragment) { + // Don't fire mutation events for nodes removed by the sanitizer. + nsAutoScriptBlockerSuppressNodeRemoved scriptBlocker; + + nsTreeSanitizer sanitizer(nsIParserUtils::SanitizerAllowStyle | + nsIParserUtils::SanitizerAllowComments); + sanitizer.Sanitize(fragment); + + ErrorResult error; + aTargetNode->AppendChild(*fragment, error); + rv = error.StealNSResult(); + } + return rv; } @@ -4764,7 +4797,8 @@ nsContentUtils::ParseFragmentXML(const nsAString& aSourceBuffer, nsIDocument* aDocument, nsTArray<nsString>& aTagStack, bool aPreventScriptExecution, - nsIDOMDocumentFragment** aReturn) + nsIDOMDocumentFragment** aReturn, + SanitizeFragments aSanitize) { AutoTimelineMarker m(aDocument->GetDocShell(), "Parse XML"); @@ -4803,6 +4837,20 @@ nsContentUtils::ParseFragmentXML(const nsAString& aSourceBuffer, rv = sXMLFragmentSink->FinishFragmentParsing(aReturn); sXMLFragmentParser->Reset(); + NS_ENSURE_SUCCESS(rv, rv); + + // If this is a chrome-privileged document, sanitize the fragment before + // returning. + if (aSanitize != NeverSanitize && !aDocument->AllowUnsafeHTML()) { + // Don't fire mutation events for nodes removed by the sanitizer. + nsAutoScriptBlockerSuppressNodeRemoved scriptBlocker; + + RefPtr<DocumentFragment> fragment = static_cast<DocumentFragment*>(*aReturn); + + nsTreeSanitizer sanitizer(nsIParserUtils::SanitizerAllowStyle | + nsIParserUtils::SanitizerAllowComments); + sanitizer.Sanitize(fragment); + } return rv; } diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 08c5cbf02e25..f2de4a16be5d 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -1344,6 +1344,11 @@ public: static bool IsValidNodeName(nsIAtom *aLocalName, nsIAtom *aPrefix, int32_t aNamespaceID); + enum SanitizeFragments { + SanitizeSystemPrivileged, + NeverSanitize, + }; + /** * Creates a DocumentFragment from text using a context node to resolve * namespaces. @@ -1357,6 +1362,8 @@ public: * @param aFragment the string which is parsed to a DocumentFragment * @param aReturn the resulting fragment * @param aPreventScriptExecution whether to mark scripts as already started + * @param aSanitize whether the fragment should be sanitized prior to + * injection */ static nsresult CreateContextualFragment(nsINode* aContextNode, const nsAString& aFragment, @@ -1365,7 +1372,16 @@ public: static already_AddRefed<mozilla::dom::DocumentFragment> CreateContextualFragment(nsINode* aContextNode, const nsAString& aFragment, bool aPreventScriptExecution, + SanitizeFragments aSanitize, mozilla::ErrorResult& aRv); + static already_AddRefed<mozilla::dom::DocumentFragment> + CreateContextualFragment(nsINode* aContextNode, const nsAString& aFragment, + bool aPreventScriptExecution, + mozilla::ErrorResult& aRv) + { + return CreateContextualFragment(aContextNode, aFragment, aPreventScriptExecution, + SanitizeSystemPrivileged, aRv); + } /** * Invoke the fragment parsing algorithm (innerHTML) using the HTML parser. @@ -1378,6 +1394,8 @@ public: * @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. + * @param aSanitize whether the fragment should be sanitized prior to + * injection * @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. @@ -1387,7 +1405,8 @@ public: nsIAtom* aContextLocalName, int32_t aContextNamespace, bool aQuirks, - bool aPreventScriptExecution); + bool aPreventScriptExecution, + SanitizeFragments aSanitize = SanitizeSystemPrivileged); /** * Invoke the fragment parsing algorithm (innerHTML) using the XML parser. @@ -1397,6 +1416,8 @@ public: * @param aTagStack the namespace mapping context * @param aPreventExecution whether to mark scripts as already started * @param aReturn the result fragment + * @param aSanitize whether the fragment should be sanitized prior to + * injection * @return NS_ERROR_DOM_INVALID_STATE_ERR if a re-entrant attempt to parse * fragments is made, a return code from the XML parser. */ @@ -1404,7 +1425,8 @@ public: nsIDocument* aDocument, nsTArray<nsString>& aTagStack, bool aPreventScriptExecution, - nsIDOMDocumentFragment** aReturn); + nsIDOMDocumentFragment** aReturn, + SanitizeFragments aSanitize = SanitizeSystemPrivileged); /** * Parse a string into a document using the HTML parser. diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 25e5d28d6c76..ba35493ad886 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -1315,7 +1315,8 @@ nsIDocument::nsIDocument() mFrameRequestCallbacksScheduled(false), mBidiOptions(IBMBIDI_DEFAULT_BIDI_OPTIONS), mPartID(0), - mUserHasInteracted(false) + mUserHasInteracted(false), + mAllowUnsafeHTML(false) { SetIsInDocument(); @@ -2659,7 +2660,7 @@ nsDocument::InitCSP(nsIChannel* aChannel) (cspSandboxFlags & SANDBOXED_ORIGIN) && !(mSandboxFlags & SANDBOXED_ORIGIN); mSandboxFlags |= cspSandboxFlags; - + if (needNewNullPrincipal) { principal = nsNullPrincipal::CreateWithInheritedAttributes(principal); principal->SetCsp(csp); @@ -5734,6 +5735,13 @@ nsDocument::CustomElementConstructor(JSContext* aCx, unsigned aArgc, JS::Value* } bool +nsIDocument::AllowUnsafeHTML() const +{ + return (!nsContentUtils::IsSystemPrincipal(NodePrincipal()) || + mAllowUnsafeHTML); +} + +bool nsDocument::IsWebComponentsEnabled(JSContext* aCx, JSObject* aObject) { JS::Rooted<JSObject*> obj(aCx, aObject); diff --git a/dom/base/nsIDocument.h b/dom/base/nsIDocument.h index 5715fd233477..3ff69a6e8e1d 100644 --- a/dom/base/nsIDocument.h +++ b/dom/base/nsIDocument.h @@ -2660,6 +2660,8 @@ public: CreateAttributeNS(const nsAString& aNamespaceURI, const nsAString& aQualifiedName, mozilla::ErrorResult& rv); + void SetAllowUnsafeHTML(bool aAllow) { mAllowUnsafeHTML = aAllow; } + bool AllowUnsafeHTML() const; void GetInputEncoding(nsAString& aInputEncoding) const; already_AddRefed<mozilla::dom::Location> GetLocation() const; void GetReferrer(nsAString& aReferrer) const; @@ -3206,6 +3208,10 @@ protected: // UpdateFrameRequestCallbackSchedulingState. bool mFrameRequestCallbacksScheduled : 1; + // True if unsafe HTML fragments should be allowed in chrome-privileged + // documents. + bool mAllowUnsafeHTML : 1; + enum Type { eUnknown, // should never be used eHTML, diff --git a/dom/base/test/chrome.ini b/dom/base/test/chrome.ini index f7e67ef6b9cb..8d4a9cd09a9c 100644 --- a/dom/base/test/chrome.ini +++ b/dom/base/test/chrome.ini @@ -16,6 +16,7 @@ support-files = [test_copypaste.xul] subsuite = clipboard [test_domrequesthelper.xul] +[test_fragment_sanitization.xul] [test_messagemanager_principal.html] [test_messagemanager_send_principal.html] skip-if = buildapp == 'mulet' diff --git a/dom/base/test/chrome/test_bug683852.xul b/dom/base/test/chrome/test_bug683852.xul index cebc8f3583bc..e7e106f7a483 100644 --- a/dom/base/test/chrome/test_bug683852.xul +++ b/dom/base/test/chrome/test_bug683852.xul @@ -20,6 +20,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=683852 /** Test for Bug 683852 **/ SimpleTest.waitForExplicitFinish(); + const NS_HTML = "http://www.w3.org/1999/xhtml"; + function startTest() { is(document.contains(document), true, "Document should contain itself!"); @@ -48,7 +50,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=683852 document.documentElement.appendChild(pi); document.contains(pi, true, "Document should contain processing instruction"); - var df = document.createRange().createContextualFragment("<div>foo</div>"); + var df = document.createRange().createContextualFragment(`<div xmlns="${NS_HTML}">foo</div>`); is(df.contains(df.firstChild), true, "Document fragment should contain its child"); is(df.contains(df.firstChild.firstChild), true, "Document fragment should contain its descendant"); diff --git a/dom/base/test/test_fragment_sanitization.xul b/dom/base/test/test_fragment_sanitization.xul new file mode 100644 index 000000000000..0c91b210125c --- /dev/null +++ b/dom/base/test/test_fragment_sanitization.xul @@ -0,0 +1,101 @@ +<?xml version="1.0"?> +<?xml-stylesheet type="text/css" href="chrome://global/skin"?> +<?xml-stylesheet type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"?> +<!-- +https://bugzilla.mozilla.org/show_bug.cgi?id=1432966 +--> +<window title="Mozilla Bug 1432966" + xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> + <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/> + <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SpawnTask.js"/> + + <script type="application/javascript"><![CDATA[ + +var { classes: Cc, interfaces: Ci } = Components; + +const NS_HTML = "http://www.w3.org/1999/xhtml"; + +function awaitLoad(frame) { + return new Promise(resolve => { + frame.addEventListener("load", resolve, {once: true}); + }); +} + +async function testFrame(frame, html, expected = html) { + document.querySelector("body").appendChild(frame); + await awaitLoad(frame); + + // Remove the xmlns attributes that will be automatically added when we're + // in an XML document, and break the comparison. + function unNS(text) { + return text.replace(RegExp(` xmlns="${NS_HTML}"`, "g"), ""); + } + + let doc = frame.contentDocument; + let body = doc.body || doc.documentElement; + + let div = doc.createElementNS(NS_HTML, "div"); + body.appendChild(div); + + div.innerHTML = html; + is(unNS(div.innerHTML), expected, "innerHTML value"); + + div.innerHTML = "<div></div>"; + div.firstChild.outerHTML = html; + is(unNS(div.innerHTML), expected, "outerHTML value"); + + div.textContent = ""; + div.insertAdjacentHTML("beforeend", html); + is(unNS(div.innerHTML), expected, "insertAdjacentHTML('beforeend') value"); + + div.innerHTML = "<a>foo</a>"; + div.firstChild.insertAdjacentHTML("afterend", html); + is(unNS(div.innerHTML), "<a>foo</a>" + expected, "insertAdjacentHTML('afterend') value"); + + frame.remove(); +} + +add_task(async function test_fragment_sanitization() { + const XUL_URL = "chrome://global/content/win.xul"; + const HTML_URL = "chrome://mochitests/content/chrome/dom/base/test/file_empty.html"; + + const HTML = '<a onclick="foo()" href="javascript:foo"><script>bar()<\/script>Meh.</a><a href="http://foo/"></a>'; + const SANITIZED = '<a>Meh.</a><a href="http://foo/"></a>'; + + info("Test content HTML document"); + { + let frame = document.createElementNS(NS_HTML, "iframe"); + frame.src = "http://example.com/"; + + await testFrame(frame, HTML); + } + + info("Test chrome HTML document"); + { + let frame = document.createElementNS(NS_HTML, "iframe"); + frame.src = HTML_URL; + + await testFrame(frame, HTML, SANITIZED); + } + + info("Test chrome XUL document"); + { + let frame = document.createElementNS(NS_HTML, "iframe"); + frame.src = XUL_URL; + + await testFrame(frame, HTML, SANITIZED); + } +}); + + ]]></script> + + <description style="-moz-user-focus: normal; -moz-user-select: text;"><![CDATA[ + hello + world + ]]></description> + + <body xmlns="http://www.w3.org/1999/xhtml"> + <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1432966" + target="_blank">Mozilla Bug 1432966</a> + </body> +</window> diff --git a/dom/webidl/Document.webidl b/dom/webidl/Document.webidl index c895fad39275..ed071cbbd375 100644 --- a/dom/webidl/Document.webidl +++ b/dom/webidl/Document.webidl @@ -95,6 +95,11 @@ interface Document : Node { Attr createAttribute(DOMString name); [NewObject, Throws] Attr createAttributeNS(DOMString? namespace, DOMString name); + + // Allows setting innerHTML without automatic sanitization. + // Do not use this. + [ChromeOnly] + attribute boolean allowUnsafeHTML; }; // http://www.whatwg.org/specs/web-apps/current-work/#the-document-object diff --git a/dom/webidl/Element.webidl b/dom/webidl/Element.webidl index ca5f1b35cd78..a66327ff96cd 100644 --- a/dom/webidl/Element.webidl +++ b/dom/webidl/Element.webidl @@ -216,6 +216,16 @@ partial interface Element { attribute DOMString outerHTML; [Throws] void insertAdjacentHTML(DOMString position, DOMString text); + + /** + * Like the innerHTML setter, but does not sanitize its values, even in + * chrome-privileged documents. + * + * If you're thinking about using this, don't. You have many, much better + * options. + */ + [ChromeOnly, Throws] + void unsafeSetInnerHTML(DOMString html); }; // http://www.w3.org/TR/selectors-api/#interface-definitions diff --git a/layout/style/test/chrome/bug418986-2.js b/layout/style/test/chrome/bug418986-2.js index 4336f4abdb2c..2fcad6d70e0b 100644 --- a/layout/style/test/chrome/bug418986-2.js +++ b/layout/style/test/chrome/bug418986-2.js @@ -222,13 +222,23 @@ var green = (function () { return getComputedStyle(temp).backgroundColor; })(); +// Injected HTML will automatically be sanitized when we're in a chrome +// document unless we use `unsafeSetInnerHTML`. That function doesn't +// exist in non-chrome documents, so add a stub to allow the same code +// to run in both. +if (!Element.prototype.unsafeSetInnerHTML) { + Element.prototype.unsafeSetInnerHTML = html => { + this.innerHTML = html; + }; +} + // __testCSS(resisting)__. // Creates a series of divs and CSS using media queries to set their // background color. If all media queries match as expected, then // all divs should have a green background color. var testCSS = function (resisting) { - document.getElementById("display").innerHTML = generateHtmlLines(resisting); - document.getElementById("test-css").innerHTML = generateCSSLines(resisting); + document.getElementById("display").unsafeSetInnerHTML(generateHtmlLines(resisting)); + document.getElementById("test-css").unsafeSetInnerHTML(generateCSSLines(resisting)); let cssTestDivs = document.querySelectorAll(".spoof,.suppress"); for (let div of cssTestDivs) { let color = window.getComputedStyle(div).backgroundColor; @@ -272,7 +282,7 @@ var testMediaQueriesInPictureElements = function* (resisting) { lines += "</picture><br/>\n"; } } - document.getElementById("pictures").innerHTML = lines; + document.getElementById("pictures").unsafeSetInnerHTML(lines); var testImages = document.getElementsByClassName("testImage"); yield sleep(0); for (let testImage of testImages) { diff --git a/mobile/android/chrome/content/config.js b/mobile/android/chrome/content/config.js index 2c868f175ee8..83f93a51c86a 100644 --- a/mobile/android/chrome/content/config.js +++ b/mobile/android/chrome/content/config.js @@ -599,7 +599,7 @@ Pref.prototype = { this.li.setAttribute("contextmenu", "prefs-context-menu"); // Create list item outline, bind to object actions - this.li.innerHTML = + this.li.unsafeSetInnerHTML( "<div class='pref-name' " + "onclick='AboutConfig.selectOrToggleBoolPref(event);'>" + this.name + @@ -623,7 +623,7 @@ Pref.prototype = { "<div class='pref-button down' " + "onclick='AboutConfig.incrOrDecrIntPref(event, -1);'>" + "</div>" + - "</div>"; + "</div>"); // Delay providing the list item values, until the LI is returned and added to the document setTimeout(this._valueSetup.bind(this), INNERHTML_VALUE_DELAY); diff --git a/toolkit/content/tests/chrome/test_bug570192.xul b/toolkit/content/tests/chrome/test_bug570192.xul index 09f73e932b5d..91c55805c9da 100644 --- a/toolkit/content/tests/chrome/test_bug570192.xul +++ b/toolkit/content/tests/chrome/test_bug570192.xul @@ -36,8 +36,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=570192 addLoadEvent(function() { try { var content = document.getElementById("content"); - content.innerHTML = '<textbox newlines="pasteintact" ' + - 'xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"/>'; + content.unsafeSetInnerHTML('<textbox newlines="pasteintact" ' + + 'xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"/>'); var textbox = content.firstChild; ok(textbox, "created the textbox"); ok(!textbox.editor, "do we have an editor?"); diff --git a/toolkit/mozapps/extensions/content/extensions.js b/toolkit/mozapps/extensions/content/extensions.js index 1e5f33b0a251..771008ed3bf8 100644 --- a/toolkit/mozapps/extensions/content/extensions.js +++ b/toolkit/mozapps/extensions/content/extensions.js @@ -3078,7 +3078,7 @@ var gDetailView = { // plugins without having bug 624602 fixed yet, and intentionally ignores // localisation. if (aAddon.isGMPlugin) { - fullDesc.innerHTML = aAddon.fullDescription; + fullDesc.unsafeSetInnerHTML(aAddon.fullDescription); } else { fullDesc.textContent = aAddon.fullDescription; }