Title: [150084] trunk
Revision
150084
Author
zol...@webkit.org
Date
2013-05-14 13:09:17 -0700 (Tue, 14 May 2013)

Log Message

Heap-use-after-free in WebCore::RenderBox::exclusionShapeOutsideInfo
https://bugs.webkit.org/show_bug.cgi?id=115566

Patch by Bem Jones-Bey <bjone...@adobe.com> on 2013-05-14
Reviewed by David Hyatt.

Source/WebCore:

When a portion of the render tree is being detached, anonymous blocks
will be combined as their children are deleted. In this process, the
anonymous block later in the tree is merged into the one preceeding it.
It can happen that the later block contains floats that the previous
block did not contain, and thus are not in the floating objects list for
the new block. This can result in the new block containing floats that
are not in it's floating objects list, but are in the floating objects
lists of siblings and parents. This can cause problems when the float
itself is deleted, since the deletion code assumes that if a float is not
in it's containing block's floating objects list, it isn't in any
floating objects list, causing dangling pointers in the floating objects
lists of the siblings and parents. In order to preserve this condition
(removing it has serious performance implications), we need to copy the
floating objects from the old block to the new block.  The float's
metrics will likely all be wrong, but since the new block is already
marked for layout, this will get fixed before anything gets displayed.

Test: fast/block/float/float-append-child-crash.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::moveAllChildrenIncludingFloatsTo): Copy
    floating objects list in addition to children.
(WebCore::RenderBlock::FloatingObject::clone): Added.
(WebCore::RenderBlock::removeChild): Use new method to copy children.
* rendering/RenderBlock.h:
(RenderBlock): Add method.

LayoutTests:

Cleaned up test from the fuzzer. Will only crash if run under a memory
checking tool like ASAN.

* fast/block/float/float-append-child-crash-expected.txt: Added.
* fast/block/float/float-append-child-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (150083 => 150084)


--- trunk/LayoutTests/ChangeLog	2013-05-14 19:57:51 UTC (rev 150083)
+++ trunk/LayoutTests/ChangeLog	2013-05-14 20:09:17 UTC (rev 150084)
@@ -1,3 +1,16 @@
+2013-05-14  Bem Jones-Bey  <bjone...@adobe.com>
+
+        Heap-use-after-free in WebCore::RenderBox::exclusionShapeOutsideInfo
+        https://bugs.webkit.org/show_bug.cgi?id=115566
+
+        Reviewed by David Hyatt.
+
+        Cleaned up test from the fuzzer. Will only crash if run under a memory
+        checking tool like ASAN.
+
+        * fast/block/float/float-append-child-crash-expected.txt: Added.
+        * fast/block/float/float-append-child-crash.html: Added.
+
 2013-05-14  Commit Queue  <commit-qu...@webkit.org>
 
         Unreviewed, rolling out r150023.

Added: trunk/LayoutTests/fast/block/float/float-append-child-crash-expected.txt (0 => 150084)


--- trunk/LayoutTests/fast/block/float/float-append-child-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/block/float/float-append-child-crash-expected.txt	2013-05-14 20:09:17 UTC (rev 150084)
@@ -0,0 +1 @@
+PASS. Test didn't crash.

Added: trunk/LayoutTests/fast/block/float/float-append-child-crash.html (0 => 150084)


--- trunk/LayoutTests/fast/block/float/float-append-child-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/block/float/float-append-child-crash.html	2013-05-14 20:09:17 UTC (rev 150084)
@@ -0,0 +1,17 @@
+<span id=enclosingInline><span><div style="backround-image: rgba(27, 48); font: 110px/7em sans-serif; zoom: 36717; "> }	 Z bP  v	ed
+</h3><span id=inlineToMove><div></div>
+x<div style='border-top: dashed; float: left; border: 4058 solid; '></div><div>x</div>
+<div><video>
+
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+}
+function crash() {
+    enclosingInline.appendChild(inlineToMove);
+    document.body.offsetTop;
+    document.body.innerHTML = "PASS. Test didn't crash.";
+}
+document.addEventListener("DOMContentLoaded", crash, false);
+</script>
+

Modified: trunk/Source/WebCore/ChangeLog (150083 => 150084)


--- trunk/Source/WebCore/ChangeLog	2013-05-14 19:57:51 UTC (rev 150083)
+++ trunk/Source/WebCore/ChangeLog	2013-05-14 20:09:17 UTC (rev 150084)
@@ -1,3 +1,37 @@
+2013-05-14  Bem Jones-Bey  <bjone...@adobe.com>
+
+        Heap-use-after-free in WebCore::RenderBox::exclusionShapeOutsideInfo
+        https://bugs.webkit.org/show_bug.cgi?id=115566
+
+        Reviewed by David Hyatt.
+
+        When a portion of the render tree is being detached, anonymous blocks
+        will be combined as their children are deleted. In this process, the
+        anonymous block later in the tree is merged into the one preceeding it.
+        It can happen that the later block contains floats that the previous
+        block did not contain, and thus are not in the floating objects list for
+        the new block. This can result in the new block containing floats that
+        are not in it's floating objects list, but are in the floating objects
+        lists of siblings and parents. This can cause problems when the float
+        itself is deleted, since the deletion code assumes that if a float is not
+        in it's containing block's floating objects list, it isn't in any
+        floating objects list, causing dangling pointers in the floating objects
+        lists of the siblings and parents. In order to preserve this condition
+        (removing it has serious performance implications), we need to copy the
+        floating objects from the old block to the new block.  The float's
+        metrics will likely all be wrong, but since the new block is already
+        marked for layout, this will get fixed before anything gets displayed.
+
+        Test: fast/block/float/float-append-child-crash.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::moveAllChildrenIncludingFloatsTo): Copy
+            floating objects list in addition to children.
+        (WebCore::RenderBlock::FloatingObject::clone): Added.
+        (WebCore::RenderBlock::removeChild): Use new method to copy children.
+        * rendering/RenderBlock.h:
+        (RenderBlock): Add method.
+
 2013-05-14  Commit Queue  <commit-qu...@webkit.org>
 
         Unreviewed, rolling out r150023.

Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (150083 => 150084)


--- trunk/Source/WebCore/rendering/RenderBlock.cpp	2013-05-14 19:57:51 UTC (rev 150083)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp	2013-05-14 20:09:17 UTC (rev 150084)
@@ -1167,6 +1167,46 @@
     anonBlock->destroy();
 }
 
+void RenderBlock::moveAllChildrenIncludingFloatsTo(RenderBlock* toBlock, bool fullRemoveInsert)
+{
+    moveAllChildrenTo(toBlock, fullRemoveInsert);
+
+    // When a portion of the render tree is being detached, anonymous blocks
+    // will be combined as their children are deleted. In this process, the
+    // anonymous block later in the tree is merged into the one preceeding it.
+    // It can happen that the later block (this) contains floats that the
+    // previous block (toBlock) did not contain, and thus are not in the
+    // floating objects list for toBlock. This can result in toBlock containing
+    // floats that are not in it's floating objects list, but are in the
+    // floating objects lists of siblings and parents. This can cause problems
+    // when the float itself is deleted, since the deletion code assumes that
+    // if a float is not in it's containing block's floating objects list, it
+    // isn't in any floating objects list. In order to preserve this condition
+    // (removing it has serious performance implications), we need to copy the
+    // floating objects from the old block (this) to the new block (toBlock).
+    // The float's metrics will likely all be wrong, but since toBlock is
+    // already marked for layout, this will get fixed before anything gets
+    // displayed.
+    // See bug https://bugs.webkit.org/show_bug.cgi?id=115566
+    if (m_floatingObjects) {
+        if (!toBlock->m_floatingObjects)
+            toBlock->createFloatingObjects();
+
+        const FloatingObjectSet& fromFloatingObjectSet = m_floatingObjects->set();
+        FloatingObjectSetIterator end = fromFloatingObjectSet.end();
+
+        for (FloatingObjectSetIterator it = fromFloatingObjectSet.begin(); it != end; ++it) {
+            FloatingObject* floatingObject = *it;
+
+            // Don't insert the object again if it's already in the list
+            if (toBlock->containsFloat(floatingObject->renderer()))
+                continue;
+
+            toBlock->m_floatingObjects->add(floatingObject->clone());
+        }
+    }
+}
+
 void RenderBlock::removeChild(RenderObject* oldChild)
 {
     // No need to waste time in merging or removing empty anonymous blocks.
@@ -1219,7 +1259,7 @@
         } else {
             // Take all the children out of the |next| block and put them in
             // the |prev| block.
-            nextBlock->moveAllChildrenTo(prevBlock, nextBlock->hasLayer() || prevBlock->hasLayer());        
+            nextBlock->moveAllChildrenIncludingFloatsTo(prevBlock, nextBlock->hasLayer() || prevBlock->hasLayer());
             
             // Delete the now-empty block's lines and nuke it.
             nextBlock->deleteLineBoxTree();

Modified: trunk/Source/WebCore/rendering/RenderBlock.h (150083 => 150084)


--- trunk/Source/WebCore/rendering/RenderBlock.h	2013-05-14 19:57:51 UTC (rev 150083)
+++ trunk/Source/WebCore/rendering/RenderBlock.h	2013-05-14 20:09:17 UTC (rev 150084)
@@ -603,6 +603,7 @@
     virtual void removeLeftoverAnonymousBlock(RenderBlock* child);
 
     static void collapseAnonymousBoxChild(RenderBlock* parent, RenderObject* child);
+    void moveAllChildrenIncludingFloatsTo(RenderBlock* toBlock, bool fullRemoveInsert);
 
     virtual void dirtyLinesFromChangedChild(RenderObject* child) { m_lineBoxes.dirtyLinesFromChangedChild(this, child); }
 
@@ -688,6 +689,18 @@
         {
         }
 
+        FloatingObject* clone() const
+        {
+            FloatingObject* cloneObject = new FloatingObject(type(), m_frameRect);
+            cloneObject->m_renderer = m_renderer;
+            cloneObject->m_originatingLine = m_originatingLine;
+            cloneObject->m_paginationStrut = m_paginationStrut;
+            cloneObject->m_shouldPaint = m_shouldPaint;
+            cloneObject->m_isDescendant = m_isDescendant;
+            cloneObject->m_isPlaced = m_isPlaced;
+            return cloneObject;
+        }
+
         Type type() const { return static_cast<Type>(m_type); }
         RenderBox* renderer() const { return m_renderer; }
         
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to