Attachment 343562 Details for Bug 186090 – Work-in-progress patch

[patch] Work-in-progress patch

WIP_062518.patch (text/plain), 8.85 KB, created by Daniel Bates on 2018-06-25 17:12:28 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2018-06-25 17:12:28 PDT

Size: 8.85 KB

patch

obsolete

>From cd1d65596534665812fe46277fb4762704c76887 Mon Sep 17 00:00:00 2001
>From: Daniel Bates <dabates@apple.com>
>Date: Mon, 25 Jun 2018 17:11:28 -0700
>Subject: [PATCH] Work-in-progress for Bug 186090
>
>---
> ...e-origin-x-frame-options-page-allow-expected.txt |  8 ++++++++
> ...rent-same-origin-x-frame-options-page-allow.html | 10 ++++++++++
> ...to-x-frame-options-parent-same-origin-allow.html | 13 +++++++++++++
> .../x-frame-options-parent-same-origin-allow.cgi    | 10 ++++++++++
> Source/WebCore/page/csp/ContentSecurityPolicy.cpp   |  3 +--
> .../page/csp/ContentSecurityPolicyDirectiveList.cpp |  7 +++----
> .../WebKit/NetworkProcess/NetworkResourceLoader.cpp |  3 ++-
> .../WebKit/NetworkProcess/NetworkResourceLoader.h   |  2 +-
> .../WebKit/WebProcess/Network/WebLoaderStrategy.cpp |  2 +-
> 9 files changed, 49 insertions(+), 9 deletions(-)
> create mode 100644 LayoutTests/http/tests/security/XFrameOptions/cross-origin-iframe-post-form-to-parent-same-origin-x-frame-options-page-allow-expected.txt
> create mode 100644 LayoutTests/http/tests/security/XFrameOptions/cross-origin-iframe-post-form-to-parent-same-origin-x-frame-options-page-allow.html
> create mode 100644 LayoutTests/http/tests/security/XFrameOptions/resources/post-form-to-x-frame-options-parent-same-origin-allow.html
>
>diff --git a/LayoutTests/http/tests/security/XFrameOptions/cross-origin-iframe-post-form-to-parent-same-origin-x-frame-options-page-allow-expected.txt b/LayoutTests/http/tests/security/XFrameOptions/cross-origin-iframe-post-form-to-parent-same-origin-x-frame-options-page-allow-expected.txt
>new file mode 100644
>index 00000000000..e35e4d350db
>--- /dev/null
>+++ b/LayoutTests/http/tests/security/XFrameOptions/cross-origin-iframe-post-form-to-parent-same-origin-x-frame-options-page-allow-expected.txt
>@@ -0,0 +1,8 @@
>+There should be content in the iframe below
>+
>+
>+
>+--------
>+Frame: ''
>+--------
>+PASS: This should show up as the parent is in the same origin.
>diff --git a/LayoutTests/http/tests/security/XFrameOptions/cross-origin-iframe-post-form-to-parent-same-origin-x-frame-options-page-allow.html b/LayoutTests/http/tests/security/XFrameOptions/cross-origin-iframe-post-form-to-parent-same-origin-x-frame-options-page-allow.html
>new file mode 100644
>index 00000000000..c965b980aa5
>--- /dev/null
>+++ b/LayoutTests/http/tests/security/XFrameOptions/cross-origin-iframe-post-form-to-parent-same-origin-x-frame-options-page-allow.html
>@@ -0,0 +1,10 @@
>+<script>
>+    if (window.testRunner) {
>+        testRunner.dumpAsText();
>+        testRunner.dumpChildFramesAsText();
>+        testRunner.waitUntilDone();
>+    }
>+</script>
>+
>+<p>There should be content in the iframe below</p>
>+<iframe style="width:500px; height:500px" src="http://localhost:8000/security/XFrameOptions/resources/post-form-to-x-frame-options-parent-same-origin-allow.html"></iframe>
>diff --git a/LayoutTests/http/tests/security/XFrameOptions/resources/post-form-to-x-frame-options-parent-same-origin-allow.html b/LayoutTests/http/tests/security/XFrameOptions/resources/post-form-to-x-frame-options-parent-same-origin-allow.html
>new file mode 100644
>index 00000000000..a27e5a72ce8
>--- /dev/null
>+++ b/LayoutTests/http/tests/security/XFrameOptions/resources/post-form-to-x-frame-options-parent-same-origin-allow.html
>@@ -0,0 +1,13 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<p>Posting form...</p>
>+<form action="http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-parent-same-origin-allow.cgi" method="POST">
>+    <input type="hidden" name="notifyDone" value="1">
>+    <input type="submit" name="Submit">
>+</form>
>+<script>
>+document.forms[0].submit();
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-parent-same-origin-allow.cgi b/LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-parent-same-origin-allow.cgi
>index 1906530fab1..94cfb303a85 100755
>--- a/LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-parent-same-origin-allow.cgi
>+++ b/LayoutTests/http/tests/security/XFrameOptions/resources/x-frame-options-parent-same-origin-allow.cgi
>@@ -1,8 +1,18 @@
> #!/usr/bin/perl -wT
> use strict;
>+use CGI;
>+
>+my $cgi = new CGI;
> 
> print "Content-Type: text/html\n";
> print "Cache-Control: no-cache, no-store\n";
> print "X-FRAME-OPTIONS: sameorigin\n\n";
> 
> print "<p>PASS: This should show up as the parent is in the same origin.</p>\n";
>+
>+if ($cgi->param("notifyDone")) {
>+    print "<script>\n";
>+    print "if (window.testRunner)\n";
>+    print "    testRunner.notifyDone();\n";
>+    print "</script>\n";
>+}
>diff --git a/Source/WebCore/page/csp/ContentSecurityPolicy.cpp b/Source/WebCore/page/csp/ContentSecurityPolicy.cpp
>index 6d226ea6a3c..b136371aa0d 100644
>--- a/Source/WebCore/page/csp/ContentSecurityPolicy.cpp
>+++ b/Source/WebCore/page/csp/ContentSecurityPolicy.cpp
>@@ -493,8 +493,7 @@ bool ContentSecurityPolicy::allowFrameAncestors(const Vector<RefPtr<SecurityOrig
> {
>     if (overrideContentSecurityPolicy)
>         return true;
>-    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!ancestorOrigins.isEmpty());
>-    bool isTopLevelFrame = ancestorOrigins.size() == 1;
>+    bool isTopLevelFrame = ancestorOrigins.isEmpty();
>     if (isTopLevelFrame)
>         return true;
>     String sourceURL;
>diff --git a/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp b/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
>index 6423cec1028..5c1bf7d47e6 100644
>--- a/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
>+++ b/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
>@@ -97,10 +97,9 @@ static inline bool checkFrameAncestors(ContentSecurityPolicySourceListDirective*
>     if (!directive)
>         return true;
>     bool didReceiveRedirectResponse = false;
>-    auto end = ancestorOrigins.end();
>-    for (auto it = ancestorOrigins.begin() + 1; it != end; ++it) {
>-        URL origin = urlFromOrigin(*(*it));
>-        if (!origin.isValid() || !directive->allows(origin, didReceiveRedirectResponse, ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::No))
>+    for (auto& origin : ancestorOrigins) {
>+        URL originURL = urlFromOrigin(*origin);
>+        if (!originURL.isValid() || !directive->allows(originURL, didReceiveRedirectResponse, ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::No))
>             return false;
>     }
>     return true;
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>index c2a4b9537b4..904d0b188e5 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -374,7 +374,8 @@ bool NetworkResourceLoader::shouldInterruptLoadForXFrameOptions(const String& xF
>         return true;
>     case XFrameOptionsSameOrigin: {
>         auto origin = SecurityOrigin::create(url);
>-        if (!origin->isSameSchemeHostPort(*m_parameters.sourceOrigin))
>+        auto topFrameOrigin = m_parameters.frameAncestorOrigins.last();
>+        if (!origin->isSameSchemeHostPort(*topFrameOrigin))
>             return true;
>         for (auto& ancestorOrigin : m_parameters.frameAncestorOrigins) {
>             if (!origin->isSameSchemeHostPort(*ancestorOrigin))
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.h b/Source/WebKit/NetworkProcess/NetworkResourceLoader.h
>index 7925710d3ed..931f65e238d 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.h
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.h
>@@ -113,7 +113,7 @@ public:
>     void convertToDownload(DownloadID, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&);
> 
>     bool isMainResource() const { return m_parameters.request.requester() == WebCore::ResourceRequest::Requester::Main; }
>-    bool isMainFrameLoad() const { return isMainResource() && m_parameters.frameAncestorOrigins.size() == 1; }
>+    bool isMainFrameLoad() const { return isMainResource() && m_parameters.frameAncestorOrigins.isEmpty(); }
> 
>     bool isAlwaysOnLoggingAllowed() const;
> 
>diff --git a/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp b/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp
>index 15cb99b7a71..199bd4daab8 100644
>--- a/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp
>+++ b/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp
>@@ -331,7 +331,7 @@ void WebLoaderStrategy::scheduleLoadFromNetworkProcess(ResourceLoader& resourceL
> 
>     if (resourceLoader.options().mode == FetchOptions::Mode::Navigate) {
>         Vector<RefPtr<SecurityOrigin>> frameAncestorOrigins;
>-        for (auto* frame = resourceLoader.frame(); frame; frame = frame->tree().parent())
>+        for (auto* frame = resourceLoader.frame()->tree().parent(); frame; frame = frame->tree().parent())
>             frameAncestorOrigins.append(makeRefPtr(frame->document()->securityOrigin()));
>         loadParameters.frameAncestorOrigins = WTFMove(frameAncestorOrigins);
>     }
>-- 
>2.17.1 (Apple Git-110)
>

Actions: View | Formatted Diff | Diff

Attachments on bug 186090: 343562 | 343595 | 343634