Attachment 342780 Details for Bug 186639 – Patch

[patch] Patch

bug-186639-20180614172844.patch (text/plain), 14.93 KB, created by youenn fablet on 2018-06-14 17:28:45 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-06-14 17:28:45 PDT

Size: 14.93 KB

patch

obsolete

>Subversion Revision: 232857
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 3a8797e97a6e345f20d2fdccaefe3746bfff1fd3..d22e661c7497e1ffa032c6918ac59e45dd95ca10 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-06-14  Youenn Fablet  <youenn@apple.com>
>+
>+        Validate Cross-Origin-Resource-Policy for resources cached in the MemoryCache
>+        https://bugs.webkit.org/show_bug.cgi?id=186639
>+        <rdar://problem/41106984>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a method to check CORP.
>+        Make use of it to validate any memory cached resource.
>+        Whitelist CORP header so that it is not filtered out by Network Process.
>+
>+        Test: http/wpt/cross-origin-resource-policy/image-in-iframe-loads.html
>+
>+        * loader/CrossOriginAccessControl.cpp:
>+        (WebCore::shouldCrossOriginResourcePolicyCancelLoad):
>+        (WebCore::validateCrossOriginResourcePolicy):
>+        * loader/CrossOriginAccessControl.h:
>+        * loader/cache/CachedResourceLoader.cpp:
>+        (WebCore::CachedResourceLoader::requestResource):
>+        * platform/network/ResourceResponseBase.cpp:
>+        (WebCore::isSafeRedirectionResponseHeader):
>+        (WebCore::isSafeCrossOriginResponseHeader):
>+
> 2018-06-14  Nan Wang  <n_wang@apple.com>
> 
>         AX: Crash in AccessibilityNodeObject::textUnderElement(WebCore::AccessibilityTextUnderElementMode) const + 536
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4df90195ea7526e1729cbf16640d7bf09b6cd532..a3efd4cd4079c8f8a17e3169098b5b9d69c96c6d 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-06-14  Youenn Fablet  <youenn@apple.com>
>+
>+        Validate Cross-Origin-Resource-Policy for resources cached in the MemoryCache
>+        https://bugs.webkit.org/show_bug.cgi?id=186639
>+        <rdar://problem/41106984>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Make use of WebCore method to check CORP.
>+
>+        * NetworkProcess/NetworkLoadChecker.cpp:
>+        (WebKit::NetworkLoadChecker::validateResponse):
>+        * NetworkProcess/NetworkLoadChecker.h:
>+
> 2018-06-14  John Wilander  <wilander@apple.com>
> 
>         Resource Load Statistics: Shortcut classification for redirect to prevalent resource
>diff --git a/Source/WebCore/loader/CrossOriginAccessControl.cpp b/Source/WebCore/loader/CrossOriginAccessControl.cpp
>index 68804f2ed01d972221024e1cebdba6a1cc1d0381..1e026b63f556f2715a914e3ebb1ae4da6493d3da 100644
>--- a/Source/WebCore/loader/CrossOriginAccessControl.cpp
>+++ b/Source/WebCore/loader/CrossOriginAccessControl.cpp
>@@ -208,4 +208,34 @@ bool validatePreflightResponse(const ResourceRequest& request, const ResourceRes
>     return true;
> }
> 
>+static inline bool shouldCrossOriginResourcePolicyCancelLoad(const SecurityOrigin& origin, const ResourceResponse& response)
>+{
>+    if (origin.canRequest(response.url()))
>+        return false;
>+
>+    auto policy = parseCrossOriginResourcePolicyHeader(response.httpHeaderField(HTTPHeaderName::CrossOriginResourcePolicy));
>+    switch (policy) {
>+    case CrossOriginResourcePolicy::None:
>+    case CrossOriginResourcePolicy::Invalid:
>+        return false;
>+    case CrossOriginResourcePolicy::SameOrigin:
>+        return true;
>+    case CrossOriginResourcePolicy::SameSite: {
>+#if ENABLE(PUBLIC_SUFFIX_LIST)
>+        return origin.isUnique() || !registrableDomainsAreEqual(response.url(), ResourceRequest::partitionName(origin.host()));
>+#else
>+        return true;
>+#endif
>+    }}
>+
>+    RELEASE_ASSERT_NOT_REACHED();
>+}
>+
>+std::optional<ResourceError> validateCrossOriginResourcePolicy(const SecurityOrigin& origin, const URL& requestURL, const ResourceResponse& response)
>+{
>+    if (shouldCrossOriginResourcePolicyCancelLoad(origin, response))
>+        return ResourceError { errorDomainWebKitInternal, 0, requestURL, makeString("Cancelled load to ", response.url().stringCenterEllipsizedToLength(), " because it violates the resource's Cross-Origin-Resource-Policy response header."), ResourceError::Type::AccessControl };
>+    return std::nullopt;
>+}
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/loader/CrossOriginAccessControl.h b/Source/WebCore/loader/CrossOriginAccessControl.h
>index 8b6fb30539962ea10f9c8304d04bc9b7288e2fdd..2c6754b10f62e817cce769aa53e8d5b60b13101f 100644
>--- a/Source/WebCore/loader/CrossOriginAccessControl.h
>+++ b/Source/WebCore/loader/CrossOriginAccessControl.h
>@@ -34,6 +34,7 @@
> namespace WebCore {
> 
> class HTTPHeaderMap;
>+class ResourceError;
> class ResourceRequest;
> class ResourceResponse;
> class SecurityOrigin;
>@@ -56,4 +57,6 @@ WEBCORE_EXPORT void cleanHTTPRequestHeadersForAccessControl(ResourceRequest&, co
> WEBCORE_EXPORT bool passesAccessControlCheck(const ResourceResponse&, StoredCredentialsPolicy, SecurityOrigin&, String& errorDescription);
> WEBCORE_EXPORT bool validatePreflightResponse(const ResourceRequest&, const ResourceResponse&, StoredCredentialsPolicy, SecurityOrigin&, String& errorDescription);
> 
>+WEBCORE_EXPORT std::optional<ResourceError> validateCrossOriginResourcePolicy(const SecurityOrigin&, const URL&, const ResourceResponse&);
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/loader/cache/CachedResourceLoader.cpp b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
>index a703e5e40f2a6b78cc93bb7e37fcf21a6dbff8b1..6c6345132d9abc7a3e210d4cf65ba2b53bfea7b3 100644
>--- a/Source/WebCore/loader/cache/CachedResourceLoader.cpp
>+++ b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
>@@ -893,6 +893,10 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ
>         break;
>     case Use:
>         ASSERT(resource);
>+        if (request.options().mode == FetchOptions::Mode::NoCors) {
>+            if (auto error = validateCrossOriginResourcePolicy(*request.origin(), request.resourceRequest().url(), resource->response()))
>+                return makeUnexpected(WTFMove(*error));
>+        }
>         if (shouldUpdateCachedResourceWithCurrentRequest(*resource, request)) {
>             resource = updateCachedResourceWithCurrentRequest(*resource, WTFMove(request));
>             if (resource->status() != CachedResource::Status::Cached)
>diff --git a/Source/WebCore/platform/network/ResourceResponseBase.cpp b/Source/WebCore/platform/network/ResourceResponseBase.cpp
>index e2df22690b2583291fee3f0aa75c27897d860f3c..6c06970040222f5df9fa80aa3b7ee715614b8fdf 100644
>--- a/Source/WebCore/platform/network/ResourceResponseBase.cpp
>+++ b/Source/WebCore/platform/network/ResourceResponseBase.cpp
>@@ -339,6 +339,7 @@ static bool isSafeRedirectionResponseHeader(HTTPHeaderName name)
>         || name == HTTPHeaderName::AccessControlAllowOrigin
>         || name == HTTPHeaderName::AccessControlExposeHeaders
>         || name == HTTPHeaderName::AccessControlMaxAge
>+        || name == HTTPHeaderName::CrossOriginResourcePolicy
>         || name == HTTPHeaderName::TimingAllowOrigin;
> }
> 
>@@ -364,6 +365,7 @@ static bool isSafeCrossOriginResponseHeader(HTTPHeaderName name)
>         || name == HTTPHeaderName::ContentSecurityPolicy
>         || name == HTTPHeaderName::ContentSecurityPolicyReportOnly
>         || name == HTTPHeaderName::ContentType
>+        || name == HTTPHeaderName::CrossOriginResourcePolicy
>         || name == HTTPHeaderName::Date
>         || name == HTTPHeaderName::ETag
>         || name == HTTPHeaderName::Expires
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>index 7ab89e89a75f2120bd83d07ca662693bdd7b0c14..ed2a3ece6dac182967e67dc05c8dc4dace7ad047 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>@@ -131,29 +131,6 @@ void NetworkLoadChecker::checkRedirection(ResourceResponse& redirectResponse, Re
>     checkRequest(WTFMove(request), WTFMove(handler));
> }
> 
>-bool NetworkLoadChecker::shouldCrossOriginResourcePolicyPolicyCancelLoad(const ResourceResponse& response)
>-{
>-    if (m_origin->canRequest(response.url()))
>-        return false;
>-
>-    auto policy = parseCrossOriginResourcePolicyHeader(response.httpHeaderField(HTTPHeaderName::CrossOriginResourcePolicy));
>-    switch (policy) {
>-    case CrossOriginResourcePolicy::None:
>-    case CrossOriginResourcePolicy::Invalid:
>-        return false;
>-    case CrossOriginResourcePolicy::SameOrigin:
>-        return true;
>-    case CrossOriginResourcePolicy::SameSite: {
>-#if ENABLE(PUBLIC_SUFFIX_LIST)
>-        return m_origin->isUnique() || !registrableDomainsAreEqual(response.url(), ResourceRequest::partitionName(m_origin->host()));
>-#else
>-        return true;
>-#endif
>-    }}
>-
>-    RELEASE_ASSERT_NOT_REACHED();
>-}
>-
> ResourceError NetworkLoadChecker::validateResponse(ResourceResponse& response)
> {
>     if (m_redirectCount)
>@@ -170,8 +147,9 @@ ResourceError NetworkLoadChecker::validateResponse(ResourceResponse& response)
>     }
> 
>     if (m_options.mode == FetchOptions::Mode::NoCors) {
>-        if (shouldCrossOriginResourcePolicyPolicyCancelLoad(response))
>-            return ResourceError { errorDomainWebKitInternal, 0, m_url, makeString("Cancelled load to ", response.url().stringCenterEllipsizedToLength(), " because it violates the resource's Cross-Origin-Resource-Policy response header."), ResourceError::Type::AccessControl };
>+        if (auto error = validateCrossOriginResourcePolicy(*m_origin, m_url, response))
>+            return WTFMove(*error);
>+
>         response.setTainting(ResourceResponse::Tainting::Opaque);
>         return { };
>     }
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>index 1442639e2575a952f4e75ff4454e80a5946af839..e399881b7f679bc8608c41c53a0976f6a2c1e15e 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>@@ -108,8 +108,6 @@ private:
>     uint64_t m_webFrameID;
>     ResourceLoadIdentifier m_loadIdentifier;
> 
>-    bool shouldCrossOriginResourcePolicyPolicyCancelLoad(const WebCore::ResourceResponse&);
>-
>     WebCore::FetchOptions m_options;
>     WebCore::StoredCredentialsPolicy m_storedCredentialsPolicy;
>     PAL::SessionID m_sessionID;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ed5faf7ab56fa4583eb638c6407a8fc6e2cc63cb..9ecbf2aff2eff077a6d9408e7fe28b20885091e1 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-06-14  Youenn Fablet  <youenn@apple.com>
>+
>+        Validate Cross-Origin-Resource-Policy for resources cached in the MemoryCache
>+        https://bugs.webkit.org/show_bug.cgi?id=186639
>+        <rdar://problem/41106984>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/wpt/cross-origin-resource-policy/image-in-iframe-loads-expected.txt: Added.
>+        * http/wpt/cross-origin-resource-policy/image-in-iframe-loads.html: Added.
>+        * http/wpt/cross-origin-resource-policy/resources/iframeImage.html: Added.
>+        * http/wpt/cross-origin-resource-policy/resources/image.py:
>+        (main):
>+
> 2018-06-14  Nan Wang  <n_wang@apple.com>
> 
>         AX: Crash in AccessibilityNodeObject::textUnderElement(WebCore::AccessibilityTextUnderElementMode) const + 536
>diff --git a/LayoutTests/http/wpt/cross-origin-resource-policy/image-in-iframe-loads-expected.txt b/LayoutTests/http/wpt/cross-origin-resource-policy/image-in-iframe-loads-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..886daf8d8df4877ead9476e8b00043d3f1aa6104
>--- /dev/null
>+++ b/LayoutTests/http/wpt/cross-origin-resource-policy/image-in-iframe-loads-expected.txt
>@@ -0,0 +1,4 @@
>+  
>+
>+PASS Ensure CORP checks in case image is cached 
>+
>diff --git a/LayoutTests/http/wpt/cross-origin-resource-policy/image-in-iframe-loads.html b/LayoutTests/http/wpt/cross-origin-resource-policy/image-in-iframe-loads.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..d6f221d1949b506c0eeca49e0fabe6757641bd7e
>--- /dev/null
>+++ b/LayoutTests/http/wpt/cross-origin-resource-policy/image-in-iframe-loads.html
>@@ -0,0 +1,38 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <script src="/resources/testharness.js"></script>
>+    <script src="/resources/testharnessreport.js"></script>
>+    <script src="/common/get-host-info.sub.js"></script>
>+</head>
>+<body>
>+    <script>
>+const host = get_host_info();
>+const remoteBaseURL = host.HTTP_REMOTE_ORIGIN + window.location.pathname.replace(/\/[^\/]*$/, '/') ;
>+const localBaseURL = host.HTTP_ORIGIN + window.location.pathname.replace(/\/[^\/]*$/, '/') ;
>+
>+function with_iframe(url) {
>+  return new Promise(function(resolve) {
>+      var frame = document.createElement('iframe');
>+      frame.src = url;
>+      frame.onload = function() { resolve(frame); };
>+      document.body.appendChild(frame);
>+    });
>+}
>+
>+promise_test(async() => {
>+    let message = new Promise((resolve) => {
>+        window.addEventListener("message", (event) => { resolve(event.data) });
>+    });
>+    await with_iframe(localBaseURL + "/resources/iframeImage.html");
>+    assert_equals(await message, "ok", "loading same origin image should succeed");
>+
>+    message = new Promise((resolve) => {
>+        window.addEventListener("message", (event) => { resolve(event.data) });
>+    });
>+    await with_iframe(remoteBaseURL + "/resources/iframeImage.html");
>+    assert_equals(await message, "ko", "loading not same origin image should succeed");
>+}, "Ensure CORP checks in case image is cached");
>+    </script>
>+</body>
>+</html>
>diff --git a/LayoutTests/http/wpt/cross-origin-resource-policy/resources/iframeImage.html b/LayoutTests/http/wpt/cross-origin-resource-policy/resources/iframeImage.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..73032882f67a3fcbe61afeda9d8c32f051e84ab9
>--- /dev/null
>+++ b/LayoutTests/http/wpt/cross-origin-resource-policy/resources/iframeImage.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+    <div id="testDiv"></div>
>+    <h3>The iframe making an image load.</h3>
>+    <script src="/common/get-host-info.sub.js"></script>
>+    <script>
>+const host = get_host_info();
>+const baseURL = host.HTTP_ORIGIN + window.location.pathname.replace(/\/[^\/]*$/, '/') ;
>+const ok = true;
>+const ko = false;
>+
>+const img = new Image();
>+img.src = baseURL + "image.py?corp=same-origin&cached";
>+img.onload = () => { parent.postMessage("ok", "*") };
>+img.onerror = () => { parent.postMessage("ko", "*") };
>+testDiv.appendChild(img);
>+    </script>
>+</body>
>+</html>
>diff --git a/LayoutTests/http/wpt/cross-origin-resource-policy/resources/image.py b/LayoutTests/http/wpt/cross-origin-resource-policy/resources/image.py
>index ba6198135a2aad55767397af46dc9dce3c3ac703..e28e5b3f92c9dc1e9e6bdc7f0ebbd8ef0f7c84d6 100644
>--- a/LayoutTests/http/wpt/cross-origin-resource-policy/resources/image.py
>+++ b/LayoutTests/http/wpt/cross-origin-resource-policy/resources/image.py
>@@ -8,6 +8,9 @@ def main(request, response):
>     response.add_required_headers = False
>     response.writer.write_status(200)
> 
>+    if 'cached' in request.GET:
>+        response.writer.write_header("Cache-Control", "max-age=600000")
>+
>     if 'corp' in request.GET:
>         response.writer.write_header("cross-origin-resource-policy", request.GET['corp'])
>     if 'acao' in request.GET:

Actions: View | Formatted Diff | Diff

Attachments on bug 186639: 342780 | 342791