Title: [149185] trunk
Revision
149185
Author
commit-qu...@webkit.org
Date
2013-04-26 08:13:02 -0700 (Fri, 26 Apr 2013)

Log Message

use-after-free removing a frame from its parent in a beforeload event of an OBJECT element
https://bugs.webkit.org/show_bug.cgi?id=113964

Source/WebCore:

Object elements have the tendecny to modify or even fully remove
the containing Document inside beforeload callback. While Document is removed,
RenderArena gets destroyed. Retained RenderWidgets fails to function with NULL arena.

Protect RendereArena from getting wiped out, when Document is removed
during FrameView::updateWidget().

Patch by Zalan Bujtas <za...@apple.com> on 2013-04-26
Reviewed by Antti Koivisto.

Test: fast/frames/crash-remove-iframe-during-object-beforeload.html

* dom/Document.cpp:
(WebCore::Document::attach):
* dom/Document.h:
(Document):
* page/FrameView.cpp:
(WebCore::FrameView::updateWidgets):
* rendering/RenderArena.h:
(RenderArena):
(WebCore::RenderArena::create):

LayoutTests:

Patch by Zalan Bujtas <za...@apple.com> on 2013-04-26
Reviewed by Antti Koivisto.

* fast/frames/crash-remove-iframe-during-object-beforeload-expected.txt: Added.
* fast/frames/crash-remove-iframe-during-object-beforeload.html: Added.
* fast/frames/resources/remove-this-during-object-beforeload.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (149184 => 149185)


--- trunk/LayoutTests/ChangeLog	2013-04-26 14:56:21 UTC (rev 149184)
+++ trunk/LayoutTests/ChangeLog	2013-04-26 15:13:02 UTC (rev 149185)
@@ -1,3 +1,14 @@
+2013-04-26  Zalan Bujtas  <za...@apple.com>
+
+        use-after-free removing a frame from its parent in a beforeload event of an OBJECT element
+        https://bugs.webkit.org/show_bug.cgi?id=113964
+
+        Reviewed by Antti Koivisto.
+
+        * fast/frames/crash-remove-iframe-during-object-beforeload-expected.txt: Added.
+        * fast/frames/crash-remove-iframe-during-object-beforeload.html: Added.
+        * fast/frames/resources/remove-this-during-object-beforeload.html: Added.
+
 2013-04-26  Zan Dobersek  <zdober...@igalia.com>
 
         [GTK] Enable Web Audio layout tests on WK2, provide platform-specific expected output

Added: trunk/LayoutTests/fast/frames/crash-remove-iframe-during-object-beforeload-expected.txt (0 => 149185)


--- trunk/LayoutTests/fast/frames/crash-remove-iframe-during-object-beforeload-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/frames/crash-remove-iframe-during-object-beforeload-expected.txt	2013-04-26 15:13:02 UTC (rev 149185)
@@ -0,0 +1,3 @@
+This tests if removing the iframe, while the iframe's object's beforeload is executed is safe.
+PASS if no crash is observed, while loading iframe.
+

Added: trunk/LayoutTests/fast/frames/crash-remove-iframe-during-object-beforeload.html (0 => 149185)


--- trunk/LayoutTests/fast/frames/crash-remove-iframe-during-object-beforeload.html	                        (rev 0)
+++ trunk/LayoutTests/fast/frames/crash-remove-iframe-during-object-beforeload.html	2013-04-26 15:13:02 UTC (rev 149185)
@@ -0,0 +1,17 @@
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+</script>
+</head>
+<body>
+<div>This tests if removing the iframe, while the iframe's object's beforeload is executed is safe.</div> 
+<div>PASS if no crash is observed, while loading iframe.</div>
+<iframe _onload_='setTimeout("if (window.testRunner) testRunner.notifyDone()", 100);' 
+        src=''></iframe>
+</body>
+</html>

Added: trunk/LayoutTests/fast/frames/resources/remove-this-during-object-beforeload.html (0 => 149185)


--- trunk/LayoutTests/fast/frames/resources/remove-this-during-object-beforeload.html	                        (rev 0)
+++ trunk/LayoutTests/fast/frames/resources/remove-this-during-object-beforeload.html	2013-04-26 15:13:02 UTC (rev 149185)
@@ -0,0 +1,19 @@
+<html>
+<head>
+<script>
+    var count = 0;
+    function handleBeforeLoad() {
+        if (++count == 2)
+            window.frameElement.parentNode.removeChild(window.frameElement);
+    }
+   			
+    function documentLoaded() {
+        var o = document.createElement('object');
+        o.addEventListener('beforeload', handleBeforeLoad, false);
+        document.body.appendChild(o);
+    }
+</script>
+</head>
+<body _onload_='documentLoaded();'>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (149184 => 149185)


--- trunk/Source/WebCore/ChangeLog	2013-04-26 14:56:21 UTC (rev 149184)
+++ trunk/Source/WebCore/ChangeLog	2013-04-26 15:13:02 UTC (rev 149185)
@@ -1,3 +1,29 @@
+2013-04-26  Zalan Bujtas  <za...@apple.com>
+
+        use-after-free removing a frame from its parent in a beforeload event of an OBJECT element
+        https://bugs.webkit.org/show_bug.cgi?id=113964
+
+        Object elements have the tendecny to modify or even fully remove 
+        the containing Document inside beforeload callback. While Document is removed, 
+        RenderArena gets destroyed. Retained RenderWidgets fails to function with NULL arena.
+        
+        Protect RendereArena from getting wiped out, when Document is removed 
+        during FrameView::updateWidget(). 
+
+        Reviewed by Antti Koivisto.
+
+        Test: fast/frames/crash-remove-iframe-during-object-beforeload.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::attach):
+        * dom/Document.h:
+        (Document):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::updateWidgets):
+        * rendering/RenderArena.h:
+        (RenderArena):
+        (WebCore::RenderArena::create):
+
 2013-04-26  Christophe Dumez  <ch.du...@sisa.samsung.com>
 
         Optimize function and interface object length computation in bindings generator

Modified: trunk/Source/WebCore/dom/Document.cpp (149184 => 149185)


--- trunk/Source/WebCore/dom/Document.cpp	2013-04-26 14:56:21 UTC (rev 149184)
+++ trunk/Source/WebCore/dom/Document.cpp	2013-04-26 15:13:02 UTC (rev 149185)
@@ -2070,7 +2070,7 @@
     ASSERT(!m_axObjectCache || this != topDocument());
 
     if (!m_renderArena)
-        m_renderArena = adoptPtr(new RenderArena);
+        m_renderArena = RenderArena::create();
     
     // Create the rendering tree
     setRenderer(new (m_renderArena.get()) RenderView(this));

Modified: trunk/Source/WebCore/dom/Document.h (149184 => 149185)


--- trunk/Source/WebCore/dom/Document.h	2013-04-26 14:56:21 UTC (rev 149184)
+++ trunk/Source/WebCore/dom/Document.h	2013-04-26 15:13:02 UTC (rev 149185)
@@ -1412,7 +1412,7 @@
     bool m_titleSetExplicitly;
     RefPtr<Element> m_titleElement;
 
-    OwnPtr<RenderArena> m_renderArena;
+    RefPtr<RenderArena> m_renderArena;
 
     OwnPtr<AXObjectCache> m_axObjectCache;
     OwnPtr<DocumentMarkerController> m_markers;

Modified: trunk/Source/WebCore/page/FrameView.cpp (149184 => 149185)


--- trunk/Source/WebCore/page/FrameView.cpp	2013-04-26 14:56:21 UTC (rev 149184)
+++ trunk/Source/WebCore/page/FrameView.cpp	2013-04-26 15:13:02 UTC (rev 149185)
@@ -2654,6 +2654,8 @@
 
     Vector<RenderObject*> objects;
     objects.reserveInitialCapacity(size);
+    // Protect RendereArena from getting wiped out, when Document is detached during updateWidget().
+    RefPtr<RenderArena> protectedArena = m_frame->document()->renderArena();
 
     RenderObjectSet::const_iterator end = m_widgetUpdateSet->end();
     for (RenderObjectSet::const_iterator it = m_widgetUpdateSet->begin(); it != end; ++it) {
@@ -2671,12 +2673,11 @@
         m_widgetUpdateSet->remove(object);
     }
 
-    RenderArena* arena = m_frame->document()->renderArena();
     for (size_t i = 0; i < size; ++i) {
         RenderObject* object = objects[i];
         if (object->isEmbeddedObject()) {
             RenderEmbeddedObject* embeddedObject = static_cast<RenderEmbeddedObject*>(object);
-            embeddedObject->deref(arena);
+            embeddedObject->deref(protectedArena.get());
         }
     }
     

Modified: trunk/Source/WebCore/rendering/RenderArena.h (149184 => 149185)


--- trunk/Source/WebCore/rendering/RenderArena.h	2013-04-26 14:56:21 UTC (rev 149184)
+++ trunk/Source/WebCore/rendering/RenderArena.h	2013-04-26 15:13:02 UTC (rev 149185)
@@ -38,15 +38,16 @@
 #include "Arena.h"
 #include <wtf/FastAllocBase.h>
 #include <wtf/Noncopyable.h>
+#include <wtf/PassRefPtr.h>
+#include <wtf/RefCounted.h>
 
 namespace WebCore {
 
 static const size_t gMaxRecycledSize = 1024;
 
-class RenderArena {
-    WTF_MAKE_NONCOPYABLE(RenderArena); WTF_MAKE_FAST_ALLOCATED;
+class RenderArena : public RefCounted<RenderArena> {
 public:
-    explicit RenderArena(unsigned arenaSize = 8192);
+    static PassRefPtr<RenderArena> create() { return adoptRef(new RenderArena); }
     ~RenderArena();
 
     // Memory management functions
@@ -57,6 +58,8 @@
     size_t totalRenderArenaAllocatedBytes() const { return m_totalAllocated; }
 
 private:
+    RenderArena(unsigned arenaSize = 8192);
+    
     // Underlying arena pool
     ArenaPool m_pool;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to