Prevent cross-site top-level navigations from third-party iframes
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 21:28:53 +0000 (21:28 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 21:28:53 +0000 (21:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193076
<rdar://problem/36074736>

Reviewed by Alex Christensen.

Source/WebCore:

Prevent cross-site top-level navigations from third-party iframes if the following conditions are met:
1. Its tries to navigate the top-level page cross-site (different eTDL+1)
2. The user has never interacted with the third-party iframe or any of its subframes

This experiment's intent is to block suspicious main-frame navigations by third-party content. The feature
is behind a runtime experimental feature flag, on by default.

Tests: http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin.html
       http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation.html
       http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation.html
       http/tests/security/block-top-level-navigations-by-third-party-iframes.html

* dom/Document.cpp:
(WebCore::printNavigationErrorMessage):
(WebCore::Document::canNavigate):
(WebCore::Document::canNavigateInternal):
(WebCore::Document::isNavigationBlockedByThirdPartyIFrameRedirectBlocking):
* dom/Document.h:
* dom/UserGestureIndicator.cpp:
* page/DOMWindow.cpp:
(WebCore::DOMWindow::setLocation):
* page/DOMWindow.h:
* page/Frame.h:
* page/Location.cpp:
(WebCore::Location::replace):
(WebCore::Location::setLocation):
* page/Settings.yaml:

Source/WebKit:

Add experimental feature flag, on by default.

* Shared/WebPreferences.yaml:

LayoutTests:

Add layout test coverage.

* http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin-expected.txt: Added.
* http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin.html: Added.
* http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation-expected.txt: Added.
* http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation.html: Added.
* http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation-expected.txt: Added.
* http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation.html: Added.
* http/tests/security/block-top-level-navigations-by-third-party-iframes-expected.txt: Added.
* http/tests/security/block-top-level-navigations-by-third-party-iframes.html: Added.
* http/tests/security/resources/navigate-top-level-frame-to-failure-page.html: Added.
* http/tests/security/resources/navigate-top-level-frame-to-success-page-same-origin.html: Added.
* http/tests/security/resources/navigate-top-level-frame-to-success-page-with-previous-user-gesture.html: Added.
* http/tests/security/resources/navigate-top-level-frame-to-success-page-with-user-gesture.html: Added.
* http/tests/security/resources/should-have-loaded.html: Added.
* http/tests/security/resources/should-not-have-loaded.html: Added.

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

31 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/cookies/same-site/resources/click-hyperlink.php
LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin.html [new file with mode: 0644]
LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation.html [new file with mode: 0644]
LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation.html [new file with mode: 0644]
LayoutTests/http/tests/security/block-top-level-navigations-by-third-party-iframes-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/block-top-level-navigations-by-third-party-iframes.html [new file with mode: 0644]
LayoutTests/http/tests/security/frameNavigation/resources/iframe-that-performs-parent-navigation.html
LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-failure-page.html [new file with mode: 0644]
LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-success-page-same-origin.html [new file with mode: 0644]
LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-success-page-with-previous-user-gesture.html [new file with mode: 0644]
LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-success-page-with-user-gesture.html [new file with mode: 0644]
LayoutTests/http/tests/security/resources/should-have-loaded.html [new file with mode: 0644]
LayoutTests/http/tests/security/resources/should-not-have-loaded.html [new file with mode: 0644]
LayoutTests/http/tests/security/resources/xss-DENIED-window-open-parent-attacker.html
LayoutTests/http/tests/security/xss-DENIED-window-open-parent-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/UserGestureIndicator.cpp
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/DOMWindow.h
Source/WebCore/page/Frame.h
Source/WebCore/page/Location.cpp
Source/WebCore/page/Settings.yaml
Source/WebCore/platform/network/ResourceRequestBase.h
Source/WebKit/ChangeLog
Source/WebKit/Shared/WebPreferences.yaml

index 4ab8be1..1acdf23 100644 (file)
@@ -1,3 +1,28 @@
+2019-01-08  Chris Dumez  <cdumez@apple.com>
+
+        Prevent cross-site top-level navigations from third-party iframes
+        https://bugs.webkit.org/show_bug.cgi?id=193076
+        <rdar://problem/36074736>
+
+        Reviewed by Alex Christensen.
+
+        Add layout test coverage.
+
+        * http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin-expected.txt: Added.
+        * http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin.html: Added.
+        * http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation-expected.txt: Added.
+        * http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation.html: Added.
+        * http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation-expected.txt: Added.
+        * http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation.html: Added.
+        * http/tests/security/block-top-level-navigations-by-third-party-iframes-expected.txt: Added.
+        * http/tests/security/block-top-level-navigations-by-third-party-iframes.html: Added.
+        * http/tests/security/resources/navigate-top-level-frame-to-failure-page.html: Added.
+        * http/tests/security/resources/navigate-top-level-frame-to-success-page-same-origin.html: Added.
+        * http/tests/security/resources/navigate-top-level-frame-to-success-page-with-previous-user-gesture.html: Added.
+        * http/tests/security/resources/navigate-top-level-frame-to-success-page-with-user-gesture.html: Added.
+        * http/tests/security/resources/should-have-loaded.html: Added.
+        * http/tests/security/resources/should-not-have-loaded.html: Added.
+
 2019-01-08  Truitt Savell  <tsavell@apple.com>
 
         Revert expectation changes to pointerevents in iOS after https://trac.webkit.org/changeset/239704/webkit
index 58ab852..0b7090a 100644 (file)
@@ -13,6 +13,10 @@ if (window.testRunner)
         $targetAttribute = 'target="' . $_GET["target"] . '"';
 ?>
 <a href="<?php echo $_GET['href']; ?>" <?php echo $targetAttribute; ?>>Click</a>
-<script>document.querySelector("a").click()</script>
+<script>
+internals.withUserGesture(() => {
+    document.querySelector("a").click();
+});
+</script>
 </body>
 </html>
diff --git a/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin-expected.txt b/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin-expected.txt
new file mode 100644 (file)
index 0000000..63cb84f
--- /dev/null
@@ -0,0 +1,5 @@
+PASS Did the load
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin.html b/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin.html
new file mode 100644 (file)
index 0000000..6b4c099
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="/js-test-resources/js-test.js"></script>
+<script>
+description("Test that top-level navigations by a third-party iframe are allowed if the destination URL is same-origin");
+jsTestIsAsync = true;
+onload = () => {
+    setTimeout(() => {
+        document.getElementById('testFrame').src = "http://localhost:8000/security/resources/navigate-top-level-frame-to-success-page-same-origin.html";
+        setTimeout(() => {
+            testFailed("Navigation by subframe should not have been blocked");
+            finishJSTest();
+        }, 5000);
+    }, 10);
+}
+</script>
+<iframe id="testFrame"></iframe>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation-expected.txt b/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation-expected.txt
new file mode 100644 (file)
index 0000000..63cb84f
--- /dev/null
@@ -0,0 +1,5 @@
+PASS Did the load
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation.html b/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation.html
new file mode 100644 (file)
index 0000000..b211d19
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="/js-test-resources/js-test.js"></script>
+<script>
+description("Test that top-level navigations by a third-party iframe are allowed with a previous user gesture.");
+jsTestIsAsync = true;
+onload = () => {
+    setTimeout(() => {
+        document.getElementById('testFrame').src = "http://localhost:8000/security/resources/navigate-top-level-frame-to-success-page-with-previous-user-gesture.html";
+        setTimeout(() => {
+            testFailed("Navigation by subframe should not have been blocked");
+            finishJSTest();
+        }, 5000);
+    }, 10);
+}
+</script>
+<iframe id="testFrame"></iframe>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation-expected.txt b/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation-expected.txt
new file mode 100644 (file)
index 0000000..63cb84f
--- /dev/null
@@ -0,0 +1,5 @@
+PASS Did the load
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation.html b/LayoutTests/http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation.html
new file mode 100644 (file)
index 0000000..966cf6c
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="/js-test-resources/js-test.js"></script>
+<script>
+description("Test that top-level navigations by a third-party iframe are allowed with a user gesture.");
+jsTestIsAsync = true;
+onload = () => {
+    setTimeout(() => {
+        document.getElementById('testFrame').src = "http://localhost:8000/security/resources/navigate-top-level-frame-to-success-page-with-user-gesture.html";
+        setTimeout(() => {
+            testFailed("Navigation by subframe should not have been blocked");
+            finishJSTest();
+        }, 5000);
+    }, 10);
+}
+</script>
+<iframe id="testFrame"></iframe>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/block-top-level-navigations-by-third-party-iframes-expected.txt b/LayoutTests/http/tests/security/block-top-level-navigations-by-third-party-iframes-expected.txt
new file mode 100644 (file)
index 0000000..3d08f69
--- /dev/null
@@ -0,0 +1,16 @@
+CONSOLE MESSAGE: line 6: Unsafe JavaScript attempt to initiate navigation for frame with URL 'http://127.0.0.1:8000/security/block-top-level-navigations-by-third-party-iframes.html' from frame with URL 'http://localhost:8000/security/resources/navigate-top-level-frame-to-failure-page.html'. The frame attempting navigation of the top-level window is cross-origin and the user has never interacted with the frame.
+
+CONSOLE MESSAGE: line 6: SecurityError: The operation is insecure.
+CONSOLE MESSAGE: line 6: Unsafe JavaScript attempt to initiate navigation for frame with URL 'http://127.0.0.1:8000/security/block-top-level-navigations-by-third-party-iframes.html' from frame with URL 'http://localhost:8000/security/resources/navigate-top-level-frame-to-failure-page.html'. The frame attempting navigation of the top-level window is cross-origin and the user has never interacted with the frame.
+
+CONSOLE MESSAGE: line 6: SecurityError: The operation is insecure.
+Test blocking of suspicious top-level navigations by a third-party iframe
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS All navigations by subframes have been blocked
+PASS successfullyParsed is true
+
+TEST COMPLETE
diff --git a/LayoutTests/http/tests/security/block-top-level-navigations-by-third-party-iframes.html b/LayoutTests/http/tests/security/block-top-level-navigations-by-third-party-iframes.html
new file mode 100644 (file)
index 0000000..7946380
--- /dev/null
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="/js-test-resources/js-test.js"></script>
+<script>
+description("Test blocking of suspicious top-level navigations by a third-party iframe");
+jsTestIsAsync = true;
+onload = () => {
+    setTimeout(() => {
+        document.getElementById('testFrame').src = "http://localhost:8000/security/resources/navigate-top-level-frame-to-failure-page.html";
+        setTimeout(() => {
+            testPassed("All navigations by subframes have been blocked");
+            finishJSTest();
+        }, 100);
+    }, 10);
+}
+</script>
+<iframe src="http://localhost:8000/security/resources/navigate-top-level-frame-to-failure-page.html"></iframe>
+<iframe id="testFrame"></iframe>
+</body>
+</html>
index df20f41..aeff329 100644 (file)
@@ -11,7 +11,9 @@
 
         function performTest()
         {
-            parent.location = "http://localhost:8000/security/frameNavigation/resources/navigation-changed-iframe.html";
+            internals.withUserGesture(() => {
+                parent.location = "http://localhost:8000/security/frameNavigation/resources/navigation-changed-iframe.html";
+            });
         }
     </script>
 </head>
diff --git a/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-failure-page.html b/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-failure-page.html
new file mode 100644 (file)
index 0000000..a2c2bf9
--- /dev/null
@@ -0,0 +1,10 @@
+<html>
+<body>
+Success! The navigation was blocked
+<script>
+window.addEventListener("load", e => {
+  top.location = "http://localhost:8000/security/resources/should-not-have-loaded.html";
+});
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-success-page-same-origin.html b/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-success-page-same-origin.html
new file mode 100644 (file)
index 0000000..8e1027c
--- /dev/null
@@ -0,0 +1,10 @@
+<html>
+<body>
+Failure! The navigation should not have been blocked
+<script>
+window.addEventListener("load", e => {
+    top.location = "http://127.0.0.1:8000/security/resources/should-have-loaded.html";
+});
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-success-page-with-previous-user-gesture.html b/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-success-page-with-previous-user-gesture.html
new file mode 100644 (file)
index 0000000..e56b5cd
--- /dev/null
@@ -0,0 +1,14 @@
+<html>
+<body>
+Failure! The navigation should not have been blocked
+<script>
+window.addEventListener("load", e => {
+  internals.withUserGesture(() => {
+      setTimeout(() => {
+          top.location = "http://localhost:8000/security/resources/should-have-loaded.html";
+      }, 0);
+  });
+});
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-success-page-with-user-gesture.html b/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-success-page-with-user-gesture.html
new file mode 100644 (file)
index 0000000..fd1cfb5
--- /dev/null
@@ -0,0 +1,12 @@
+<html>
+<body>
+Failure! The navigation should not have been blocked
+<script>
+window.addEventListener("load", e => {
+  internals.withUserGesture(() => {
+      top.location = "http://localhost:8000/security/resources/should-have-loaded.html";
+  });
+});
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/resources/should-have-loaded.html b/LayoutTests/http/tests/security/resources/should-have-loaded.html
new file mode 100644 (file)
index 0000000..0d4494b
--- /dev/null
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+<script src="/js-test-resources/js-test.js"></script>
+<body>
+<script>
+jsTestIsAsync = true;
+testPassed("Did the load");
+finishJSTest();
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/resources/should-not-have-loaded.html b/LayoutTests/http/tests/security/resources/should-not-have-loaded.html
new file mode 100644 (file)
index 0000000..d7cc247
--- /dev/null
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+<script src="/js-test-resources/js-test.js"></script>
+<body>
+<script>
+jsTestIsAsync = true;
+testFailed("Should not have loaded");
+finishJSTest();
+</script>
+</body>
+</html>
index d85453b..bbcccbd 100644 (file)
@@ -1,4 +1,6 @@
 <script>
-open("javascript:alert('failed')", "_top");
-parent.postMessage("", "*");
+internals.withUserGesture(() => {
+    open("javascript:alert('failed')", "_top");
+    parent.postMessage("", "*");
+});
 </script>
index bc1f167..b3052d6 100644 (file)
@@ -1,3 +1,3 @@
-CONSOLE MESSAGE: line 2: Blocked a frame with origin "http://localhost:8080" from accessing a frame with origin "http://127.0.0.1:8000". Protocols, domains, and ports must match.
+CONSOLE MESSAGE: line 3: Blocked a frame with origin "http://localhost:8080" from accessing a frame with origin "http://127.0.0.1:8000". Protocols, domains, and ports must match.
 This test passes if there is no alert dialog.
 
index a2e2b1f..62e149d 100644 (file)
@@ -1,3 +1,39 @@
+2019-01-08  Chris Dumez  <cdumez@apple.com>
+
+        Prevent cross-site top-level navigations from third-party iframes
+        https://bugs.webkit.org/show_bug.cgi?id=193076
+        <rdar://problem/36074736>
+
+        Reviewed by Alex Christensen.
+
+        Prevent cross-site top-level navigations from third-party iframes if the following conditions are met:
+        1. Its tries to navigate the top-level page cross-site (different eTDL+1)
+        2. The user has never interacted with the third-party iframe or any of its subframes
+
+        This experiment's intent is to block suspicious main-frame navigations by third-party content. The feature
+        is behind a runtime experimental feature flag, on by default.
+
+        Tests: http/tests/security/allow-top-level-navigations-by-third-party-iframes-to-same-origin.html
+               http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-previous-user-activation.html
+               http/tests/security/allow-top-level-navigations-by-third-party-iframes-with-user-activation.html
+               http/tests/security/block-top-level-navigations-by-third-party-iframes.html
+
+        * dom/Document.cpp:
+        (WebCore::printNavigationErrorMessage):
+        (WebCore::Document::canNavigate):
+        (WebCore::Document::canNavigateInternal):
+        (WebCore::Document::isNavigationBlockedByThirdPartyIFrameRedirectBlocking):
+        * dom/Document.h:
+        * dom/UserGestureIndicator.cpp:
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::setLocation):
+        * page/DOMWindow.h:
+        * page/Frame.h:
+        * page/Location.cpp:
+        (WebCore::Location::replace):
+        (WebCore::Location::setLocation):
+        * page/Settings.yaml:
+
 2019-01-08  Alex Christensen  <achristensen@webkit.org>
 
         Stop using NetworkStorageSession in WebProcess
index f752db8..e60a0b4 100644 (file)
@@ -456,12 +456,12 @@ static bool canAccessAncestor(const SecurityOrigin& activeSecurityOrigin, Frame*
     return false;
 }
 
-static void printNavigationErrorMessage(Frame* frame, const URL& activeURL, const char* reason)
+static void printNavigationErrorMessage(Frame& frame, const URL& activeURL, const char* reason)
 {
-    String message = "Unsafe JavaScript attempt to initiate navigation for frame with URL '" + frame->document()->url().string() + "' from frame with URL '" + activeURL.string() + "'. " + reason + "\n";
+    String message = "Unsafe JavaScript attempt to initiate navigation for frame with URL '" + frame.document()->url().string() + "' from frame with URL '" + activeURL.string() + "'. " + reason + "\n";
 
     // FIXME: should we print to the console of the document performing the navigation instead?
-    frame->document()->domWindow()->printErrorMessage(message);
+    frame.document()->domWindow()->printErrorMessage(message);
 }
 
 uint64_t Document::s_globalTreeVersion = 0;
@@ -3337,7 +3337,7 @@ SocketProvider* Document::socketProvider()
     return m_socketProvider.get();
 }
     
-bool Document::canNavigate(Frame* targetFrame)
+bool Document::canNavigate(Frame* targetFrame, const URL& destinationURL)
 {
     if (!m_frame)
         return false;
@@ -3348,30 +3348,45 @@ bool Document::canNavigate(Frame* targetFrame)
     if (!targetFrame)
         return true;
 
+    if (!canNavigateInternal(*targetFrame))
+        return false;
+
+    if (isNavigationBlockedByThirdPartyIFrameRedirectBlocking(*targetFrame, destinationURL)) {
+        printNavigationErrorMessage(*targetFrame, url(), "The frame attempting navigation of the top-level window is cross-origin and the user has never interacted with the frame."_s);
+        return false;
+    }
+
+    return true;
+}
+
+bool Document::canNavigateInternal(Frame& targetFrame)
+{
+    ASSERT(m_frame);
+
     // Cases (i), (ii) and (iii) pass the tests from the specifications but might not pass the "security origin" tests.
 
     // i. A frame can navigate its top ancestor when its 'allow-top-navigation' flag is set (sometimes known as 'frame-busting').
-    if (!isSandboxed(SandboxTopNavigation) && targetFrame == &m_frame->tree().top())
+    if (!isSandboxed(SandboxTopNavigation) && &targetFrame == &m_frame->tree().top())
         return true;
 
     // ii. A frame can navigate its top ancestor when its 'allow-top-navigation-by-user-activation' flag is set and navigation is triggered by user activation.
-    if (!isSandboxed(SandboxTopNavigationByUserActivation) && UserGestureIndicator::processingUserGesture() && targetFrame == &m_frame->tree().top())
+    if (!isSandboxed(SandboxTopNavigationByUserActivation) && UserGestureIndicator::processingUserGesture() && &targetFrame == &m_frame->tree().top())
         return true;
 
     // iii. A sandboxed frame can always navigate its descendants.
-    if (isSandboxed(SandboxNavigation) && targetFrame->tree().isDescendantOf(m_frame))
+    if (isSandboxed(SandboxNavigation) && targetFrame.tree().isDescendantOf(m_frame))
         return true;
 
     // From https://html.spec.whatwg.org/multipage/browsers.html#allowed-to-navigate.
     // 1. If A is not the same browsing context as B, and A is not one of the ancestor browsing contexts of B, and B is not a top-level browsing context, and A's active document's active sandboxing
     // flag set has its sandboxed navigation browsing context flag set, then abort these steps negatively.
-    if (m_frame != targetFrame && isSandboxed(SandboxNavigation) && targetFrame->tree().parent() && !targetFrame->tree().isDescendantOf(m_frame)) {
+    if (m_frame != &targetFrame && isSandboxed(SandboxNavigation) && targetFrame.tree().parent() && !targetFrame.tree().isDescendantOf(m_frame)) {
         printNavigationErrorMessage(targetFrame, url(), "The frame attempting navigation is sandboxed, and is therefore disallowed from navigating its ancestors."_s);
         return false;
     }
 
     // 2. Otherwise, if B is a top-level browsing context, and is one of the ancestor browsing contexts of A, then:
-    if (m_frame != targetFrame && targetFrame == &m_frame->tree().top()) {
+    if (m_frame != &targetFrame && &targetFrame == &m_frame->tree().top()) {
         bool triggeredByUserActivation = UserGestureIndicator::processingUserGesture();
         // 1. If this algorithm is triggered by user activation and A's active document's active sandboxing flag set has its sandboxed top-level navigation with user activation browsing context flag set, then abort these steps negatively.
         if (triggeredByUserActivation && isSandboxed(SandboxTopNavigationByUserActivation)) {
@@ -3387,7 +3402,7 @@ bool Document::canNavigate(Frame* targetFrame)
 
     // 3. Otherwise, if B is a top-level browsing context, and is neither A nor one of the ancestor browsing contexts of A, and A's Document's active sandboxing flag set has its
     // sandboxed navigation browsing context flag set, and A is not the one permitted sandboxed navigator of B, then abort these steps negatively.
-    if (!targetFrame->tree().parent() && m_frame != targetFrame && targetFrame != &m_frame->tree().top() && isSandboxed(SandboxNavigation) && targetFrame->loader().opener() != m_frame) {
+    if (!targetFrame.tree().parent() && m_frame != &targetFrame && &targetFrame != &m_frame->tree().top() && isSandboxed(SandboxNavigation) && targetFrame.loader().opener() != m_frame) {
         printNavigationErrorMessage(targetFrame, url(), "The frame attempting navigation is sandboxed, and is not allowed to navigate this popup."_s);
         return false;
     }
@@ -3401,7 +3416,7 @@ bool Document::canNavigate(Frame* targetFrame)
     //
     // See http://www.adambarth.com/papers/2008/barth-jackson-mitchell.pdf for
     // historical information about this security check.
-    if (canAccessAncestor(securityOrigin(), targetFrame))
+    if (canAccessAncestor(securityOrigin(), &targetFrame))
         return true;
 
     // Top-level frames are easier to navigate than other frames because they
@@ -3415,11 +3430,11 @@ bool Document::canNavigate(Frame* targetFrame)
     // some way related to the frame being navigate (e.g., by the "opener"
     // and/or "parent" relation). Requiring some sort of relation prevents a
     // document from navigating arbitrary, unrelated top-level frames.
-    if (!targetFrame->tree().parent()) {
-        if (targetFrame == m_frame->loader().opener())
+    if (!targetFrame.tree().parent()) {
+        if (&targetFrame == m_frame->loader().opener())
             return true;
 
-        if (canAccessAncestor(securityOrigin(), targetFrame->loader().opener()))
+        if (canAccessAncestor(securityOrigin(), targetFrame.loader().opener()))
             return true;
     }
 
@@ -3427,6 +3442,37 @@ bool Document::canNavigate(Frame* targetFrame)
     return false;
 }
 
+// Prevent cross-site top-level redirects from third-party iframes unless the user has ever interacted with the frame.
+bool Document::isNavigationBlockedByThirdPartyIFrameRedirectBlocking(Frame& targetFrame, const URL& destinationURL)
+{
+    if (!settings().thirdPartyIframeRedirectBlockingEnabled())
+        return false;
+
+    // Only prevent top frame navigations by subframes.
+    if (m_frame == &targetFrame || &targetFrame != &m_frame->tree().top())
+        return false;
+
+    // Only prevent navigations by subframes that the user has not interacted with.
+    if (m_frame->hasHadUserInteraction())
+        return false;
+
+    // Only prevent navigations by unsandboxed iframes. Such navigations by unsandboxed iframes would have already been blocked unless
+    // "allow-top-navigation" / "allow-top-navigation-by-user-activation" was explicitly specified.
+    if (sandboxFlags() != SandboxNone)
+        return false;
+
+    // Only prevent navigations by third-party iframes.
+    if (canAccessAncestor(securityOrigin(), &targetFrame))
+        return false;
+
+    // Only prevent cross-site navigations.
+    auto* targetDocument = targetFrame.document();
+    if (targetDocument && (targetDocument->securityOrigin().canAccess(SecurityOrigin::create(destinationURL)) || registrableDomainsAreEqual(targetDocument->url(), destinationURL)))
+        return false;
+
+    return true;
+}
+
 void Document::didRemoveAllPendingStylesheet()
 {
     if (auto* parser = scriptableDocumentParser())
index 8025ef2..77f2dbd 100644 (file)
@@ -706,7 +706,7 @@ public:
 #endif
     SocketProvider* socketProvider() final;
 
-    bool canNavigate(Frame* targetFrame);
+    bool canNavigate(Frame* targetFrame, const URL& destinationURL = URL());
 
     bool usesStyleBasedEditability() const;
     void setHasElementUsingStyleBasedEditability();
@@ -1644,6 +1644,9 @@ private:
     void checkViewportDependentPictures();
     void checkAppearanceDependentPictures();
 
+    bool canNavigateInternal(Frame& targetFrame);
+    bool isNavigationBlockedByThirdPartyIFrameRedirectBlocking(Frame& targetFrame, const URL& destinationURL);
+
 #if ENABLE(INTERSECTION_OBSERVER)
     void notifyIntersectionObserversTimerFired();
 #endif
index d1685b9..f691d38 100644 (file)
@@ -27,6 +27,7 @@
 #include "UserGestureIndicator.h"
 
 #include "Document.h"
+#include "Frame.h"
 #include "ResourceLoadObserver.h"
 #include <wtf/MainThread.h>
 #include <wtf/NeverDestroyed.h>
@@ -59,6 +60,12 @@ UserGestureIndicator::UserGestureIndicator(Optional<ProcessingUserGestureState>
         if (processInteractionStyle == ProcessInteractionStyle::Immediate)
             ResourceLoadObserver::shared().logUserInteractionWithReducedTimeResolution(document->topDocument());
         document->topDocument().setUserDidInteractWithPage(true);
+        if (auto* frame = document->frame()) {
+            if (!frame->hasHadUserInteraction()) {
+                for (; frame; frame = frame->tree().parent())
+                    frame->setHasHadUserInteraction();
+            }
+        }
     }
 }
 
index 1759def..2c766f9 100644 (file)
@@ -2103,7 +2103,7 @@ void DOMWindow::finishedLoading()
     }
 }
 
-void DOMWindow::setLocation(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& urlString, SetLocationLocking locking)
+void DOMWindow::setLocation(DOMWindow& activeWindow, const URL& completedURL, SetLocationLocking locking)
 {
     if (!isCurrentlyDisplayedInFrame())
         return;
@@ -2113,15 +2113,7 @@ void DOMWindow::setLocation(DOMWindow& activeWindow, DOMWindow& firstWindow, con
         return;
 
     auto* frame = this->frame();
-    if (!activeDocument->canNavigate(frame))
-        return;
-
-    Frame* firstFrame = firstWindow.frame();
-    if (!firstFrame)
-        return;
-
-    URL completedURL = firstFrame->document()->completeURL(urlString);
-    if (completedURL.isNull())
+    if (!activeDocument->canNavigate(frame, completedURL))
         return;
 
     if (isInsecureScriptAccess(activeWindow, completedURL))
index effd3f7..8a9c80a 100644 (file)
@@ -145,7 +145,7 @@ public:
     Navigator& clientInformation() { return navigator(); }
 
     Location& location();
-    void setLocation(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& location, SetLocationLocking = LockHistoryBasedOnGestureState);
+    void setLocation(DOMWindow& activeWindow, const URL& completedURL, SetLocationLocking = LockHistoryBasedOnGestureState);
 
     DOMSelection* getSelection();
 
index 9faf243..8ad13d6 100644 (file)
@@ -172,6 +172,9 @@ public:
 
     bool documentIsBeingReplaced() const { return m_documentIsBeingReplaced; }
 
+    bool hasHadUserInteraction() const { return m_hasHadUserInteraction; }
+    void setHasHadUserInteraction() { m_hasHadUserInteraction = true; }
+
 // ======== All public functions below this point are candidates to move out of Frame into another class. ========
 
     WEBCORE_EXPORT void injectUserScripts(UserScriptInjectionTime);
@@ -348,6 +351,7 @@ private:
     bool m_documentIsBeingReplaced { false };
     unsigned m_navigationDisableCount { 0 };
     unsigned m_selfOnlyRefCount { 0 };
+    bool m_hasHadUserInteraction { false };
 
 protected:
     UniqueRef<EventHandler> m_eventHandler;
index 1640cfc..9691f59 100644 (file)
@@ -225,15 +225,25 @@ ExceptionOr<void> Location::assign(DOMWindow& activeWindow, DOMWindow& firstWind
     return setLocation(activeWindow, firstWindow, url);
 }
 
-void Location::replace(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& url)
+void Location::replace(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& urlString)
 {
     auto* frame = this->frame();
     if (!frame)
         return;
     ASSERT(frame->document());
     ASSERT(frame->document()->domWindow());
+
+    Frame* firstFrame = firstWindow.frame();
+    if (!firstFrame || !firstFrame->document())
+        return;
+
+    URL completedURL = firstFrame->document()->completeURL(urlString);
+    // FIXME: The specification says to throw a SyntaxError if the URL is not valid.
+    if (completedURL.isNull())
+        return;
+
     // We call DOMWindow::setLocation directly here because replace() always operates on the current frame.
-    frame->document()->domWindow()->setLocation(activeWindow, firstWindow, url, LockHistoryAndBackForwardList);
+    frame->document()->domWindow()->setLocation(activeWindow, completedURL, LockHistoryAndBackForwardList);
 }
 
 void Location::reload(DOMWindow& activeWindow)
@@ -264,15 +274,26 @@ void Location::reload(DOMWindow& activeWindow)
     frame->navigationScheduler().scheduleRefresh(activeDocument);
 }
 
-ExceptionOr<void> Location::setLocation(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& url)
+ExceptionOr<void> Location::setLocation(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& urlString)
 {
     auto* frame = this->frame();
     ASSERT(frame);
-    if (!activeWindow.document()->canNavigate(frame))
+
+    Frame* firstFrame = firstWindow.frame();
+    if (!firstFrame || !firstFrame->document())
+        return { };
+
+    URL completedURL = firstFrame->document()->completeURL(urlString);
+    // FIXME: The specification says to throw a SyntaxError if the URL is not valid.
+    if (completedURL.isNull())
+        return { };
+
+    if (!activeWindow.document()->canNavigate(frame, completedURL))
         return Exception { SecurityError };
+
     ASSERT(frame->document());
     ASSERT(frame->document()->domWindow());
-    frame->document()->domWindow()->setLocation(activeWindow, firstWindow, url);
+    frame->document()->domWindow()->setLocation(activeWindow, completedURL);
     return { };
 }
 
index cb29eac..2b187e1 100644 (file)
@@ -332,6 +332,9 @@ requestAnimationFrameEnabled:
 HTTPSUpgradeEnabled:
   initial: false
 
+thirdPartyIframeRedirectBlockingEnabled:
+  initial: true
+
 cookieEnabled:
   initial: true
 mediaEnabled:
index eb50727..b470274 100644 (file)
@@ -260,7 +260,10 @@ bool equalIgnoringHeaderFields(const ResourceRequestBase&, const ResourceRequest
 // FIXME: Find a better place for these functions.
 inline String toRegistrableDomain(const URL& a)
 {
-    return ResourceRequestBase::partitionName(a.host().toString());
+    auto host = a.host().toString();
+    auto registrableDomain = ResourceRequestBase::partitionName(host);
+    // Fall back to the host if we cannot determine the registrable domain.
+    return registrableDomain.isEmpty() ? host : registrableDomain;
 }
 
 inline bool registrableDomainsAreEqual(const URL& a, const URL& b)
index e32a8cb..6203b37 100644 (file)
@@ -1,3 +1,15 @@
+2019-01-08  Chris Dumez  <cdumez@apple.com>
+
+        Prevent cross-site top-level navigations from third-party iframes
+        https://bugs.webkit.org/show_bug.cgi?id=193076
+        <rdar://problem/36074736>
+
+        Reviewed by Alex Christensen.
+
+        Add experimental feature flag, on by default.
+
+        * Shared/WebPreferences.yaml:
+
 2019-01-08  Alex Christensen  <achristensen@webkit.org>
 
         Remove more use of NetworkProcess::singleton
index a640597..00a8de8 100644 (file)
@@ -42,6 +42,13 @@ HTTPSUpgradeEnabled:
    humanReadableDescription: "Automatic HTTPS upgrade for known supported sites"
    category: experimental
 
+ThirdPartyIframeRedirectBlockingEnabled:
+   type: bool
+   defaultValue: true
+   humanReadableName: "Block top-level redirects by third-party iframes"
+   humanReadableDescription: "Block top-level redirects by third-party iframes"
+   category: experimental
+
 JavaEnabled:
   type: bool
   defaultValue: false