[Backport] CVE-2018-16066

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 <michael.bruning@qt.io>

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;