Changeset 139788 in webkit

Timestamp:

Jan 15, 2013, 2:14:29 PM (12 years ago)

Author:

esprehn@chromium.org

Message:

Heap-use-after-free in WebCore::RenderObject::willBeRemovedFromTree
​https://bugs.webkit.org/show_bug.cgi?id=106384

Reviewed by Abhishek Arya.

Source/WebCore:

Always walk up from beforeChild until the parent() is the owner of the
child list, otherwise we can end up in situations where
newChild->parent() == owner but newChild->nextSibling()->parent() != owner
which is a recipe for security bugs. Previously we only walked up through
anonymous blocks, but missed anonymous inline blocks like those generated
by <ruby>.

Test: fast/css-generated-content/bug-106384.html

• rendering/RenderObjectChildList.cpp:

(WebCore::RenderObjectChildList::insertChildNode):

LayoutTests:

Add a test for <ruby> and generated content causing asserts and
crashes.

• fast/css-generated-content/bug-106384-expected.txt: Added.
• fast/css-generated-content/bug-106384.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css-generated-content/bug-106384-expected.txt (added)
• LayoutTests/fast/css-generated-content/bug-106384.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderObjectChildList.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-01-15  Elliott Sprehn  <esprehn@chromium.org>
+
+        Heap-use-after-free in WebCore::RenderObject::willBeRemovedFromTree
+        https://bugs.webkit.org/show_bug.cgi?id=106384
+
+        Reviewed by Abhishek Arya.
+
+        Add a test for <ruby> and generated content causing asserts and
+        crashes.
+
+        * fast/css-generated-content/bug-106384-expected.txt: Added.
+        * fast/css-generated-content/bug-106384.html: Added.
+
 2013-01-15  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-15  Elliott Sprehn  <esprehn@chromium.org>
+
+        Heap-use-after-free in WebCore::RenderObject::willBeRemovedFromTree
+        https://bugs.webkit.org/show_bug.cgi?id=106384
+
+        Reviewed by Abhishek Arya.
+
+        Always walk up from beforeChild until the parent() is the owner of the
+        child list, otherwise we can end up in situations where
+        newChild->parent() == owner but newChild->nextSibling()->parent() != owner
+        which is a recipe for security bugs. Previously we only walked up through
+        anonymous blocks, but missed anonymous inline blocks like those generated
+        by <ruby>.
+
+        Test: fast/css-generated-content/bug-106384.html
+
+        * rendering/RenderObjectChildList.cpp:
+        (WebCore::RenderObjectChildList::insertChildNode):
+
 2013-01-15  Ojan Vafai  <ojan@chromium.org>
 

trunk/Source/WebCore/rendering/RenderObjectChildList.cpp

@@ -154 +154 @@
 
     ASSERT(!child->parent());
-    while (beforeChild->parent() != owner && beforeChild->parent()->isAnonymousBlock())
+    while (beforeChild->parent() && beforeChild->parent() != owner)
         beforeChild = beforeChild->parent();
-    ASSERT(beforeChild->parent() == owner);
+
+    // This should never happen, but if it does prevent render tree corruption
+    // where child->parent() ends up being owner but child->nextSibling()->parent()
+    // is not owner.
+    if (beforeChild->parent() != owner) {
+        ASSERT_NOT_REACHED();
+        return;
+    }
 
     ASSERT(!owner->isBlockFlow() || (!child->isTableSection() && !child->isTableRow() && !child->isTableCell()));