X-Frame-Options: SAMEORIGIN needs to check all ancestor frames
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 12 May 2018 04:11:16 +0000 (04:11 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 12 May 2018 04:11:16 +0000 (04:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185567
<rdar://problem/40175008>

Reviewed by Brent Fulgham.

Source/WebCore:

Change the behavior of "X-Frame-Options: SAMEORIGIN" to ensure that all ancestors frames
are same-origin with the document that delivered this header. This prevents an intermediary
malicious frame from clickjacking a child frame whose document is same-origin with the top-
level frame. It also makes the behavior of X-Frame-Options in WebKit more closely match
the behavior of X-Frame-Options in other browsers, including Chrome and Firefox.

Currently a document delivered with "X-Frame-Options: SAMEORIGIN" must only be same-origin
with the top-level frame's document in order to be displayed. This prevents clickjacking by
a malicious page that embeds a page delivered with "X-Frame-Options: SAMEORIGIN". However,
it does not protect against clickjacking of the "X-Frame-Options: SAMEORIGIN" page (victim)
if embedded by an intermediate malicious iframe, say a "rogue ad", that was embedded in a
document same origin with the victim page. We should protect against such attacks.

Tests: http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow.html
       http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::shouldInterruptLoadForXFrameOptions):

Source/WebKit:

Change the behavior of "X-Frame-Options: SAMEORIGIN" to ensure that all ancestors frames
are same-origin with the document that delivered this header. This prevents an intermediary
malicious frame from clickjacking a child frame whose document is same-origin with the top-
level frame. It also makes the behavior of X-Frame-Options in WebKit more closely match
the behavior of X-Frame-Options in other browsers, including Chrome and Firefox.

Currently a document delivered with "X-Frame-Options: SAMEORIGIN" must only be same-origin
with the top-level frame's document in order to be displayed. This prevents clickjacking by
a malicious page that embeds a page delivered with "X-Frame-Options: SAMEORIGIN". However,
it does not protect against clickjacking of the "X-Frame-Options: SAMEORIGIN" page (victim)
if embedded by an intermediate malicious iframe, say a "rogue ad", that was embedded in a
document same origin with the victim page. We should protect against such attacks.

* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::shouldInterruptLoadForXFrameOptions):

LayoutTests:

Add tests to ensure that "X-Frame-Options: SAMEORIGIN" checks ancestor frames.

* http/tests/cookies/same-site/fetch-after-navigating-iframe-in-cross-origin-page.html:
* http/tests/cookies/same-site/fetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.html:
* http/tests/cookies/same-site/fetch-in-cross-origin-iframe.html:
* http/tests/resources/echo-iframe-src.php: Copied from LayoutTests/http/tests/cookies/same-site/resources/echo-iframe-src.php.
* http/tests/security/XFrameOptions/resources/x-frame-options-ancestors-same-origin-deny.html: Added.
* http/tests/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-allow.cgi: Added.
* http/tests/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-deny.cgi: Added.
* http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow-expected.txt: Added.
* http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow.html: Added.
* http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny-expected.txt: Added.
* http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny.html: Renamed from LayoutTests/http/tests/cookies/same-site/resources/echo-iframe-src.php.

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/cookies/same-site/fetch-after-navigating-iframe-in-cross-origin-page.html
LayoutTests/http/tests/cookies/same-site/fetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.html
LayoutTests/http/tests/cookies/same-site/fetch-in-cross-origin-iframe.html
LayoutTests/http/tests/resources/echo-iframe-src.php [moved from LayoutTests/http/tests/cookies/same-site/resources/echo-iframe-src.php with 100% similarity]
LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-ancestors-same-origin-deny.html [new file with mode: 0644]
LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-allow.cgi [new file with mode: 0755]
LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-deny.cgi [new file with mode: 0755]
LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow.html [new file with mode: 0644]
LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

index 749bac8..5d13163 100644 (file)
@@ -1,3 +1,25 @@
+2018-05-11  Daniel Bates  <dabates@apple.com>
+
+        X-Frame-Options: SAMEORIGIN needs to check all ancestor frames
+        https://bugs.webkit.org/show_bug.cgi?id=185567
+        <rdar://problem/40175008>
+
+        Reviewed by Brent Fulgham.
+
+        Add tests to ensure that "X-Frame-Options: SAMEORIGIN" checks ancestor frames.
+
+        * http/tests/cookies/same-site/fetch-after-navigating-iframe-in-cross-origin-page.html:
+        * http/tests/cookies/same-site/fetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.html:
+        * http/tests/cookies/same-site/fetch-in-cross-origin-iframe.html:
+        * http/tests/resources/echo-iframe-src.php: Copied from LayoutTests/http/tests/cookies/same-site/resources/echo-iframe-src.php.
+        * http/tests/security/XFrameOptions/resources/x-frame-options-ancestors-same-origin-deny.html: Added.
+        * http/tests/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-allow.cgi: Added.
+        * http/tests/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-deny.cgi: Added.
+        * http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow-expected.txt: Added.
+        * http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow.html: Added.
+        * http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny-expected.txt: Added.
+        * http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny.html: Renamed from LayoutTests/http/tests/cookies/same-site/resources/echo-iframe-src.php.
+
 2018-05-11  Nan Wang  <n_wang@apple.com>
 
         AX: In role=dialog elements with aria-modal=true VoiceOver iOS/macOS can't manually focus or read dialog paragraph description text inside the modal.
index cb2e520..b902fb2 100644 (file)
@@ -11,7 +11,7 @@ async function runTest()
     await setCookie("implicit-strict", "6", {"SameSite": null, "Max-Age": 100, "path": "/"});
     await setCookie("strict-because-invalid-SameSite-value", "6", {"SameSite": "invalid", "Max-Age": 100, "path": "/"});
     await setCookie("lax", "6", {"SameSite": "Lax", "Max-Age": 100, "path": "/"});
-    window.location.href = "http://localhost:8000/cookies/same-site/resources/echo-iframe-src.php?src=http%3A//127.0.0.1%3A8000/cookies/same-site/resources/click-hyperlink.php%3Fhref%3Dfetch-after-navigating-iframe-in-cross-origin-page.php";
+    window.location.href = "http://localhost:8000/resources/echo-iframe-src.php?src=http%3A//127.0.0.1%3A8000/cookies/same-site/resources/click-hyperlink.php%3Fhref%3Dfetch-after-navigating-iframe-in-cross-origin-page.php";
 }
 runTest();
 </script>
index 66143b8..824925e 100644 (file)
@@ -11,7 +11,7 @@ async function runTest()
     await setCookie("implicit-strict", "4", {"SameSite": null, "Max-Age": 100, "path": "/"});
     await setCookie("strict-because-invalid-SameSite-value", "4", {"SameSite": "invalid", "Max-Age": 100, "path": "/"});
     await setCookie("lax", "4", {"SameSite": "Lax", "Max-Age": 100, "path": "/"});
-    window.location.href = "http://localhost:8000/cookies/same-site/resources/echo-iframe-src.php?src=http%3A//127.0.0.1%3A8000/cookies/same-site/resources/click-hyperlink.php%3Fhref%3Dfetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.php%26target%3D_top";
+    window.location.href = "http://localhost:8000/resources/echo-iframe-src.php?src=http%3A//127.0.0.1%3A8000/cookies/same-site/resources/click-hyperlink.php%3Fhref%3Dfetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.php%26target%3D_top";
 }
 runTest();
 </script>
index 8dc9aa6..8a1c690 100644 (file)
@@ -11,7 +11,7 @@ async function runTest()
     await setCookie("implicit-strict", "3", {"SameSite": null, "Max-Age": 100, "path": "/"});
     await setCookie("strict-because-invalid-SameSite-value", "3", {"SameSite": "invalid", "Max-Age": 100, "path": "/"});
     await setCookie("lax", "3", {"SameSite": "Lax", "Max-Age": 100, "path": "/"});
-    window.location.href = "http://127.0.0.1:8000/cookies/same-site/resources/echo-iframe-src.php?src=http://localhost:8000/cookies/same-site/resources/fetch-in-cross-origin-iframe.html";
+    window.location.href = "http://127.0.0.1:8000/resources/echo-iframe-src.php?src=http://localhost:8000/cookies/same-site/resources/fetch-in-cross-origin-iframe.html";
 }
 runTest();
 </script>
diff --git a/LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-ancestors-same-origin-deny.html b/LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-ancestors-same-origin-deny.html
new file mode 100644 (file)
index 0000000..ea7fc44
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+function checkIfDone() {
+    try {
+        var url = document.querySelector("iframe").contentWindow.location.href;
+        if (url)
+            console.log("FAIL: Could read contentWindow.location.href");
+        else
+            throw null;
+    } catch (e) {
+        if (e)
+            console.log(e);
+        console.log("PASS: Could not read contentWindow.location.href");
+    }
+    testRunner.notifyDone();
+}
+</script>
+</head>
+<body>
+<iframe src="http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-deny.cgi" onload="checkIfDone()"></iframe>
+</body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-allow.cgi b/LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-allow.cgi
new file mode 100755 (executable)
index 0000000..c202015
--- /dev/null
@@ -0,0 +1,14 @@
+#!/usr/bin/perl -wT
+use strict;
+
+print "Content-Type: text/html\n";
+print "Cache-Control: no-cache, no-store\n";
+print "X-FRAME-OPTIONS: sameorigin\n\n";
+
+print <<"EOF";
+<p>PASS: This should show up as all frame ancestors are same origin with this page.</p>
+<script>
+if (window.testRunner)
+    testRunner.notifyDone();
+</script>
+EOF
diff --git a/LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-deny.cgi b/LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-deny.cgi
new file mode 100755 (executable)
index 0000000..2b09700
--- /dev/null
@@ -0,0 +1,8 @@
+#!/usr/bin/perl -wT
+use strict;
+
+print "Content-Type: text/html\n";
+print "Cache-Control: no-cache, no-store\n";
+print "X-FRAME-OPTIONS: sameorigin\n\n";
+
+print "<p>FAIL: This should not show up as one or more frame ancestors are not same origin with this page.</p>\n";
diff --git a/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow-expected.txt b/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow-expected.txt
new file mode 100644 (file)
index 0000000..4a59226
--- /dev/null
@@ -0,0 +1,11 @@
+
+
+--------
+Frame: '<!--frame1-->'
+--------
+
+
+--------
+Frame: '<!--frame2-->'
+--------
+PASS: This should show up as all frame ancestors are same origin with this page.
diff --git a/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow.html b/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow.html
new file mode 100644 (file)
index 0000000..528dfd9
--- /dev/null
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.dumpChildFramesAsText();
+    testRunner.waitUntilDone();
+}
+</script>
+</head>
+<head>
+<iframe src="http://127.0.0.1:8000/resources/echo-iframe-src.php?src=http%3A//127.0.0.1%3A8000/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-allow.cgi"></iframe>
+</head>
+</html>
diff --git a/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny-expected.txt b/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny-expected.txt
new file mode 100644 (file)
index 0000000..d58ace4
--- /dev/null
@@ -0,0 +1,14 @@
+CONSOLE MESSAGE: Refused to display 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-frame-ancestors-same-origin-deny.cgi' in a frame because it set 'X-Frame-Options' to 'sameorigin'.
+CONSOLE MESSAGE: line 14: SecurityError: Sandbox access violation: Blocked a frame at "http://localhost:8000" from accessing a cross-origin frame.  The frame being accessed is sandboxed and lacks the "allow-same-origin" flag.
+CONSOLE MESSAGE: line 15: PASS: Could not read contentWindow.location.href
+
+
+--------
+Frame: '<!--frame1-->'
+--------
+
+
+--------
+Frame: '<!--frame2-->'
+--------
+
diff --git a/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny.html b/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny.html
new file mode 100644 (file)
index 0000000..0e5095c
--- /dev/null
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.dumpChildFramesAsText();
+    testRunner.waitUntilDone();
+}
+</script>
+</head>
+<head>
+<iframe src="http://localhost:8000/security/XFrameOptions/resources/x-frame-options-ancestors-same-origin-deny.html"></iframe>
+</head>
+</html>
index efb1d03..17d5abc 100644 (file)
@@ -1,5 +1,32 @@
 2018-05-11  Daniel Bates  <dabates@apple.com>
 
+        X-Frame-Options: SAMEORIGIN needs to check all ancestor frames
+        https://bugs.webkit.org/show_bug.cgi?id=185567
+        <rdar://problem/40175008>
+
+        Reviewed by Brent Fulgham.
+
+        Change the behavior of "X-Frame-Options: SAMEORIGIN" to ensure that all ancestors frames
+        are same-origin with the document that delivered this header. This prevents an intermediary
+        malicious frame from clickjacking a child frame whose document is same-origin with the top-
+        level frame. It also makes the behavior of X-Frame-Options in WebKit more closely match
+        the behavior of X-Frame-Options in other browsers, including Chrome and Firefox.
+        
+        Currently a document delivered with "X-Frame-Options: SAMEORIGIN" must only be same-origin
+        with the top-level frame's document in order to be displayed. This prevents clickjacking by
+        a malicious page that embeds a page delivered with "X-Frame-Options: SAMEORIGIN". However,
+        it does not protect against clickjacking of the "X-Frame-Options: SAMEORIGIN" page (victim)
+        if embedded by an intermediate malicious iframe, say a "rogue ad", that was embedded in a
+        document same origin with the victim page. We should protect against such attacks. 
+
+        Tests: http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-allow.html
+               http/tests/security/XFrameOptions/x-frame-options-ancestors-same-origin-deny.html
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::shouldInterruptLoadForXFrameOptions):
+
+2018-05-11  Daniel Bates  <dabates@apple.com>
+
         [iOS] Text decoration of dragged content does not paint with opacity
         https://bugs.webkit.org/show_bug.cgi?id=185551
         <rdar://problem/40166867>
index d7711c3..a204b4f 100644 (file)
@@ -3420,7 +3420,7 @@ bool FrameLoader::shouldInterruptLoadForXFrameOptions(const String& content, con
             return true;
         for (Frame* frame = m_frame.tree().parent(); frame; frame = frame->tree().parent()) {
             if (!origin->isSameSchemeHostPort(frame->document()->securityOrigin()))
-                break;
+                return true;
         }
         return false;
     }
index 66bb43c..f2ec028 100644 (file)
@@ -1,3 +1,27 @@
+2018-05-11  Daniel Bates  <dabates@apple.com>
+
+        X-Frame-Options: SAMEORIGIN needs to check all ancestor frames
+        https://bugs.webkit.org/show_bug.cgi?id=185567
+        <rdar://problem/40175008>
+
+        Reviewed by Brent Fulgham.
+
+        Change the behavior of "X-Frame-Options: SAMEORIGIN" to ensure that all ancestors frames
+        are same-origin with the document that delivered this header. This prevents an intermediary
+        malicious frame from clickjacking a child frame whose document is same-origin with the top-
+        level frame. It also makes the behavior of X-Frame-Options in WebKit more closely match
+        the behavior of X-Frame-Options in other browsers, including Chrome and Firefox.
+        
+        Currently a document delivered with "X-Frame-Options: SAMEORIGIN" must only be same-origin
+        with the top-level frame's document in order to be displayed. This prevents clickjacking by
+        a malicious page that embeds a page delivered with "X-Frame-Options: SAMEORIGIN". However,
+        it does not protect against clickjacking of the "X-Frame-Options: SAMEORIGIN" page (victim)
+        if embedded by an intermediate malicious iframe, say a "rogue ad", that was embedded in a
+        document same origin with the victim page. We should protect against such attacks.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::shouldInterruptLoadForXFrameOptions):
+
 2018-05-11  Dean Jackson  <dino@apple.com>
 
         WKWebViewContentProvider should know what MIME type it was created to handle
index 696c9af..8915fbb 100644 (file)
@@ -414,8 +414,16 @@ bool NetworkResourceLoader::shouldInterruptLoadForXFrameOptions(const String& xF
         return false;
     case XFrameOptionsDeny:
         return true;
-    case XFrameOptionsSameOrigin:
-        return !SecurityOrigin::create(url)->isSameSchemeHostPort(*m_parameters.sourceOrigin);
+    case XFrameOptionsSameOrigin: {
+        auto origin = SecurityOrigin::create(url);
+        if (!origin->isSameSchemeHostPort(*m_parameters.sourceOrigin))
+            return true;
+        for (auto& ancestorOrigin : m_parameters.frameAncestorOrigins) {
+            if (!origin->isSameSchemeHostPort(*ancestorOrigin))
+                return true;
+        }
+        return false;
+    }
     case XFrameOptionsConflict: {
         String errorMessage = "Multiple 'X-Frame-Options' headers with conflicting values ('" + xFrameOptions + "') encountered when loading '" + url.stringCenterEllipsizedToLength() + "'. Falling back to 'DENY'.";
         send(Messages::WebPage::AddConsoleMessage { m_parameters.webFrameID,  MessageSource::JS, MessageLevel::Error, errorMessage, identifier() }, m_parameters.webPageID);