Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-20 08:45:46 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1444394 - Remove Element::UnsafeSetInnerHTML. r=bz,kmag

Browse Source 
The last remaining user is already turned off and being removed
in bug 1444395 so that we can finally remove this unsafe code and
sleep a little better knowing that XSS through markup injections
will be impossible in chrome contexts.

MozReview-Commit-ID: KcZq8fRPiD4

--HG--
extra : rebase_source : 5def3abb50ed8f1b43e17072088e38a44394488b
This commit is contained in:
Johann Hofmann 2018-05-28 22:55:52 +02:00
parent 0fe264f656
commit ded334a8cb
7 changed files with 26 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
dom/base/Element.cpp
View File

@ -3782,12 +3782,6 @@ Element::SetInnerHTML(const nsAString& aInnerHTML, nsIPrincipal* aSubjectPrincip
SetInnerHTMLInternal(aInnerHTML, aError);
}
void
Element::UnsafeSetInnerHTML(const nsAString& aInnerHTML, ErrorResult& aError)
{
SetInnerHTMLInternal(aInnerHTML, aError, true);
}
void
Element::GetOuterHTML(nsAString& aOuterHTML)
{

1
dom/base/Element.h
View File

@ -1369,7 +1369,6 @@ public:
virtual void GetInnerHTML(nsAString& aInnerHTML, OOMReporter& aError);
virtual void SetInnerHTML(const nsAString& aInnerHTML, nsIPrincipal* aSubjectPrincipal, 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,

11
dom/base/FragmentOrElement.cpp
View File

@ -2276,8 +2276,7 @@ ContainsMarkup(const nsAString& aStr)
}
void
FragmentOrElement::SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult& aError,
bool aNeverSanitize)
FragmentOrElement::SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult& aError)
{
FragmentOrElement* target = this;
// Handle template case.
@ -2329,9 +2328,6 @@ 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,
@ -2340,8 +2336,7 @@ FragmentOrElement::SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult
contextNameSpaceID,
doc->GetCompatibilityMode() ==
eCompatibility_NavQuirks,
true,
sanitize);
true);
mb.NodesAdded();
// HTML5 parser has notified, but not fired mutation events.
nsContentUtils::FireMutationEventsForDirectParsing(doc, target,
@ -2349,7 +2344,7 @@ FragmentOrElement::SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult
} else {
RefPtr<DocumentFragment> df =
nsContentUtils::CreateContextualFragment(target, aInnerHTML, true,
sanitize, aError);
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

3
dom/base/FragmentOrElement.h
View File

@ -268,8 +268,7 @@ public:
protected:
void GetMarkup(bool aIncludeSelf, nsAString& aMarkup);
void SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult& aError,
bool aNeverSanitize = false);
void SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult& aError);
// Override from nsINode
nsIContent::nsContentSlots* CreateSlots() override

22
dom/base/nsContentUtils.cpp
View File

@ -4906,7 +4906,6 @@ already_AddRefed<DocumentFragment>
nsContentUtils::CreateContextualFragment(nsINode* aContextNode,
const nsAString& aFragment,
bool aPreventScriptExecution,
SanitizeFragments aSanitize,
ErrorResult& aRv)
{
if (!aContextNode) {
@ -4942,16 +4941,14 @@ nsContentUtils::CreateContextualFragment(nsINode* aContextNode,
contextAsContent->GetNameSpaceID(),
(document->GetCompatibilityMode() ==
eCompatibility_NavQuirks),
aPreventScriptExecution,
aSanitize);
aPreventScriptExecution);
} else {
aRv = ParseFragmentHTML(aFragment, frag,
nsGkAtoms::body,
kNameSpaceID_XHTML,
(document->GetCompatibilityMode() ==
eCompatibility_NavQuirks),
aPreventScriptExecution,
aSanitize);
aPreventScriptExecution);
}
return frag.forget();
@ -5015,8 +5012,7 @@ nsContentUtils::CreateContextualFragment(nsINode* aContextNode,
RefPtr<DocumentFragment> frag;
aRv = ParseFragmentXML(aFragment, document, tagStack,
aPreventScriptExecution, getter_AddRefs(frag),
aSanitize);
aPreventScriptExecution, getter_AddRefs(frag));
return frag.forget();
}
@ -5043,8 +5039,7 @@ nsContentUtils::ParseFragmentHTML(const nsAString& aSourceBuffer,
nsAtom* aContextLocalName,
int32_t aContextNamespace,
bool aQuirks,
bool aPreventScriptExecution,
SanitizeFragments aSanitize)
bool aPreventScriptExecution)
{
AutoTimelineMarker m(aTargetNode->OwnerDoc()->GetDocShell(), "Parse HTML");
@ -5064,8 +5059,7 @@ nsContentUtils::ParseFragmentHTML(const nsAString& aSourceBuffer,
// If this is a chrome-privileged document, create a fragment first, and
// sanitize it before insertion.
RefPtr<DocumentFragment> fragment;
if (aSanitize != NeverSanitize &&
IsSystemPrincipal(aTargetNode->NodePrincipal())) {
if (IsSystemPrincipal(aTargetNode->NodePrincipal())) {
fragment = new DocumentFragment(aTargetNode->OwnerDoc()->NodeInfoManager());
target = fragment;
}
@ -5128,8 +5122,7 @@ nsContentUtils::ParseFragmentXML(const nsAString& aSourceBuffer,
nsIDocument* aDocument,
nsTArray<nsString>& aTagStack,
bool aPreventScriptExecution,
DocumentFragment** aReturn,
SanitizeFragments aSanitize)
DocumentFragment** aReturn)
{
AutoTimelineMarker m(aDocument->GetDocShell(), "Parse XML");
@ -5172,8 +5165,7 @@ nsContentUtils::ParseFragmentXML(const nsAString& aSourceBuffer,
// If this is a chrome-privileged document, sanitize the fragment before
// returning.
if (aSanitize != NeverSanitize &&
IsSystemPrincipal(aDocument->NodePrincipal())) {
if (IsSystemPrincipal(aDocument->NodePrincipal())) {
// Don't fire mutation events for nodes removed by the sanitizer.
nsAutoScriptBlockerSuppressNodeRemoved scriptBlocker;

40
dom/base/nsContentUtils.h
View File

@ -1593,15 +1593,14 @@ public:
static bool IsValidNodeName(nsAtom *aLocalName, nsAtom *aPrefix,
int32_t aNamespaceID);
enum SanitizeFragments {
SanitizeSystemPrivileged,
NeverSanitize,
};
/**
* Creates a DocumentFragment from text using a context node to resolve
* namespaces.
*
* Please note that for safety reasons, if the node principal of
* aContextNode is the system principal, this function will automatically
* sanitize its input using nsTreeSanitizer.
*
* Note! In the HTML case with the HTML5 parser enabled, this is only called
* from Range.createContextualFragment() and the implementation here is
* quirky accordingly (html context node behaves like a body context node).
@ -1611,26 +1610,19 @@ 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 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.
*
* Please note that for safety reasons, if the node principal of aTargetNode
* is the system principal, this function will automatically sanitize its
* input using nsTreeSanitizer.
*
* @param aSourceBuffer the string being set as innerHTML
* @param aTargetNode the target container
* @param aContextLocalName local name of context node
@ -1639,8 +1631,6 @@ 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.
@ -1650,19 +1640,20 @@ public:
nsAtom* aContextLocalName,
int32_t aContextNamespace,
bool aQuirks,
bool aPreventScriptExecution,
SanitizeFragments aSanitize = SanitizeSystemPrivileged);
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 aTargetNode the target container
* @param aDocument the target document
* @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.
*/
@ -1670,8 +1661,7 @@ public:
nsIDocument* aDocument,
nsTArray<nsString>& aTagStack,
bool aPreventScriptExecution,
mozilla::dom::DocumentFragment** aReturn,
SanitizeFragments aSanitize = SanitizeSystemPrivileged);
mozilla::dom::DocumentFragment** aReturn);
/**
* Parse a string into a document using the HTML parser.

10
dom/webidl/Element.webidl
View File

@ -236,16 +236,6 @@ partial interface Element {
attribute DOMString outerHTML;
[CEReactions, 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