[webkit-changes] [280236] releases/WebKitGTK/webkit-2.32

Fri, 23 Jul 2021 03:11:58 -0700

Title: [280236] releases/WebKitGTK/webkit-2.32
Revision
280236
Author
[email protected]
Date
2021-07-23 03:11:10 -0700 (Fri, 23 Jul 2021)

Log Message

Merge r276230 - Blob URLs should use for their owner origin for CSP checks
https://bugs.webkit.org/show_bug.cgi?id=224535
<rdar://76458106>

Reviewed by Alex Christensen.

Source/WebCore:

Before the patch, we were checking blob origin directly with ancestors.
As per https://w3c.github.io/webappsec-csp/#match-url-to-source-_expression_ step 4.1,
we need to get the URL origin, which by spec is the origin of the blob creator.
We only do this for navigation loads as script loads should be kept the current way, as a cross-site scripting protection,
and to remain compatible with other browsers.

Make some refactoring to add helper routines to get origin and secure context state of blob URLs in BlobURL.
Make use of it in MixedContentChecker as a refactoring.
Make use of the helper routine in ContentSecurityPolicySource::matches to fix the bug.

Test: http/tests/security/frame-src-and-blob-download.https.html

* fileapi/BlobURL.cpp:
(WebCore::blobOwner):
(WebCore::BlobURL::getOriginURL):
(WebCore::BlobURL::isSecureBlobURL):
* fileapi/BlobURL.h:
* fileapi/ThreadableBlobRegistry.cpp:
(WebCore::isBlobURLContainsNullOrigin):
* loader/MixedContentChecker.cpp:
(WebCore::MixedContentChecker::isMixedContent):
* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::isSecure):
* page/csp/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::urlMatchesSelf const):
* page/csp/ContentSecurityPolicy.h:
* page/csp/ContentSecurityPolicySourceList.cpp:
(WebCore::ContentSecurityPolicySourceList::matches const):

LayoutTests:

* http/tests/security/frame-src-and-blob-download.https-expected.txt: Added.
* http/tests/security/frame-src-and-blob-download.https.html:
* http/tests/security/resources/frame-src-and-blob-download-frame.html: Added.
* platform/mac-wk1/TestExpectations:
* platform/win/TestExpectations:

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.32/LayoutTests/ChangeLog (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/LayoutTests/ChangeLog	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/LayoutTests/ChangeLog	2021-07-23 10:11:10 UTC (rev 280236)
@@ -1,3 +1,17 @@
+2021-04-18  Youenn Fablet  <[email protected]>
+
+        Blob URLs should use for their owner origin for CSP checks
+        https://bugs.webkit.org/show_bug.cgi?id=224535
+        <rdar://76458106>
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/security/frame-src-and-blob-download.https-expected.txt: Added.
+        * http/tests/security/frame-src-and-blob-download.https.html:
+        * http/tests/security/resources/frame-src-and-blob-download-frame.html: Added.
+        * platform/mac-wk1/TestExpectations:
+        * platform/win/TestExpectations:
+
 2021-04-20  Diego Pino Garcia  <[email protected]>
 
         [GTK][WPE] Unreviewed test gardening. Emit new port baselines after r276193.

Modified: releases/WebKitGTK/webkit-2.32/LayoutTests/platform/mac-wk1/TestExpectations (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/LayoutTests/platform/mac-wk1/TestExpectations	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/LayoutTests/platform/mac-wk1/TestExpectations	2021-07-23 10:11:10 UTC (rev 280236)
@@ -134,6 +134,7 @@
 webkit.org/b/156069 http/tests/download/anchor-download-attribute-content-disposition-no-extension-text-plain.html [ Skip ]
 webkit.org/b/156069 http/tests/security/anchor-download-octet-stream-no-extension.html [ Skip ]
 webkit.org/b/156069 http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404.html [ Skip ]
+http/tests/security/frame-src-and-blob-download.https.html [ Skip ]
 
 # Not supported on WK1
 media/no-fullscreen-when-hidden.html [ Skip ]

Modified: releases/WebKitGTK/webkit-2.32/LayoutTests/platform/win/TestExpectations (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/LayoutTests/platform/win/TestExpectations	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/LayoutTests/platform/win/TestExpectations	2021-07-23 10:11:10 UTC (rev 280236)
@@ -222,6 +222,7 @@
 webkit.org/b/29287 http/tests/local/fileapi/ [ Skip ]
 webkit.org/b/29287 http/tests/local/blob/send-hybrid-blob.html [ Skip ]
 webkit.org/b/29287 http/tests/local/formdata/ [ Skip ]
+http/tests/security/frame-src-and-blob-download.https.html [ Skip ]
 
 # Software keyboard is not supported.
 fast/events/autoscroll-with-software-keyboard.html [ Skip ]

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog	2021-07-23 10:11:10 UTC (rev 280236)
@@ -1,3 +1,40 @@
+2021-04-18  Youenn Fablet  <[email protected]>
+
+        Blob URLs should use for their owner origin for CSP checks
+        https://bugs.webkit.org/show_bug.cgi?id=224535
+        <rdar://76458106>
+
+        Reviewed by Alex Christensen.
+
+        Before the patch, we were checking blob origin directly with ancestors.
+        As per https://w3c.github.io/webappsec-csp/#match-url-to-source-_expression_ step 4.1,
+        we need to get the URL origin, which by spec is the origin of the blob creator.
+        We only do this for navigation loads as script loads should be kept the current way, as a cross-site scripting protection,
+        and to remain compatible with other browsers.
+
+        Make some refactoring to add helper routines to get origin and secure context state of blob URLs in BlobURL.
+        Make use of it in MixedContentChecker as a refactoring.
+        Make use of the helper routine in ContentSecurityPolicySource::matches to fix the bug.
+
+        Test: http/tests/security/frame-src-and-blob-download.https.html
+
+        * fileapi/BlobURL.cpp:
+        (WebCore::blobOwner):
+        (WebCore::BlobURL::getOriginURL):
+        (WebCore::BlobURL::isSecureBlobURL):
+        * fileapi/BlobURL.h:
+        * fileapi/ThreadableBlobRegistry.cpp:
+        (WebCore::isBlobURLContainsNullOrigin):
+        * loader/MixedContentChecker.cpp:
+        (WebCore::MixedContentChecker::isMixedContent):
+        * page/SecurityOrigin.cpp:
+        (WebCore::SecurityOrigin::isSecure):
+        * page/csp/ContentSecurityPolicy.cpp:
+        (WebCore::ContentSecurityPolicy::urlMatchesSelf const):
+        * page/csp/ContentSecurityPolicy.h:
+        * page/csp/ContentSecurityPolicySourceList.cpp:
+        (WebCore::ContentSecurityPolicySourceList::matches const):
+
 2021-06-16  Chris Dumez  <[email protected]>
 
         Protect Element before calling dispatchMouseEvent() on it

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/fileapi/BlobURL.cpp (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/fileapi/BlobURL.cpp	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/fileapi/BlobURL.cpp	2021-07-23 10:11:10 UTC (rev 280236)
@@ -31,9 +31,11 @@
 #include "config.h"
 
 #include "BlobURL.h"
+#include "Document.h"
+#include "SecurityOrigin.h"
+#include "ThreadableBlobRegistry.h"
 
 #include <wtf/URL.h>
-#include "SecurityOrigin.h"
 #include <wtf/UUID.h>
 #include <wtf/text/WTFString.h>
 
@@ -52,15 +54,41 @@
     return createBlobURL("blobinternal://");
 }
 
-String BlobURL::getOrigin(const URL& url)
+static const Document* blobOwner(const SecurityOrigin& blobOrigin)
 {
+    if (!isMainThread())
+        return nullptr;
+
+    for (const auto* document : Document::allDocuments()) {
+        if (&document->securityOrigin() == &blobOrigin)
+            return document;
+    }
+    return nullptr;
+}
+
+URL BlobURL::getOriginURL(const URL& url)
+{
     ASSERT(url.protocolIs(kBlobProtocol));
 
-    unsigned startIndex = url.pathStart();
-    unsigned endIndex = url.pathAfterLastSlash();
-    return url.string().substring(startIndex, endIndex - startIndex - 1);
+    if (auto blobOrigin = ThreadableBlobRegistry::getCachedOrigin(url)) {
+        if (auto* document = blobOwner(*blobOrigin))
+            return document->url();
+    }
+    return SecurityOrigin::extractInnerURL(url);
 }
 
+bool BlobURL::isSecureBlobURL(const URL& url)
+{
+    ASSERT(url.protocolIs(kBlobProtocol));
+
+    // As per https://github.com/w3c/webappsec-mixed-content/issues/41, Blob URL is secure if the document that created it is secure.
+    if (auto origin = ThreadableBlobRegistry::getCachedOrigin(url)) {
+        if (auto* document = blobOwner(*origin))
+            return document->isSecureContext();
+    }
+    return SecurityOrigin::isSecure(url);
+}
+
 URL BlobURL::createBlobURL(const String& originString)
 {
     ASSERT(!originString.isEmpty());

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/fileapi/BlobURL.h (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/fileapi/BlobURL.h	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/fileapi/BlobURL.h	2021-07-23 10:11:10 UTC (rev 280236)
@@ -49,8 +49,10 @@
 public:
     static URL createPublicURL(SecurityOrigin*);
     static URL createInternalURL();
-    static String getOrigin(const URL&);
 
+    static URL getOriginURL(const URL&);
+    static bool isSecureBlobURL(const URL&);
+
 private:
     static URL createBlobURL(const String& originString);
     BlobURL() { }

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/fileapi/ThreadableBlobRegistry.cpp (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/fileapi/ThreadableBlobRegistry.cpp	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/fileapi/ThreadableBlobRegistry.cpp	2021-07-23 10:11:10 UTC (rev 280236)
@@ -94,7 +94,9 @@
 static inline bool isBlobURLContainsNullOrigin(const URL& url)
 {
     ASSERT(url.protocolIsBlob());
-    return BlobURL::getOrigin(url) == "null";
+    unsigned startIndex = url.pathStart();
+    unsigned endIndex = url.pathAfterLastSlash();
+    return url.string().substring(startIndex, endIndex - startIndex - 1) == "null";
 }
 
 void ThreadableBlobRegistry::registerBlobURL(SecurityOrigin* origin, const URL& url, const URL& srcURL)

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/page/SecurityOrigin.cpp (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/page/SecurityOrigin.cpp	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/page/SecurityOrigin.cpp	2021-07-23 10:11:10 UTC (rev 280236)
@@ -263,8 +263,8 @@
         return true;
 
     // URLs that wrap inner URLs are secure if those inner URLs are secure.
-    if (shouldUseInnerURL(url) && LegacySchemeRegistry::shouldTreatURLSchemeAsSecure(extractInnerURL(url).protocol().toStringWithoutCopying()))
-        return true;
+    if (shouldUseInnerURL(url))
+        return LegacySchemeRegistry::shouldTreatURLSchemeAsSecure(extractInnerURL(url).protocol().toStringWithoutCopying()) || BlobURL::isSecureBlobURL(url);
 
     return false;
 }

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/page/csp/ContentSecurityPolicy.cpp (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/page/csp/ContentSecurityPolicy.cpp	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/page/csp/ContentSecurityPolicy.cpp	2021-07-23 10:11:10 UTC (rev 280236)
@@ -27,6 +27,7 @@
 #include "config.h"
 #include "ContentSecurityPolicy.h"
 
+#include "BlobURL.h"
 #include "ContentSecurityPolicyClient.h"
 #include "ContentSecurityPolicyDirective.h"
 #include "ContentSecurityPolicyDirectiveList.h"
@@ -277,8 +278,12 @@
     m_overrideInlineStyleAllowed = value;
 }
 
-bool ContentSecurityPolicy::urlMatchesSelf(const URL& url) const
+bool ContentSecurityPolicy::urlMatchesSelf(const URL& url, bool forFrameSrc) const
 {
+    // As per https://w3c.github.io/webappsec-csp/#match-url-to-source-_expression_, we compare the URL origin with the policy origin.
+    // We get origin using https://url.spec.whatwg.org/#concept-url-origin which has specific blob URLs treatment as follow.
+    if (forFrameSrc && url.protocolIsBlob())
+        return m_selfSource->matches(BlobURL::getOriginURL(url));
     return m_selfSource->matches(url);
 }

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/page/csp/ContentSecurityPolicy.h (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/page/csp/ContentSecurityPolicy.h	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/page/csp/ContentSecurityPolicy.h	2021-07-23 10:11:10 UTC (rev 280236)
@@ -138,7 +138,7 @@
     void reportDirectiveAsSourceExpression(const String& directiveName, const String& sourceExpression) const;
     void reportInvalidPathCharacter(const String& directiveName, const String& value, const char) const;
     void reportInvalidSourceExpression(const String& directiveName, const String& source) const;
-    bool urlMatchesSelf(const URL&) const;
+    bool urlMatchesSelf(const URL&, bool forFrameSrc) const;
     bool allowContentSecurityPolicySourceStarToMatchAnyProtocol() const;
 
     // Used by ContentSecurityPolicyDirectiveList

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp (280235 => 280236)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp	2021-07-23 09:03:58 UTC (rev 280235)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp	2021-07-23 10:11:10 UTC (rev 280236)
@@ -132,7 +132,8 @@
     if (m_allowStar && isProtocolAllowedByStar(url))
         return true;
 
-    if (m_allowSelf && m_policy.urlMatchesSelf(url))
+    if (m_allowSelf && m_policy.urlMatchesSelf(url, equalIgnoringASCIICase(m_directiveName, ContentSecurityPolicyDirectiveNames::frameSrc)
+))
         return true;
 
     for (auto& entry : m_list) {

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to