[RenderTreeUpdater] NULL ptr deref in updateRenderTree

https://bugs.webkit.org/show_bug.cgi?id=230581 Patch by Brandon Stewart <brandonstewart@apple.com> on 2021-09-24 Reviewed by Antti Koivisto. Source/WebCore: Text element changes are buffered. This can lead to undesirable behavior when switching a node to a document that is not rendered, and then proceeding with a rendering update. If we cannot find a renderer in a node or its ancestors then just give up instead of returning a document. Test: fast/dom/Document/clearPendingRenderTreeUpdater.html * rendering/updating/RenderTreeUpdater.cpp: (WebCore::findRenderingRoot): (WebCore::RenderTreeUpdater::commit): (WebCore::RenderTreeUpdater::createRenderer): (WebCore::RenderTreeUpdater::textRendererIsNeeded): LayoutTests: New regression test to handle case where we trigger a text update, and then switch the node to a new unrendered document. * fast/dom/Document/clearPendingRenderTreeUpdater-expected.txt: Added. * fast/dom/Document/clearPendingRenderTreeUpdater.html: Added. Canonical link: https://commits.webkit.org/242090@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@283030 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Sep 24, 2021 · 8d308cf · 8d308cf
1 parent a079be7
commit 8d308cf
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 5 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2021-09-24  Brandon Stewart  <brandonstewart@apple.com>
+
+        [RenderTreeUpdater] NULL ptr deref in updateRenderTree
+        https://bugs.webkit.org/show_bug.cgi?id=230581
+
+        Reviewed by Antti Koivisto.
+
+        New regression test to handle case where we trigger a text update,
+        and then switch the node to a new unrendered document.
+
+        * fast/dom/Document/clearPendingRenderTreeUpdater-expected.txt: Added.
+        * fast/dom/Document/clearPendingRenderTreeUpdater.html: Added.
+
 2021-09-24  Sihui Liu  <sihui_liu@apple.com>
 
         Add initial support for File System Access API

diff --git a/LayoutTests/fast/dom/Document/clearPendingRenderTreeUpdater-expected.txt b/LayoutTests/fast/dom/Document/clearPendingRenderTreeUpdater-expected.txt
@@ -0,0 +1 @@
+This test shall pass if it does not crash.
diff --git a/LayoutTests/fast/dom/Document/clearPendingRenderTreeUpdater.html b/LayoutTests/fast/dom/Document/clearPendingRenderTreeUpdater.html
@@ -0,0 +1,22 @@
+<html>
+<body>
+<script>
+    onload = () => {
+        if (window.testRunner)
+          testRunner.dumpAsText()
+
+        let div0 = document.createElement('div');
+        div0.style.display = 'contents';
+        let div1 = document.createElement('div');
+        div1.append('ab');
+        div0.appendChild(div1);
+        document.body.appendChild(div0);
+        document.body.offsetTop;
+        div1.innerHTML = 'a';
+        new Document().appendChild(div0);
+    }
+</script>
+
+This test shall pass if it does not crash.
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2021-09-24  Brandon Stewart  <brandonstewart@apple.com>
+
+        [RenderTreeUpdater] NULL ptr deref in updateRenderTree
+        https://bugs.webkit.org/show_bug.cgi?id=230581
+
+        Reviewed by Antti Koivisto.
+
+        Text element changes are buffered. This can lead to undesirable behavior
+        when switching a node to a document that is not rendered, and then proceeding
+        with a rendering update.
+
+        If we cannot find a renderer in a node or its ancestors then just give up
+        instead of returning a document.
+
+        Test: fast/dom/Document/clearPendingRenderTreeUpdater.html
+
+        * rendering/updating/RenderTreeUpdater.cpp:
+        (WebCore::findRenderingRoot):
+        (WebCore::RenderTreeUpdater::commit):
+        (WebCore::RenderTreeUpdater::createRenderer):
+        (WebCore::RenderTreeUpdater::textRendererIsNeeded):
+
 2021-09-24  Sihui Liu  <sihui_liu@apple.com>
 
         Add initial support for File System Access API

diff --git a/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp b/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2021 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -96,7 +96,7 @@ static ContainerNode* findRenderingRoot(ContainerNode& node)
         if (!ancestor.hasDisplayContents())
             return nullptr;
     }
-    return &node.document();
+    return nullptr;
 }
 
 static ListHashSet<ContainerNode*> findRenderingRoots(const Style::Update& update)
@@ -117,7 +117,7 @@ void RenderTreeUpdater::commit(std::unique_ptr<const Style::Update> styleUpdate)
 
     if (!m_document.shouldCreateRenderers() || !m_document.renderView())
         return;
-    
+
     TraceScope scope(RenderTreeBuildStart, RenderTreeBuildEnd);
 
     m_styleUpdate = WTFMove(styleUpdate);
@@ -373,7 +373,7 @@ void RenderTreeUpdater::createRenderer(Element& element, RenderStyle&& style)
         renderTreePosition().computeNextSibling(element);
         return renderTreePosition();
     };
-    
+
     if (!shouldCreateRenderer(element, renderTreePosition().parent()))
         return;
 
@@ -443,7 +443,7 @@ bool RenderTreeUpdater::textRendererIsNeeded(const Text& textNode)
     } else {
         if (parentRenderer.isRenderBlock() && !parentRenderer.childrenInline() && (!previousRenderer || !previousRenderer->isInline()))
             return false;
-        
+
         RenderObject* first = parentRenderer.firstChild();
         while (first && first->isFloatingOrOutOfFlowPositioned())
             first = first->nextSibling();