SecurityOrigin should be unique for null blob URLs that have been unregistered
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Dec 2019 11:04:41 +0000 (11:04 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Dec 2019 11:04:41 +0000 (11:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=205169

Reviewed by Darin Adler.

Source/WebCore:

In case we cannot retrieve a cached origin for a null origin, just create a unique one.
This is better than having an origin with an empty host and empty scheme.

Test: http/tests/security/blob-null-url-location-origin.html

* fileapi/ThreadableBlobRegistry.cpp:
(WebCore::ThreadableBlobRegistry::unregisterBlobURL):
(WebCore::ThreadableBlobRegistry::getCachedOrigin):

LayoutTests:

* http/tests/security/blob-null-url-location-origin-expected.txt: Added.
* http/tests/security/blob-null-url-location-origin.html: Added.
* platform/win/TestExpectations: Skipping test as timing out in windows.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@253544 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/security/blob-null-url-location-origin-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/blob-null-url-location-origin.html [new file with mode: 0644]
LayoutTests/platform/win/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/fileapi/ThreadableBlobRegistry.cpp

index bc16a2f..fc67c9a 100644 (file)
@@ -1,3 +1,14 @@
+2019-12-16  youenn fablet  <youenn@apple.com>
+
+        SecurityOrigin should be unique for null blob URLs that have been unregistered
+        https://bugs.webkit.org/show_bug.cgi?id=205169
+
+        Reviewed by Darin Adler.
+
+        * http/tests/security/blob-null-url-location-origin-expected.txt: Added.
+        * http/tests/security/blob-null-url-location-origin.html: Added.
+        * platform/win/TestExpectations: Skipping test as timing out in windows.
+
 2019-12-15  Emilio Cobos Álvarez  <emilio@crisal.io>
 
         Remove -webkit-marquee.
diff --git a/LayoutTests/http/tests/security/blob-null-url-location-origin-expected.txt b/LayoutTests/http/tests/security/blob-null-url-location-origin-expected.txt
new file mode 100644 (file)
index 0000000..d0eb419
--- /dev/null
@@ -0,0 +1,5 @@
+CONSOLE MESSAGE: line 1: data URL frame loaded
+CONSOLE MESSAGE: line 1: blob popup opened
+CONSOLE MESSAGE: line 1: blob popup loadednull
+CONSOLE MESSAGE: line 1: blob popup message posted
+PASS
diff --git a/LayoutTests/http/tests/security/blob-null-url-location-origin.html b/LayoutTests/http/tests/security/blob-null-url-location-origin.html
new file mode 100644 (file)
index 0000000..43207c1
--- /dev/null
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<body>
+<script>
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+    testRunner.setCanOpenWindows();
+}
+
+let count = 0;
+onmessage = (event) => {
+    ++count;
+    if (event.data !== "null") {
+        document.body.innerHTML = "FAIL, got " + event.data + " with count " + count;
+        if (window.testRunner)
+            testRunner.notifyDone();
+        window.clearTimeout(timer);
+        count = 3;
+        return;
+    }
+    if (count === 2) {
+        document.body.innerHTML = "PASS";
+        if (window.testRunner)
+            testRunner.notifyDone();
+        window.clearTimeout(timer);
+    }
+};
+
+const timer = setTimeout(() => {
+    document.body.innerHTML = "TIMEOUT";
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, 10000);
+</script>
+<iframe src="data:text/html,<script>console.log('data URL frame loaded'); const blob = new Blob(['<'+ 'script>console.log(\'blob popup loaded\' + location.origin); onload = () => window.opener.postMessage(location.origin, \'*\'); console.log(\'blob popup message posted\'); onmessage = () => window.opener.postMessage(location.origin, \'*\'); <' + '/script>'], {type: 'text/html'}); const blobURL = URL.createObjectURL(blob); const popup = window.open(blobURL); onmessage = (event) => { popup.postMessage('check', '*'); parent.postMessage(event.data, '*'); URL.revokeObjectURL(blobURL); onmessage = (event) => { parent.postMessage(event.data, '*'); popup.close(); }; }; console.log('blob popup opened'); </script>"></iframe>
+</body>
index aaf90c7..5a4bfef 100644 (file)
@@ -744,6 +744,8 @@ http/tests/security/contentSecurityPolicy/same-origin-plugin-document-allowed-in
 http/tests/security/contentSecurityPolicy/same-origin-plugin-document-blocked-in-child-window-report.php [ Skip ]
 http/tests/security/contentSecurityPolicy/same-origin-plugin-document-with-csp-blocked-in-child-window.html [ Skip ]
 
+http/tests/security/blob-null-url-location-origin.html [ Skip ]
+
 ################################################################################
 ############################   End Plugin Failures   ###########################
 ################################################################################
index 71ef85d..7a7ff08 100644 (file)
@@ -1,3 +1,19 @@
+2019-12-16  youenn fablet  <youenn@apple.com>
+
+        SecurityOrigin should be unique for null blob URLs that have been unregistered
+        https://bugs.webkit.org/show_bug.cgi?id=205169
+
+        Reviewed by Darin Adler.
+
+        In case we cannot retrieve a cached origin for a null origin, just create a unique one.
+        This is better than having an origin with an empty host and empty scheme.
+
+        Test: http/tests/security/blob-null-url-location-origin.html
+
+        * fileapi/ThreadableBlobRegistry.cpp:
+        (WebCore::ThreadableBlobRegistry::unregisterBlobURL):
+        (WebCore::ThreadableBlobRegistry::getCachedOrigin):
+
 2019-12-15  Emilio Cobos Álvarez  <emilio@crisal.io>
 
         CSSParserMode::HTMLAttributeMode is unused.
index 272530b..5e1d4c0 100644 (file)
@@ -89,10 +89,16 @@ void ThreadableBlobRegistry::registerBlobURL(const URL& url, Vector<BlobPart>&&
     });
 }
 
+static inline bool isBlobURLContainsNullOrigin(const URL& url)
+{
+    ASSERT(url.protocolIsBlob());
+    return BlobURL::getOrigin(url) == "null";
+}
+
 void ThreadableBlobRegistry::registerBlobURL(SecurityOrigin* origin, const URL& url, const URL& srcURL)
 {
     // If the blob URL contains null origin, as in the context with unique security origin or file URL, save the mapping between url and origin so that the origin can be retrived when doing security origin check.
-    if (origin && BlobURL::getOrigin(url) == "null")
+    if (origin && isBlobURLContainsNullOrigin(url))
         originMap()->add(url.string(), origin);
 
     if (isMainThread()) {
@@ -145,7 +151,7 @@ unsigned long long ThreadableBlobRegistry::blobSize(const URL& url)
 
 void ThreadableBlobRegistry::unregisterBlobURL(const URL& url)
 {
-    if (BlobURL::getOrigin(url) == "null")
+    if (isBlobURLContainsNullOrigin(url))
         originMap()->remove(url.string());
 
     if (isMainThread()) {
@@ -159,7 +165,14 @@ void ThreadableBlobRegistry::unregisterBlobURL(const URL& url)
 
 RefPtr<SecurityOrigin> ThreadableBlobRegistry::getCachedOrigin(const URL& url)
 {
-    return originMap()->get(url.string());
+    if (auto cachedOrigin = originMap()->get(url.string()))
+        return cachedOrigin;
+
+    if (!url.protocolIsBlob() || !isBlobURLContainsNullOrigin(url))
+        return nullptr;
+
+    // If we do not have a cached origin for null blob URLs, we use a unique origin.
+    return SecurityOrigin::createUnique();
 }
 
 } // namespace WebCore