ASSERTION FAILED: canvas()->securityOrigin()->toString() == cachedImage.origin()...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Oct 2016 08:04:20 +0000 (08:04 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Oct 2016 08:04:20 +0000 (08:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163242

Patch by Youenn Fablet <youenn@apple.com> on 2016-10-24
Reviewed by Darin Adler.

Source/WebCore:

Test: http/tests/security/cross-origin-cached-images-canvas.html

We were previously on Origin HTTP header to check whether requests were made from different origins.
This is fine for CORS enabled requests but not for GET no CORS requests since they will not have any Origin header.

Now that CachedResource and CachedResourceRequest own their origin, it is best to use these directly.

* loader/cache/CachedResourceLoader.cpp:
(WebCore::isRequestMatchingResourceOrigin):
(WebCore::CachedResourceLoader::shouldUpdateCachedResourceWithCurrentRequest):

LayoutTests:

* http/tests/security/cross-origin-cached-images-canvas-expected.txt: Added.
* http/tests/security/cross-origin-cached-images-canvas.html: Added.
* http/tests/security/resources/cross-origin-cached-image-canvas-iframe.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/security/cross-origin-cached-images-canvas-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/cross-origin-cached-images-canvas.html [new file with mode: 0644]
LayoutTests/http/tests/security/resources/cross-origin-cached-image-canvas-iframe.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedResourceLoader.cpp

index 4d2397d..7f7ddc7 100644 (file)
@@ -1,5 +1,16 @@
 2016-10-24  Youenn Fablet  <youenn@apple.com>
 
+        ASSERTION FAILED: canvas()->securityOrigin()->toString() == cachedImage.origin()->toString()
+        https://bugs.webkit.org/show_bug.cgi?id=163242
+
+        Reviewed by Darin Adler.
+
+        * http/tests/security/cross-origin-cached-images-canvas-expected.txt: Added.
+        * http/tests/security/cross-origin-cached-images-canvas.html: Added.
+        * http/tests/security/resources/cross-origin-cached-image-canvas-iframe.html: Added.
+
+2016-10-24  Youenn Fablet  <youenn@apple.com>
+
         Redirections should be upgraded if CSP policy says so
         https://bugs.webkit.org/show_bug.cgi?id=163544
 
diff --git a/LayoutTests/http/tests/security/cross-origin-cached-images-canvas-expected.txt b/LayoutTests/http/tests/security/cross-origin-cached-images-canvas-expected.txt
new file mode 100644 (file)
index 0000000..b2e67e7
--- /dev/null
@@ -0,0 +1,6 @@
+CONSOLE MESSAGE: line 27: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.
+Trying to load and render in canvas sequentially the same image from different origins (no cors mode).
+
+PASS: Successfully retrieved image data.
+PASS: The image is not same-origin with this document.
+  
diff --git a/LayoutTests/http/tests/security/cross-origin-cached-images-canvas.html b/LayoutTests/http/tests/security/cross-origin-cached-images-canvas.html
new file mode 100644 (file)
index 0000000..24090b3
--- /dev/null
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>Trying to load and render in canvas sequentially the same image from different origins (no cors mode).</p>
+<div id="console"></div>
+<div>
+    <iframe id="iframe1"></iframe>
+    <iframe id="iframe2"></iframe>
+</div>
+<script>
+if (window.testRunner) {
+   testRunner.dumpAsText();
+   testRunner.waitUntilDone();
+}
+
+window.addEventListener("message", function(event) {
+    document.getElementById('console').innerHTML += event.data + "<br/>";
+    loadNextFrame();
+});
+
+var iframeURL8000 = "http://127.0.0.1:8000/security/resources/cross-origin-cached-image-canvas-iframe.html";
+var iframeURL8080 = "http://127.0.0.1:8080/security/resources/cross-origin-cached-image-canvas-iframe.html";
+
+var allowAllImage = "http://127.0.0.1:8000/security/resources/abe-allow-star.php?allowCache";
+
+var counter = 0;
+function loadNextFrame()
+{
+    counter++;
+    if (counter == 1)
+        document.getElementById('iframe1').src = iframeURL8000 + "#" +
+            encodeURIComponent(JSON.stringify({node: "img", url: allowAllImage, shouldPass:true, id: 1}));
+    else if (counter == 2)
+        document.getElementById('iframe2').src = iframeURL8080 + "#" +
+            encodeURIComponent(JSON.stringify({node: "img", url: allowAllImage, shouldPass:false, id: 2}));
+    else if (window.testRunner)
+        testRunner.notifyDone();
+}
+
+loadNextFrame();
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/resources/cross-origin-cached-image-canvas-iframe.html b/LayoutTests/http/tests/security/resources/cross-origin-cached-image-canvas-iframe.html
new file mode 100644 (file)
index 0000000..a1bf45f
--- /dev/null
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<body>
+<canvas id="canvas">
+<img id="img" onload="drawInCanvas()"></img>
+<script>
+var test = JSON.parse(decodeURIComponent(location.hash.substring(1)));
+
+function logError()
+{
+    parent.postMessage((!test.shouldPass ? "PASS": "FAIL") + ": The image is not same-origin with this document.", "*");
+}
+
+function logPass()
+{
+    parent.postMessage((test.shouldPass ? "PASS": "FAIL") + ": Successfully retrieved image data.", "*");
+}
+
+function drawInCanvas()
+{
+    var canvas = document.getElementById('canvas');
+    var ctx = canvas.getContext('2d');
+    ctx.drawImage(document.getElementById('img'), 0, 0);
+
+    // Grab a pixel to verify that the image is same-origin:
+    try {
+        var pixel = ctx.getImageData(0, 0, 1, 1);
+        logPass();
+    } catch (e) {
+        logError();
+    }
+}
+
+var node = document.getElementById(test.node);
+node.src = test.url;
+</script>
+</body>
+</html>
index 2637d07..e2359c9 100644 (file)
@@ -1,5 +1,23 @@
 2016-10-24  Youenn Fablet  <youenn@apple.com>
 
+        ASSERTION FAILED: canvas()->securityOrigin()->toString() == cachedImage.origin()->toString()
+        https://bugs.webkit.org/show_bug.cgi?id=163242
+
+        Reviewed by Darin Adler.
+
+        Test: http/tests/security/cross-origin-cached-images-canvas.html
+
+        We were previously on Origin HTTP header to check whether requests were made from different origins.
+        This is fine for CORS enabled requests but not for GET no CORS requests since they will not have any Origin header.
+
+        Now that CachedResource and CachedResourceRequest own their origin, it is best to use these directly.
+
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::isRequestMatchingResourceOrigin):
+        (WebCore::CachedResourceLoader::shouldUpdateCachedResourceWithCurrentRequest):
+
+2016-10-24  Youenn Fablet  <youenn@apple.com>
+
         Remove CachedResource::passesSameOriginPolicyCheck
         https://bugs.webkit.org/show_bug.cgi?id=163593
 
index 0502395..d832de8 100644 (file)
@@ -550,6 +550,17 @@ bool CachedResourceLoader::shouldContinueAfterNotifyingLoadedFromMemoryCache(con
     return !newRequest.isNull();
 }
 
+static inline bool originsMatch(const CachedResourceRequest& request, const CachedResource& resource)
+{
+    if (request.origin() == resource.origin())
+        return true;
+    if (!request.origin() || !resource.origin())
+        return false;
+    // We use string comparison as this is how they are serialized as HTTP Origin header value.
+    // This is in particular useful for unique origins that are serialized as "null"
+    return request.origin()->toString() == resource.origin()->toString();
+}
+
 bool CachedResourceLoader::shouldUpdateCachedResourceWithCurrentRequest(const CachedResource& resource, const CachedResourceRequest& request)
 {
     // WebKit is not supporting CORS for fonts (https://bugs.webkit.org/show_bug.cgi?id=86817), no need to update the resource before reusing it.
@@ -583,7 +594,7 @@ bool CachedResourceLoader::shouldUpdateCachedResourceWithCurrentRequest(const Ca
         break;
     }
 
-    if (resource.options().mode != request.options().mode || request.resourceRequest().httpOrigin() != resource.resourceRequest().httpOrigin())
+    if (resource.options().mode != request.options().mode || !originsMatch(request, resource))
         return true;
 
     if (resource.options().redirect != request.options().redirect && resource.hasRedirections())