- Revision
- 276230
- Author
- [email protected]
- Date
- 2021-04-18 09:42:06 -0700 (Sun, 18 Apr 2021)
Log 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:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (276229 => 276230)
--- trunk/LayoutTests/ChangeLog 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/LayoutTests/ChangeLog 2021-04-18 16:42:06 UTC (rev 276230)
@@ -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-17 Tim Nguyen <[email protected]>
Add support for inline-{start/end} values to float & clear properties
Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (276229 => 276230)
--- trunk/LayoutTests/platform/mac-wk1/TestExpectations 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations 2021-04-18 16:42:06 UTC (rev 276230)
@@ -137,6 +137,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: trunk/LayoutTests/platform/win/TestExpectations (276229 => 276230)
--- trunk/LayoutTests/platform/win/TestExpectations 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/LayoutTests/platform/win/TestExpectations 2021-04-18 16:42:06 UTC (rev 276230)
@@ -224,6 +224,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: trunk/Source/WebCore/ChangeLog (276229 => 276230)
--- trunk/Source/WebCore/ChangeLog 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/Source/WebCore/ChangeLog 2021-04-18 16:42:06 UTC (rev 276230)
@@ -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-04-18 Cathie Chen <[email protected]>
The implicit aspect-ratio from width and height attributes with float value is not accurate enough
Modified: trunk/Source/WebCore/fileapi/BlobURL.cpp (276229 => 276230)
--- trunk/Source/WebCore/fileapi/BlobURL.cpp 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/Source/WebCore/fileapi/BlobURL.cpp 2021-04-18 16:42:06 UTC (rev 276230)
@@ -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: trunk/Source/WebCore/fileapi/BlobURL.h (276229 => 276230)
--- trunk/Source/WebCore/fileapi/BlobURL.h 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/Source/WebCore/fileapi/BlobURL.h 2021-04-18 16:42:06 UTC (rev 276230)
@@ -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: trunk/Source/WebCore/fileapi/ThreadableBlobRegistry.cpp (276229 => 276230)
--- trunk/Source/WebCore/fileapi/ThreadableBlobRegistry.cpp 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/Source/WebCore/fileapi/ThreadableBlobRegistry.cpp 2021-04-18 16:42:06 UTC (rev 276230)
@@ -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: trunk/Source/WebCore/loader/MixedContentChecker.cpp (276229 => 276230)
--- trunk/Source/WebCore/loader/MixedContentChecker.cpp 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/Source/WebCore/loader/MixedContentChecker.cpp 2021-04-18 16:42:06 UTC (rev 276230)
@@ -37,7 +37,6 @@
#include "FrameLoaderClient.h"
#include "SecurityOrigin.h"
#include "Settings.h"
-#include "ThreadableBlobRegistry.h"
#include <wtf/text/CString.h>
#include <wtf/text/WTFString.h>
@@ -49,22 +48,6 @@
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.
return !SecurityOrigin::isSecure(url);
}
Modified: trunk/Source/WebCore/page/SecurityOrigin.cpp (276229 => 276230)
--- trunk/Source/WebCore/page/SecurityOrigin.cpp 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/Source/WebCore/page/SecurityOrigin.cpp 2021-04-18 16:42:06 UTC (rev 276230)
@@ -265,8 +265,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: trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp (276229 => 276230)
--- trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp 2021-04-18 16:42:06 UTC (rev 276230)
@@ -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: trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h (276229 => 276230)
--- trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h 2021-04-18 16:42:06 UTC (rev 276230)
@@ -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: trunk/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp (276229 => 276230)
--- trunk/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp 2021-04-18 13:40:37 UTC (rev 276229)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp 2021-04-18 16:42:06 UTC (rev 276230)
@@ -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) {
