Title: [273920] branches/safari-611.1.21.1-branch
Revision
273920
Author
repst...@apple.com
Date
2021-03-04 15:04:08 -0800 (Thu, 04 Mar 2021)

Log Message

Cherry-pick r273901. rdar://problem/75058990

    window proxy of detached iframe doesn't respect updates to global values
    https://bugs.webkit.org/show_bug.cgi?id=206445

    Reviewed by Chris Dumez.

    Source/WebCore:

    According to the html spec the frame should only be needing for
    COOP access violation reporting, which we don't support. This
    patch removes our old behavior of blocking stores to windows that
    have been detached.

    I also removed some stale caching code from
    getOwnPropertySlotByIndex since it's only accessed once now.

    * bindings/js/JSDOMWindowCustom.cpp:
    (WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
    (WebCore::JSDOMWindow::doPutPropertySecurityCheck):
    (WebCore::JSDOMWindow::put):
    (WebCore::JSDOMWindow::putByIndex):

    LayoutTests:

    * fast/frames/iframe-detached-window-still-writable-eval-expected.txt: Added.
    * fast/frames/iframe-detached-window-still-writable-eval.html: Added.
    * fast/frames/iframe-detached-window-still-writable-expected.txt: Added.
    * fast/frames/iframe-detached-window-still-writable.html: Added.
    * http/tests/dom/cross-origin-detached-window-properties-expected.txt:
    * http/tests/dom/cross-origin-detached-window-properties.html:

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@273901 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Added Paths

Diff

Modified: branches/safari-611.1.21.1-branch/LayoutTests/ChangeLog (273919 => 273920)


--- branches/safari-611.1.21.1-branch/LayoutTests/ChangeLog	2021-03-04 22:51:25 UTC (rev 273919)
+++ branches/safari-611.1.21.1-branch/LayoutTests/ChangeLog	2021-03-04 23:04:08 UTC (rev 273920)
@@ -1,3 +1,54 @@
+2021-03-04  Alan Coon  <alanc...@apple.com>
+
+        Cherry-pick r273901. rdar://problem/75058990
+
+    window proxy of detached iframe doesn't respect updates to global values
+    https://bugs.webkit.org/show_bug.cgi?id=206445
+    
+    Reviewed by Chris Dumez.
+    
+    Source/WebCore:
+    
+    According to the html spec the frame should only be needing for
+    COOP access violation reporting, which we don't support. This
+    patch removes our old behavior of blocking stores to windows that
+    have been detached.
+    
+    I also removed some stale caching code from
+    getOwnPropertySlotByIndex since it's only accessed once now.
+    
+    * bindings/js/JSDOMWindowCustom.cpp:
+    (WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
+    (WebCore::JSDOMWindow::doPutPropertySecurityCheck):
+    (WebCore::JSDOMWindow::put):
+    (WebCore::JSDOMWindow::putByIndex):
+    
+    LayoutTests:
+    
+    * fast/frames/iframe-detached-window-still-writable-eval-expected.txt: Added.
+    * fast/frames/iframe-detached-window-still-writable-eval.html: Added.
+    * fast/frames/iframe-detached-window-still-writable-expected.txt: Added.
+    * fast/frames/iframe-detached-window-still-writable.html: Added.
+    * http/tests/dom/cross-origin-detached-window-properties-expected.txt:
+    * http/tests/dom/cross-origin-detached-window-properties.html:
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@273901 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-03-04  Keith Miller  <keith_mil...@apple.com>
+
+            window proxy of detached iframe doesn't respect updates to global values
+            https://bugs.webkit.org/show_bug.cgi?id=206445
+
+            Reviewed by Chris Dumez.
+
+            * fast/frames/iframe-detached-window-still-writable-eval-expected.txt: Added.
+            * fast/frames/iframe-detached-window-still-writable-eval.html: Added.
+            * fast/frames/iframe-detached-window-still-writable-expected.txt: Added.
+            * fast/frames/iframe-detached-window-still-writable.html: Added.
+            * http/tests/dom/cross-origin-detached-window-properties-expected.txt:
+            * http/tests/dom/cross-origin-detached-window-properties.html:
+
 2021-03-03  Ruben Turcios  <rubent...@apple.com>
 
         Cherry-pick r273764. rdar://problem/74992878

Added: branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable-eval-expected.txt (0 => 273920)


--- branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable-eval-expected.txt	                        (rev 0)
+++ branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable-eval-expected.txt	2021-03-04 23:04:08 UTC (rev 273920)
@@ -0,0 +1,13 @@
+Verify that scripts running in a detached window can still modify global properties
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS () => foo is 2
+PASS () => globalThis[0] is 2
+PASS iframeContentWindow.foo is 2
+PASS iframeContentWindow[0] is 2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable-eval.html (0 => 273920)


--- branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable-eval.html	                        (rev 0)
+++ branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable-eval.html	2021-03-04 23:04:08 UTC (rev 273920)
@@ -0,0 +1,24 @@
+<html>
+<body>
+<script src=""
+<script>
+    description("Verify that scripts running in a detached window can still modify global properties");
+    const iframe = document.createElement('iframe');
+    document.body.appendChild(iframe);
+    const iframeContentWindow = iframe.contentWindow;
+    const iframeEval = iframeContentWindow.eval;
+    iframeContentWindow.foo = 1;
+    iframeContentWindow[0] = 1;
+    iframeContentWindow.shouldBe = shouldBe;
+    iframe.remove();
+    iframeEval(`
+        foo++;
+        globalThis[0]++;
+        shouldBe(() => foo, "2");
+        shouldBe(() => globalThis[0], "2");
+    `);
+    shouldBe("iframeContentWindow.foo", "2");
+    shouldBe("iframeContentWindow[0]", "2");
+</script>
+</body>
+</html>
\ No newline at end of file

Added: branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable-expected.txt (0 => 273920)


--- branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable-expected.txt	                        (rev 0)
+++ branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable-expected.txt	2021-03-04 23:04:08 UTC (rev 273920)
@@ -0,0 +1,10 @@
+Verify that detached windows are still writable
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS iframeContentWindow.foo is 2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable.html (0 => 273920)


--- branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable.html	                        (rev 0)
+++ branches/safari-611.1.21.1-branch/LayoutTests/fast/frames/iframe-detached-window-still-writable.html	2021-03-04 23:04:08 UTC (rev 273920)
@@ -0,0 +1,15 @@
+<html>
+<body>
+<script src=""
+<script>
+    description("Verify that detached windows are still writable");
+    const iframe = document.createElement('iframe');
+    document.body.appendChild(iframe);
+    const iframeContentWindow = iframe.contentWindow;
+    iframeContentWindow.foo = 1;
+    iframe.remove();
+    iframeContentWindow.foo++;
+    shouldBe("iframeContentWindow.foo", "2");
+</script>
+</body>
+</html>
\ No newline at end of file

Modified: branches/safari-611.1.21.1-branch/LayoutTests/http/tests/dom/cross-origin-detached-window-properties-expected.txt (273919 => 273920)


--- branches/safari-611.1.21.1-branch/LayoutTests/http/tests/dom/cross-origin-detached-window-properties-expected.txt	2021-03-04 22:51:25 UTC (rev 273919)
+++ branches/safari-611.1.21.1-branch/LayoutTests/http/tests/dom/cross-origin-detached-window-properties-expected.txt	2021-03-04 23:04:08 UTC (rev 273920)
@@ -32,6 +32,8 @@
 PASS w.navigator threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
 PASS w.performance threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
 PASS w.foo threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
+PASS w.foo = 1 threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
+PASS w[0] = 1 threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
 PASS w.location.foo threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
 
 * After GC
@@ -63,6 +65,8 @@
 PASS w.navigator threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
 PASS w.performance threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
 PASS w.foo threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
+PASS w.foo = 1 threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
+PASS w[0] = 1 threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
 PASS w.location.foo threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
 PASS successfullyParsed is true
 

Modified: branches/safari-611.1.21.1-branch/LayoutTests/http/tests/dom/cross-origin-detached-window-properties.html (273919 => 273920)


--- branches/safari-611.1.21.1-branch/LayoutTests/http/tests/dom/cross-origin-detached-window-properties.html	2021-03-04 22:51:25 UTC (rev 273919)
+++ branches/safari-611.1.21.1-branch/LayoutTests/http/tests/dom/cross-origin-detached-window-properties.html	2021-03-04 23:04:08 UTC (rev 273920)
@@ -48,6 +48,8 @@
     shouldThrowErrorName("w.performance", "SecurityError");
 
     shouldThrowErrorName("w.foo", "SecurityError");
+    shouldThrowErrorName("w.foo = 1", "SecurityError");
+    shouldThrowErrorName("w[0] = 1", "SecurityError");
     shouldThrowErrorName("w.location.foo", "SecurityError");
 }
 

Modified: branches/safari-611.1.21.1-branch/Source/WebCore/ChangeLog (273919 => 273920)


--- branches/safari-611.1.21.1-branch/Source/WebCore/ChangeLog	2021-03-04 22:51:25 UTC (rev 273919)
+++ branches/safari-611.1.21.1-branch/Source/WebCore/ChangeLog	2021-03-04 23:04:08 UTC (rev 273920)
@@ -1,5 +1,63 @@
 2021-03-04  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r273901. rdar://problem/75058990
+
+    window proxy of detached iframe doesn't respect updates to global values
+    https://bugs.webkit.org/show_bug.cgi?id=206445
+    
+    Reviewed by Chris Dumez.
+    
+    Source/WebCore:
+    
+    According to the html spec the frame should only be needing for
+    COOP access violation reporting, which we don't support. This
+    patch removes our old behavior of blocking stores to windows that
+    have been detached.
+    
+    I also removed some stale caching code from
+    getOwnPropertySlotByIndex since it's only accessed once now.
+    
+    * bindings/js/JSDOMWindowCustom.cpp:
+    (WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
+    (WebCore::JSDOMWindow::doPutPropertySecurityCheck):
+    (WebCore::JSDOMWindow::put):
+    (WebCore::JSDOMWindow::putByIndex):
+    
+    LayoutTests:
+    
+    * fast/frames/iframe-detached-window-still-writable-eval-expected.txt: Added.
+    * fast/frames/iframe-detached-window-still-writable-eval.html: Added.
+    * fast/frames/iframe-detached-window-still-writable-expected.txt: Added.
+    * fast/frames/iframe-detached-window-still-writable.html: Added.
+    * http/tests/dom/cross-origin-detached-window-properties-expected.txt:
+    * http/tests/dom/cross-origin-detached-window-properties.html:
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@273901 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-03-04  Keith Miller  <keith_mil...@apple.com>
+
+            window proxy of detached iframe doesn't respect updates to global values
+            https://bugs.webkit.org/show_bug.cgi?id=206445
+
+            Reviewed by Chris Dumez.
+
+            According to the html spec the frame should only be needing for
+            COOP access violation reporting, which we don't support. This
+            patch removes our old behavior of blocking stores to windows that
+            have been detached.
+
+            I also removed some stale caching code from
+            getOwnPropertySlotByIndex since it's only accessed once now.
+
+            * bindings/js/JSDOMWindowCustom.cpp:
+            (WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
+            (WebCore::JSDOMWindow::doPutPropertySecurityCheck):
+            (WebCore::JSDOMWindow::put):
+            (WebCore::JSDOMWindow::putByIndex):
+
+2021-03-04  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r273842. rdar://problem/75049285
 
     Crash under SubresourceLoader::notifyDone()

Modified: branches/safari-611.1.21.1-branch/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (273919 => 273920)


--- branches/safari-611.1.21.1-branch/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2021-03-04 22:51:25 UTC (rev 273919)
+++ branches/safari-611.1.21.1-branch/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2021-03-04 23:04:08 UTC (rev 273920)
@@ -288,14 +288,6 @@
     // Indexed getters take precendence over regular properties, so caching would be invalid.
     slot.disableCaching();
 
-    String errorMessage;
-    Optional<bool> cachedIsCrossOriginAccess;
-    auto isCrossOriginAccess = [&] {
-        if (!cachedIsCrossOriginAccess)
-            cachedIsCrossOriginAccess = !BindingSecurity::shouldAllowAccessToDOMWindow(*lexicalGlobalObject, window, errorMessage);
-        return *cachedIsCrossOriginAccess;
-    };
-
     // (1) First, indexed properties.
     // These are also allowed cross-origin, so come before the access check.
     if (frame && index < frame->tree().scopedChildCount()) {
@@ -304,7 +296,8 @@
     }
 
     // Hand off all cross-domain/frameless access to jsDOMWindowGetOwnPropertySlotRestrictedAccess.
-    if (isCrossOriginAccess())
+    String errorMessage;
+    if (!BindingSecurity::shouldAllowAccessToDOMWindow(*lexicalGlobalObject, window, errorMessage))
         return jsDOMWindowGetOwnPropertySlotRestrictedAccess<DOMWindowType::Local>(thisObject, window, *lexicalGlobalObject, Identifier::from(vm, index), slot, errorMessage);
 
     // (2) Regular own properties.
@@ -317,8 +310,6 @@
     auto scope = DECLARE_THROW_SCOPE(vm);
 
     auto* thisObject = jsCast<JSDOMWindow*>(cell);
-    if (!thisObject->wrapped().frame())
-        return;
 
     String errorMessage;
     if (!BindingSecurity::shouldAllowAccessToDOMWindow(*lexicalGlobalObject, thisObject->wrapped(), errorMessage)) {
@@ -336,8 +327,6 @@
     auto scope = DECLARE_THROW_SCOPE(vm);
 
     auto* thisObject = jsCast<JSDOMWindow*>(cell);
-    if (!thisObject->wrapped().frame())
-        return false;
 
     String errorMessage;
     if (!BindingSecurity::shouldAllowAccessToDOMWindow(*lexicalGlobalObject, thisObject->wrapped(), errorMessage)) {
@@ -357,9 +346,15 @@
 
 bool JSDOMWindow::putByIndex(JSCell* cell, JSGlobalObject* lexicalGlobalObject, unsigned index, JSValue value, bool shouldThrow)
 {
+    VM& vm = lexicalGlobalObject->vm();
     auto* thisObject = jsCast<JSDOMWindow*>(cell);
-    if (!thisObject->wrapped().frame() || !BindingSecurity::shouldAllowAccessToDOMWindow(lexicalGlobalObject, thisObject->wrapped()))
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    String errorMessage;
+    if (!BindingSecurity::shouldAllowAccessToDOMWindow(*lexicalGlobalObject, thisObject->wrapped(), errorMessage)) {
+        throwSecurityError(*lexicalGlobalObject, scope, errorMessage);
         return false;
+    }
     
     return Base::putByIndex(thisObject, lexicalGlobalObject, index, value, shouldThrow);
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to