[webkit-changes] [182923] trunk/Source/WebCore

Thu, 16 Apr 2015 16:42:07 -0700

Title: [182923] trunk/Source/WebCore
Revision
182923
Author
cdu...@apple.com
Date
2015-04-16 16:41:27 -0700 (Thu, 16 Apr 2015)

Log Message

Add assertions to make sure ActiveDOMObject::suspend() / resume() / stop() overrides don't fire events
https://bugs.webkit.org/show_bug.cgi?id=143850

Reviewed by Alexey Proskuryakov.

Add assertions to make sure ActiveDOMObject::suspend() / resume() / stop()
overrides don't fire events as this is not allowed. This would cause
arbitrary JS execution which would be very dangerous in these stages.

Firing JS events from these functions is a common source of crashes.

* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::suspend):
(WebCore::WebSocket::resume):
* dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::suspendActiveDOMObjects):
(WebCore::ScriptExecutionContext::resumeActiveDOMObjects):
(WebCore::ScriptExecutionContext::stopActiveDOMObjects):
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::suspend):
(WebCore::XMLHttpRequest::resume):
(WebCore::XMLHttpRequest::stop):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (182922 => 182923)


--- trunk/Source/WebCore/ChangeLog	2015-04-16 23:41:22 UTC (rev 182922)
+++ trunk/Source/WebCore/ChangeLog	2015-04-16 23:41:27 UTC (rev 182923)
@@ -1,3 +1,28 @@
+2015-04-16  Chris Dumez  <cdu...@apple.com>
+
+        Add assertions to make sure ActiveDOMObject::suspend() / resume() / stop() overrides don't fire events
+        https://bugs.webkit.org/show_bug.cgi?id=143850
+
+        Reviewed by Alexey Proskuryakov.
+
+        Add assertions to make sure ActiveDOMObject::suspend() / resume() / stop()
+        overrides don't fire events as this is not allowed. This would cause
+        arbitrary JS execution which would be very dangerous in these stages.
+
+        Firing JS events from these functions is a common source of crashes.
+
+        * Modules/websockets/WebSocket.cpp:
+        (WebCore::WebSocket::suspend):
+        (WebCore::WebSocket::resume):
+        * dom/ScriptExecutionContext.cpp:
+        (WebCore::ScriptExecutionContext::suspendActiveDOMObjects):
+        (WebCore::ScriptExecutionContext::resumeActiveDOMObjects):
+        (WebCore::ScriptExecutionContext::stopActiveDOMObjects):
+        * xml/XMLHttpRequest.cpp:
+        (WebCore::XMLHttpRequest::suspend):
+        (WebCore::XMLHttpRequest::resume):
+        (WebCore::XMLHttpRequest::stop):
+
 2015-04-16  Brady Eidson  <beid...@apple.com>
 
         Media element can manipulate DOM during Document destruction.

Modified: trunk/Source/WebCore/Modules/websockets/WebSocket.cpp (182922 => 182923)


--- trunk/Source/WebCore/Modules/websockets/WebSocket.cpp	2015-04-16 23:41:22 UTC (rev 182922)
+++ trunk/Source/WebCore/Modules/websockets/WebSocket.cpp	2015-04-16 23:41:27 UTC (rev 182923)
@@ -468,8 +468,6 @@
 
 void WebSocket::suspend(ReasonForSuspension reason)
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
-
     if (m_resumeTimer.isActive())
         m_resumeTimer.stop();
 
@@ -486,8 +484,6 @@
 
 void WebSocket::resume()
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
-
     if (m_channel)
         m_channel->resume();
     else if (!m_pendingEvents.isEmpty() && !m_resumeTimer.isActive()) {

Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.cpp (182922 => 182923)


--- trunk/Source/WebCore/dom/ScriptExecutionContext.cpp	2015-04-16 23:41:22 UTC (rev 182922)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.cpp	2015-04-16 23:41:27 UTC (rev 182923)
@@ -227,6 +227,7 @@
     // functions should not add new active DOM objects, nor execute arbitrary _javascript_.
     // An ASSERT or RELEASE_ASSERT will fire if this happens, but it's important to code
     // suspend functions so it will not happen!
+    NoEventDispatchAssertion assertNoEventDispatch;
     for (auto* activeDOMObject : m_activeDOMObjects)
         activeDOMObject->suspend(why);
 
@@ -256,6 +257,7 @@
     // functions should not add new active DOM objects, nor execute arbitrary _javascript_.
     // An ASSERT or RELEASE_ASSERT will fire if this happens, but it's important to code
     // resume functions so it will not happen!
+    NoEventDispatchAssertion assertNoEventDispatch;
     for (auto* activeDOMObject : m_activeDOMObjects)
         activeDOMObject->resume();
 
@@ -283,6 +285,7 @@
     // stop functions should not add new active DOM objects, nor execute arbitrary _javascript_.
     // A RELEASE_ASSERT will fire if this happens, but it's important to code stop functions
     // so it will not happen!
+    NoEventDispatchAssertion assertNoEventDispatch;
     for (auto* activeDOMObject : possibleActiveDOMObjects) {
         // Check if this object was deleted already. If so, just skip it.
         // Calling contains on a possibly-already-deleted object is OK because we guarantee

Modified: trunk/Source/WebCore/xml/XMLHttpRequest.cpp (182922 => 182923)


--- trunk/Source/WebCore/xml/XMLHttpRequest.cpp	2015-04-16 23:41:22 UTC (rev 182922)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.cpp	2015-04-16 23:41:27 UTC (rev 182923)
@@ -1265,8 +1265,6 @@
 
 void XMLHttpRequest::suspend(ReasonForSuspension reason)
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
-
     m_progressEventThrottle.suspend();
 
     if (m_resumeTimer.isActive()) {
@@ -1287,8 +1285,6 @@
 
 void XMLHttpRequest::resume()
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
-
     m_progressEventThrottle.resume();
 
     // We are not allowed to execute arbitrary JS in resume() so dispatch
@@ -1306,7 +1302,6 @@
 
 void XMLHttpRequest::stop()
 {
-    NoEventDispatchAssertion assertNoEventDispatch;
     internalAbort();
 }
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to