From dd94e423493662e92055c4e8a5bfef022e2b0739 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Wed, 5 Sep 2018 11:59:58 +0200 Subject: [Backport] CVE-2018-16066 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do not crash while reentrantly appending to style element. When a node is inserted into a container, it is notified via ::InsertedInto. However, a node may request a second notification via DidNotifySubtreeInsertionsToDocument, which occurs after all the children have been notified as well. *StyleElement is currently using this second notification. This causes a problem, because *ScriptElement is using the same mechanism, which in turn means that scripts can execute before the state of *StyleElements are properly updated. This patch avoids ::DidNotifySubtreeInsertionsToDocument, and instead processes the stylesheet in ::InsertedInto. The original reason for using ::DidNotifySubtreeInsertionsToDocument in the first place appears to be invalid now, as the test case is still passing. R=futhark@chromium.org, hayato@chromium.org Bug: 853709, 847570 Reviewed-on: https://chromium-review.googlesource.com/1104347 Change-Id: I1f8b3397f970c690d0f769788dbaa84136206816 Reviewed-by: Michael Brüning --- .../WebKit/Source/core/html/HTMLStyleElement.cpp | 16 ++++++++-------- .../WebKit/Source/core/html/HTMLStyleElement.h | 1 - .../WebKit/Source/core/svg/SVGStyleElement.cpp | 16 ++++++++-------- .../third_party/WebKit/Source/core/svg/SVGStyleElement.h | 1 - 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/chromium/third_party/WebKit/Source/core/html/HTMLStyleElement.cpp b/chromium/third_party/WebKit/Source/core/html/HTMLStyleElement.cpp index c6ef4ece6f6..39b314a4c3d 100644 --- a/chromium/third_party/WebKit/Source/core/html/HTMLStyleElement.cpp +++ b/chromium/third_party/WebKit/Source/core/html/HTMLStyleElement.cpp @@ -77,7 +77,14 @@ void HTMLStyleElement::FinishParsingChildren() { Node::InsertionNotificationRequest HTMLStyleElement::InsertedInto( ContainerNode* insertion_point) { HTMLElement::InsertedInto(insertion_point); - return kInsertionShouldCallDidNotifySubtreeInsertions; + if (isConnected()) { + if (StyleElement::ProcessStyleSheet(GetDocument(), *this) == + StyleElement::kProcessingFatalError) { + NotifyLoadedSheetAndAllCriticalSubresources( + kErrorOccurredLoadingSubresource); + } + } + return kInsertionDone; } void HTMLStyleElement::RemovedFrom(ContainerNode* insertion_point) { @@ -85,13 +92,6 @@ void HTMLStyleElement::RemovedFrom(ContainerNode* insertion_point) { StyleElement::RemovedFrom(*this, insertion_point); } -void HTMLStyleElement::DidNotifySubtreeInsertionsToDocument() { - if (StyleElement::ProcessStyleSheet(GetDocument(), *this) == - StyleElement::kProcessingFatalError) - NotifyLoadedSheetAndAllCriticalSubresources( - kErrorOccurredLoadingSubresource); -} - void HTMLStyleElement::ChildrenChanged(const ChildrenChange& change) { HTMLElement::ChildrenChanged(change); if (StyleElement::ChildrenChanged(*this) == diff --git a/chromium/third_party/WebKit/Source/core/html/HTMLStyleElement.h b/chromium/third_party/WebKit/Source/core/html/HTMLStyleElement.h index cea34b8db74..b420f03a15d 100644 --- a/chromium/third_party/WebKit/Source/core/html/HTMLStyleElement.h +++ b/chromium/third_party/WebKit/Source/core/html/HTMLStyleElement.h @@ -56,7 +56,6 @@ class CORE_EXPORT HTMLStyleElement final : public HTMLElement, // overload from HTMLElement void ParseAttribute(const AttributeModificationParams&) override; InsertionNotificationRequest InsertedInto(ContainerNode*) override; - void DidNotifySubtreeInsertionsToDocument() override; void RemovedFrom(ContainerNode*) override; void ChildrenChanged(const ChildrenChange&) override; diff --git a/chromium/third_party/WebKit/Source/core/svg/SVGStyleElement.cpp b/chromium/third_party/WebKit/Source/core/svg/SVGStyleElement.cpp index e3838ce5059..fef6d115e0f 100644 --- a/chromium/third_party/WebKit/Source/core/svg/SVGStyleElement.cpp +++ b/chromium/third_party/WebKit/Source/core/svg/SVGStyleElement.cpp @@ -105,14 +105,14 @@ void SVGStyleElement::FinishParsingChildren() { Node::InsertionNotificationRequest SVGStyleElement::InsertedInto( ContainerNode* insertion_point) { SVGElement::InsertedInto(insertion_point); - return kInsertionShouldCallDidNotifySubtreeInsertions; -} - -void SVGStyleElement::DidNotifySubtreeInsertionsToDocument() { - if (StyleElement::ProcessStyleSheet(GetDocument(), *this) == - StyleElement::kProcessingFatalError) - NotifyLoadedSheetAndAllCriticalSubresources( - kErrorOccurredLoadingSubresource); + if (isConnected()) { + if (StyleElement::ProcessStyleSheet(GetDocument(), *this) == + StyleElement::kProcessingFatalError) { + NotifyLoadedSheetAndAllCriticalSubresources( + kErrorOccurredLoadingSubresource); + } + } + return kInsertionDone; } void SVGStyleElement::RemovedFrom(ContainerNode* insertion_point) { diff --git a/chromium/third_party/WebKit/Source/core/svg/SVGStyleElement.h b/chromium/third_party/WebKit/Source/core/svg/SVGStyleElement.h index 9a090b95f93..305fbdbd2e8 100644 --- a/chromium/third_party/WebKit/Source/core/svg/SVGStyleElement.h +++ b/chromium/third_party/WebKit/Source/core/svg/SVGStyleElement.h @@ -58,7 +58,6 @@ class SVGStyleElement final : public SVGElement, public StyleElement { void ParseAttribute(const AttributeModificationParams&) override; InsertionNotificationRequest InsertedInto(ContainerNode*) override; - void DidNotifySubtreeInsertionsToDocument() override; void RemovedFrom(ContainerNode*) override; void ChildrenChanged(const ChildrenChange&) override; -- cgit v1.2.3