use after free in WebCore::DocumentOrderedMap::remove / WebCore::TreeScope::removeElementById

https://bugs.webkit.org/show_bug.cgi?id=121324

Reviewed by Ryosuke Niwa.

Update the document ordered map for an image element before dispatching
load or error events
when it's inserted into a document.

Source/WebCore:

Test: fast/dom/modify-node-and-while-in-the-callback-too-crash.html

* dom/DocumentOrderedMap.cpp: defensive fix to avoid use after free
issues.
(WebCore::DocumentOrderedMap::remove):
* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::insertedInto):
* loader/ImageLoader.cpp:
(WebCore::ImageLoader::updateFromElement): setting m_failedLoadURL makes
repeated updateFromElement calls return early.

Change-Id: I305e56de969d0efe3dc67930cdf585a201e8c6a5
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@159481 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Reviewed-by: Michael Bruning <michael.bruning@digia.com>

3 files changed, 11 insertions, 3 deletions

diff --git a/Source/WebCore/dom/DocumentOrderedMap.cpp b/Source/WebCore/dom/DocumentOrderedMap.cpp
index 783c99b05..7530b99c6 100644
--- a/Source/WebCore/dom/DocumentOrderedMap.cpp
+++ b/Source/WebCore/dom/DocumentOrderedMap.cpp
@@ -107,6 +107,9 @@ void DocumentOrderedMap::remove(AtomicStringImpl* key, Element* element)
     m_map.checkConsistency();
     Map::iterator it = m_map.find(key);
     ASSERT(it != m_map.end());
+    if (it == m_map.end())
+        return;
+
     MapEntry& entry = it->value;
 
     ASSERT(entry.count);
diff --git a/Source/WebCore/html/HTMLImageElement.cpp b/Source/WebCore/html/HTMLImageElement.cpp
index 0dd581dfc..6dcd52989 100644
--- a/Source/WebCore/html/HTMLImageElement.cpp
+++ b/Source/WebCore/html/HTMLImageElement.cpp
@@ -204,12 +204,16 @@ Node::InsertionNotificationRequest HTMLImageElement::insertedInto(ContainerNode*
         }
     }
 
+    // Insert needs to complete first, before we start updating the loader. Loader dispatches events which could result
+    // in callbacks back to this node.
+    Node::InsertionNotificationRequest insertNotificationRequest = HTMLElement::insertedInto(insertionPoint);
+
     // If we have been inserted from a renderer-less document,
     // our loader may have not fetched the image, so do it now.
     if (insertionPoint->inDocument() && !m_imageLoader.image())
         m_imageLoader.updateFromElement();
 
-    return HTMLElement::insertedInto(insertionPoint);
+    return insertNotificationRequest;
 }
 
 void HTMLImageElement::removedFrom(ContainerNode* insertionPoint)
diff --git a/Source/WebCore/loader/ImageLoader.cpp b/Source/WebCore/loader/ImageLoader.cpp
index c9fc3a976..40a66ea01 100644
--- a/Source/WebCore/loader/ImageLoader.cpp
+++ b/Source/WebCore/loader/ImageLoader.cpp
@@ -212,8 +212,9 @@ void ImageLoader::updateFromElement()
             clearFailedLoadURL();
     } else if (!attr.isNull()) {
         // Fire an error event if the url is empty.
-        // FIXME: Should we fire this event asynchronoulsy via errorEventSender()?
-        m_element->dispatchEvent(Event::create(eventNames().errorEvent, false, false));
+        m_failedLoadURL = attr;
+        m_hasPendingErrorEvent = true;
+        errorEventSender().dispatchEventSoon(this);
     }
     
     CachedImage* oldImage = m_image.get();