Changeset 276230 in webkit

Timestamp:

Apr 18, 2021, 9:42:06 AM (5 years ago)

Author:

youenn@apple.com

Message:

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac-wk1/TestExpectations (modified) (1 diff)
• LayoutTests/platform/win/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/fileapi/BlobURL.cpp (modified) (2 diffs)
• Source/WebCore/fileapi/BlobURL.h (modified) (1 diff)
• Source/WebCore/fileapi/ThreadableBlobRegistry.cpp (modified) (1 diff)
• Source/WebCore/loader/MixedContentChecker.cpp (modified) (2 diffs)
• Source/WebCore/page/SecurityOrigin.cpp (modified) (1 diff)
• Source/WebCore/page/csp/ContentSecurityPolicy.cpp (modified) (2 diffs)
• Source/WebCore/page/csp/ContentSecurityPolicy.h (modified) (1 diff)
• Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-04-18  Youenn Fablet  <youenn@apple.com>
+
+        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-17  Tim Nguyen  <ntim@apple.com>
 

trunk/LayoutTests/platform/mac-wk1/TestExpectations

@@ -138 +138 @@
 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

trunk/LayoutTests/platform/win/TestExpectations

@@ -225 +225 @@
 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.

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-04-18  Youenn Fablet  <youenn@apple.com>
+
+        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-04-18  Cathie Chen  <cathiechen@igalia.com>
 

trunk/Source/WebCore/fileapi/BlobURL.cpp

@@ -32 +32 @@
 
 #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>
@@ -53 +55 @@
 }
 
-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);
 }
 

trunk/Source/WebCore/fileapi/BlobURL.h

@@ -50 +50 @@
     static URL createPublicURL(SecurityOrigin*);
     static URL createInternalURL();
-    static String getOrigin(const URL&);
+
+    static URL getOriginURL(const URL&);
+    static bool isSecureBlobURL(const URL&);
 
 private:

trunk/Source/WebCore/fileapi/ThreadableBlobRegistry.cpp

@@ -95 +95 @@
 {
     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";
 }
 

trunk/Source/WebCore/loader/MixedContentChecker.cpp

@@ -38 +38 @@
 #include "SecurityOrigin.h"
 #include "Settings.h"
-#include "ThreadableBlobRegistry.h"
 #include <wtf/text/CString.h>
 #include <wtf/text/WTFString.h>
@@ -49 +48 @@
     if (securityOrigin.protocol() != "https")
         return false; // We only care about HTTPS security origins.
-
-    if (url.protocolIsBlob()) {
-        // As per https://github.com/w3c/webappsec-mixed-content/issues/41, Blob URL is secure if the document that created it is secure.
-        // This code path is specific to opaque origins.
-        if (auto origin = ThreadableBlobRegistry::getCachedOrigin(url)) {
-            const Document* blobDocument = nullptr;
-            for (const auto* document : Document::allDocuments()) {
-                if (&document->securityOrigin() == origin.get()) {
-                    blobDocument = document;
-                    break;
-                }
-            }
-            if (blobDocument && blobDocument->isSecureContext())
-                return false;
-        }
-    }
 
     // We're in a secure context, so |url| is mixed content if it's insecure.

trunk/Source/WebCore/page/SecurityOrigin.cpp

@@ -266 +266 @@
 
     // 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;

trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp

@@ -28 +28 @@
 #include "ContentSecurityPolicy.h"
 
+#include "BlobURL.h"
 #include "ContentSecurityPolicyClient.h"
 #include "ContentSecurityPolicyDirective.h"
@@ -278 +279 @@
 }
 
-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);
 }

trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h

@@ -139 +139 @@
     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;
 

trunk/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp

@@ -133 +133 @@
         return true;
 
-    if (m_allowSelf && m_policy.urlMatchesSelf(url))
+    if (m_allowSelf && m_policy.urlMatchesSelf(url, equalIgnoringASCIICase(m_directiveName, ContentSecurityPolicyDirectiveNames::frameSrc)
+))
         return true;