[[Delete]] should throw for cross-origin Window / Location objects

https://bugs.webkit.org/show_bug.cgi?id=161397 Reviewed by Ryosuke Niwa. Source/WebCore: [[Delete]] should throw for cross-origin Window / Location objects: - whatwg/html#1728 Firefox and Chrome already throw. Previously, WebKit was merely ignoring the call and logging an error message. No new tests, updated existing test. * bindings/js/JSDOMWindowCustom.cpp: (WebCore::JSDOMWindow::deleteProperty): (WebCore::JSDOMWindow::deletePropertyByIndex): * bindings/js/JSLocationCustom.cpp: (WebCore::JSLocation::deleteProperty): (WebCore::JSLocation::deletePropertyByIndex): LayoutTests: Update / rebaseline existing test to reflect behavior change. * http/tests/security/cross-frame-access-delete-expected.txt: * http/tests/security/cross-frame-access-delete.html: * http/tests/security/resources/cross-frame-iframe-for-delete-test.html: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@205200 268f45cc-cd09-0410-ab3c-d52691b4dbfc
paul99 · Aug 30, 2016 · 1a72400 · 1a72400
1 parent e5fba8d
commit 1a72400
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 23 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2016-08-30  Chris Dumez  <cdumez@apple.com>
+
+        [[Delete]] should throw for cross-origin Window / Location objects
+        https://bugs.webkit.org/show_bug.cgi?id=161397
+
+        Reviewed by Ryosuke Niwa.
+
+        Update / rebaseline existing test to reflect behavior change.
+
+        * http/tests/security/cross-frame-access-delete-expected.txt:
+        * http/tests/security/cross-frame-access-delete.html:
+        * http/tests/security/resources/cross-frame-iframe-for-delete-test.html:
+
 2016-08-30  Ryan Haddad  <ryanhaddad@apple.com>
 
         Marking js/dom/prototype-chain-caching-with-impure-get-own-property-slot-traps-5.html as flaky on mac.

diff --git a/LayoutTests/http/tests/security/cross-frame-access-delete-expected.txt b/LayoutTests/http/tests/security/cross-frame-access-delete-expected.txt
@@ -1,12 +1,18 @@
-CONSOLE MESSAGE: line 1: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
-CONSOLE MESSAGE: line 1: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
-CONSOLE MESSAGE: line 1: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
-CONSOLE MESSAGE: line 1: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
-
-PASS: eval('delete targetWindow.existingProperty') should be 'false' and is.
-PASS: eval('delete targetWindow[1]') should be 'false' and is.
-PASS: eval('delete targetWindow.location.existingProperty') should be 'false' and is.
-PASS: eval('delete targetWindow.location[1]') should be 'false' and is.
+Tests [[Delete]] for cross origin Window / Location.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS delete targetWindow.existingProperty threw exception SecurityError (DOM Exception 18): Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match..
+PASS delete targetWindow.name threw exception SecurityError (DOM Exception 18): Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match..
+PASS delete targetWindow[1] threw exception SecurityError (DOM Exception 18): Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match..
+PASS delete targetWindow.location.existingProperty threw exception SecurityError (DOM Exception 18): Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match..
+PASS delete targetWindow.location.host threw exception SecurityError (DOM Exception 18): Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match..
+PASS delete targetWindow.location[1] threw exception SecurityError (DOM Exception 18): Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match..
+PASS: successfullyParsed should be 'true' and is.
+
+TEST COMPLETE
+
 
 
 --------

diff --git a/LayoutTests/http/tests/security/cross-frame-access-delete.html b/LayoutTests/http/tests/security/cross-frame-access-delete.html
@@ -1,28 +1,33 @@
 <html>
 <head>
+    <script src="/js-test-resources/js-test-pre.js"></script>
     <script src="resources/cross-frame-access.js"></script>
     <script>
-        if (window.testRunner) {
-            testRunner.dumpAsText();
+        description("Tests [[Delete]] for cross origin Window / Location.");
+        jsTestIsAsync = true;
+
+        if (window.testRunner)
             testRunner.dumpChildFramesAsText();
-            testRunner.waitUntilDone();
-        }
 
         receiver = function(e)
         {
             if (e.data == "setValuesComplete")
                 deleteTest();
+            if (e.data == "checkValuesComplete")
+                finishJSTest();
         }
         addEventListener('message', receiver, false);
 
         deleteTest = function()
         {
             targetWindow = frames[0];
 
-            shouldBe("eval('delete targetWindow.existingProperty')", "false");
-            shouldBe("eval('delete targetWindow[1]')", "false");
-            shouldBe("eval('delete targetWindow.location.existingProperty')", "false");
-            shouldBe("eval('delete targetWindow.location[1]')", "false");
+            shouldThrowErrorName("delete targetWindow.existingProperty", "SecurityError");
+            shouldThrowErrorName("delete targetWindow.name", "SecurityError");
+            shouldThrowErrorName("delete targetWindow[1]", "SecurityError");
+            shouldThrowErrorName("delete targetWindow.location.existingProperty", "SecurityError");
+            shouldThrowErrorName("delete targetWindow.location.host", "SecurityError");
+            shouldThrowErrorName("delete targetWindow.location[1]", "SecurityError");
 
             targetWindow.postMessage("deletingValuesComplete", "*");
         }
@@ -31,5 +36,6 @@
 <body>
     <iframe src="http://localhost:8000/security/resources/cross-frame-iframe-for-delete-test.html"></iframe>
     <pre id="console"></pre>
+    <script src="/js-test-resources/js-test-post.js"></script>
 </body>
 </html>
diff --git a/LayoutTests/http/tests/security/resources/cross-frame-iframe-for-delete-test.html b/LayoutTests/http/tests/security/resources/cross-frame-iframe-for-delete-test.html
@@ -32,8 +32,7 @@
             shouldBe("window.location.existingProperty", "'test value'");
             shouldBe("window.location[1]", "'test value'");
 
-            if (window.testRunner)
-                testRunner.notifyDone();
+             window.parent.postMessage("checkValuesComplete", "*");
         }
     </script>
 </head>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2016-08-30  Chris Dumez  <cdumez@apple.com>
+
+        [[Delete]] should throw for cross-origin Window / Location objects
+        https://bugs.webkit.org/show_bug.cgi?id=161397
+
+        Reviewed by Ryosuke Niwa.
+
+        [[Delete]] should throw for cross-origin Window / Location objects:
+        - https://github.com/whatwg/html/pull/1728
+
+        Firefox and Chrome already throw. Previously, WebKit was merely
+        ignoring the call and logging an error message.
+
+        No new tests, updated existing test.
+
+        * bindings/js/JSDOMWindowCustom.cpp:
+        (WebCore::JSDOMWindow::deleteProperty):
+        (WebCore::JSDOMWindow::deletePropertyByIndex):
+        * bindings/js/JSLocationCustom.cpp:
+        (WebCore::JSLocation::deleteProperty):
+        (WebCore::JSLocation::deletePropertyByIndex):
+
 2016-08-30  Brady Eidson  <beidson@apple.com>
 
         GameController.framework backend for gamepad API.

diff --git a/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp b/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp
@@ -269,7 +269,7 @@ bool JSDOMWindow::deleteProperty(JSCell* cell, ExecState* exec, PropertyName pro
 {
     JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(cell);
     // Only allow deleting properties by frames in the same origin.
-    if (!BindingSecurity::shouldAllowAccessToDOMWindow(exec, thisObject->wrapped()))
+    if (!BindingSecurity::shouldAllowAccessToDOMWindow(exec, thisObject->wrapped(), ThrowSecurityError))
         return false;
     return Base::deleteProperty(thisObject, exec, propertyName);
 }
@@ -278,7 +278,7 @@ bool JSDOMWindow::deletePropertyByIndex(JSCell* cell, ExecState* exec, unsigned
 {
     JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(cell);
     // Only allow deleting properties by frames in the same origin.
-    if (!BindingSecurity::shouldAllowAccessToDOMWindow(exec, thisObject->wrapped()))
+    if (!BindingSecurity::shouldAllowAccessToDOMWindow(exec, thisObject->wrapped(), ThrowSecurityError))
         return false;
     return Base::deletePropertyByIndex(thisObject, exec, propertyName);
 }

diff --git a/Source/WebCore/bindings/js/JSLocationCustom.cpp b/Source/WebCore/bindings/js/JSLocationCustom.cpp
@@ -94,7 +94,7 @@ bool JSLocation::deleteProperty(JSCell* cell, ExecState* exec, PropertyName prop
 {
     JSLocation* thisObject = jsCast<JSLocation*>(cell);
     // Only allow deleting by frames in the same origin.
-    if (!shouldAllowAccessToFrame(exec, thisObject->wrapped().frame()))
+    if (!BindingSecurity::shouldAllowAccessToFrame(exec, thisObject->wrapped().frame(), ThrowSecurityError))
         return false;
     return Base::deleteProperty(thisObject, exec, propertyName);
 }
@@ -103,7 +103,7 @@ bool JSLocation::deletePropertyByIndex(JSCell* cell, ExecState* exec, unsigned p
 {
     JSLocation* thisObject = jsCast<JSLocation*>(cell);
     // Only allow deleting by frames in the same origin.
-    if (!shouldAllowAccessToFrame(exec, thisObject->wrapped().frame()))
+    if (!BindingSecurity::shouldAllowAccessToFrame(exec, thisObject->wrapped().frame(), ThrowSecurityError))
         return false;
     return Base::deletePropertyByIndex(thisObject, exec, propertyName);
 }