Changeset 224290 in webkit

Timestamp:

Nov 1, 2017 1:38:58 PM (6 years ago)

Author:

rniwa@webkit.org

Message:

Assert that NoEventDispatchAssertion is not in the stack when executing a script
​https://bugs.webkit.org/show_bug.cgi?id=179107

Reviewed by Simon Fraser.

Assert that NoEventDispatchAssertion::isEventAllowedInMainThread() is true when we're about to execute a script
by adding the assertion in ScriptController::canExecuteScripts which gets called with AboutToExecuteScript
whenever we're about to run scripts in event handlers, etc...

We don't assert the construction of event handlers in JSLazyEventListener since this happens while copying
the DOM tree inside a SVG use element and creating a event handler's JS function won't execute arbitrary scripts.

No new tests since there should be no behavioral change other

• bindings/js/JSLazyEventListener.cpp:

(WebCore::JSLazyEventListener::initializeJSFunction const): Use newly added AboutToCreateEventListener.
(WebCore::JSLazyEventListener::create): Ditto.

• bindings/js/ScriptController.cpp:

(WebCore::ScriptController::canExecuteScripts): Added the assertion.

• bindings/js/ScriptController.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/JSLazyEventListener.cpp (modified) (2 diffs)
• bindings/js/ScriptController.cpp (modified) (2 diffs)
• bindings/js/ScriptController.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-11-01  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Assert that NoEventDispatchAssertion is not in the stack when executing a script
+        https://bugs.webkit.org/show_bug.cgi?id=179107
+
+        Reviewed by Simon Fraser.
+
+        Assert that NoEventDispatchAssertion::isEventAllowedInMainThread() is true when we're about to execute a script
+        by adding the assertion in ScriptController::canExecuteScripts which gets called with AboutToExecuteScript
+        whenever we're about to run scripts in event handlers, etc...
+
+        We don't assert the construction of event handlers in JSLazyEventListener since this happens while copying
+        the DOM tree inside a SVG use element and creating a event handler's JS function won't execute arbitrary scripts.
+
+        No new tests since there should be no behavioral change other 
+
+        * bindings/js/JSLazyEventListener.cpp:
+        (WebCore::JSLazyEventListener::initializeJSFunction const): Use newly added AboutToCreateEventListener.
+        (WebCore::JSLazyEventListener::create): Ditto.
+        * bindings/js/ScriptController.cpp:
+        (WebCore::ScriptController::canExecuteScripts): Added the assertion.
+        * bindings/js/ScriptController.h:
+
 2017-11-01  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/bindings/js/JSLazyEventListener.cpp

@@ -98 +98 @@
 
     ScriptController& script = document.frame()->script();
-    if (!script.canExecuteScripts(AboutToExecuteScript) || script.isPaused())
+    if (!script.canExecuteScripts(AboutToCreateEventListener) || script.isPaused())
         return nullptr;
 
@@ -169 +169 @@
     String sourceURL;
     if (Frame* frame = arguments.document.frame()) {
-        if (!frame->script().canExecuteScripts(AboutToExecuteScript))
+        if (!frame->script().canExecuteScripts(AboutToCreateEventListener))
             return nullptr;
         position = frame->script().eventHandlerPosition();

trunk/Source/WebCore/bindings/js/ScriptController.cpp

@@ -44 +44 @@
 #include "ModuleFetchParameters.h"
 #include "NP_jsobject.h"
+#include "NoEventDispatchAssertion.h"
 #include "Page.h"
 #include "PageConsoleClient.h"
@@ -677 +678 @@
 bool ScriptController::canExecuteScripts(ReasonForCallingCanExecuteScripts reason)
 {
+    if (reason == AboutToExecuteScript)
+        ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());
+
     if (m_frame.document() && m_frame.document()->isSandboxed(SandboxScripts)) {
         // FIXME: This message should be moved off the console once a solution to https://bugs.webkit.org/show_bug.cgi?id=103274 exists.
-        if (reason == AboutToExecuteScript)
+        if (reason == AboutToExecuteScript || reason == AboutToCreateEventListener)
             m_frame.document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, "Blocked script execution in '" + m_frame.document()->url().stringCenterEllipsizedToLength() + "' because the document's frame is sandboxed and the 'allow-scripts' permission is not set.");
         return false;

trunk/Source/WebCore/bindings/js/ScriptController.h

@@ -65 +65 @@
 
 enum ReasonForCallingCanExecuteScripts {
+    AboutToCreateEventListener,
     AboutToExecuteScript,
     NotAboutToExecuteScript