Attachment 160572 Details for Bug 94918 – Patch

[patch] Patch

bug-94918-20120826002203.patch (text/plain), 15.71 KB, created by Mike West on 2012-08-25 15:22:10 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mike West

Created: 2012-08-25 15:22:10 PDT

Size: 15.71 KB

patch

obsolete

>Subversion Revision: 126681
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e2efb07c71c6b2d80df5423390a4d67ba4f222ca..1714ee52345b7506d26eec07b2e2099d7e9099e2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+2012-08-25  Mike West  <mkwst@chromium.org>
>+
>+        'self' in a CSP directive should match blob: and filesystem: URLs.
>+        https://bugs.webkit.org/show_bug.cgi?id=94918
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        'blob:' and 'filesystem:' URLs are same-origin with the page on which
>+        they were created. Currently, we're using the wrong URL for comparison
>+        when matching against CSP directive source lists. This patch adjusts the
>+        matching logic to compare against the blob's inner URL, rather than
>+        directly against the blob itself.
>+
>+        Tests: http/tests/security/contentSecurityPolicy/blob-urls-match-self.html
>+               http/tests/security/contentSecurityPolicy/filesystem-urls-match-self.html
>+               http/tests/security/contentSecurityPolicy/source-list-parsing-08.html
>+
>+        * page/ContentSecurityPolicy.cpp:
>+        (WebCore::CSPSourceList::matches):
>+            If we should use the inner URL of a given resource, extract it into
>+            a local variable, and pass that into CSPSource for comparison.
>+        * page/SecurityOrigin.cpp:
>+        (WebCore::SecurityOrigin::shouldUseInnerURL):
>+        (WebCore::SecurityOrigin::extractInnerURL):
>+            Move shouldUseInnerURL and extractInnerURL to SecurityOrigin's
>+            public signature.
>+        (WebCore::shouldTreatAsUniqueOrigin):
>+        (WebCore::SecurityOrigin::create):
>+        (WebCore::SecurityOrigin::isSecure):
>+            shouldUseInnerURL and extractInnerURL are now static methods of
>+            SecurityOrigin: updating calls to mathc.
>+        * page/SecurityOrigin.h:
>+
> 2012-08-25  Michelangelo De Simone  <michelangelo@webkit.org>
> 
>         [Crash] Null pointer in CSSParser::parseMixFunction()
>diff --git a/Source/WebCore/page/ContentSecurityPolicy.cpp b/Source/WebCore/page/ContentSecurityPolicy.cpp
>index a3834faa7ae389b6ae5b9fe59ab388be114e33df..aba263feabcdb0f5ca15e5f2b28e99a01445a138 100644
>--- a/Source/WebCore/page/ContentSecurityPolicy.cpp
>+++ b/Source/WebCore/page/ContentSecurityPolicy.cpp
>@@ -244,8 +244,10 @@ bool CSPSourceList::matches(const KURL& url)
>     if (m_allowStar)
>         return true;
> 
>+    KURL urlToMatch = SecurityOrigin::shouldUseInnerURL(url) ? SecurityOrigin::extractInnerURL(url) : url;
>+
>     for (size_t i = 0; i < m_list.size(); ++i) {
>-        if (m_list[i].matches(url))
>+        if (m_list[i].matches(urlToMatch))
>             return true;
>     }
> 
>diff --git a/Source/WebCore/page/SecurityOrigin.cpp b/Source/WebCore/page/SecurityOrigin.cpp
>index f5cd1c52e9cb1246743fd9ca610d4325c6104e02..1dc4a225d0ab0022ee5959b0a267c8866a64d525 100644
>--- a/Source/WebCore/page/SecurityOrigin.cpp
>+++ b/Source/WebCore/page/SecurityOrigin.cpp
>@@ -53,18 +53,7 @@ static bool schemeRequiresAuthority(const KURL& url)
>     return url.protocolIsInHTTPFamily() || url.protocolIs("ftp");
> }
> 
>-// Some URL schemes use nested URLs for their security context. For example,
>-// filesystem URLs look like the following:
>-//
>-//   filesystem:http://example.com/temporary/path/to/file.png
>-//
>-// We're supposed to use "http://example.com" as the origin.
>-//
>-// Generally, we add URL schemes to this list when WebKit support them. For
>-// example, we don't include the "jar" scheme, even though Firefox understands
>-// that jar uses an inner URL for it's security origin.
>-//
>-static bool shouldUseInnerURL(const KURL& url)
>+bool SecurityOrigin::shouldUseInnerURL(const KURL& url)
> {
> #if ENABLE(BLOB)
>     if (url.protocolIs("blob"))
>@@ -81,7 +70,7 @@ static bool shouldUseInnerURL(const KURL& url)
> // In general, extracting the inner URL varies by scheme. It just so happens
> // that all the URL schemes we currently support that use inner URLs for their
> // security origin can be parsed using this algorithm.
>-static KURL extractInnerURL(const KURL& url)
>+KURL SecurityOrigin::extractInnerURL(const KURL& url)
> {
>     if (url.innerURL())
>         return *url.innerURL();
>@@ -105,7 +94,7 @@ static bool shouldTreatAsUniqueOrigin(const KURL& url)
>         return true;
> 
>     // FIXME: Do we need to unwrap the URL further?
>-    KURL innerURL = shouldUseInnerURL(url) ? extractInnerURL(url) : url;
>+    KURL innerURL = SecurityOrigin::shouldUseInnerURL(url) ? SecurityOrigin::extractInnerURL(url) : url;
> 
>     // FIXME: Check whether innerURL is valid.
> 
>@@ -202,8 +191,8 @@ PassRefPtr<SecurityOrigin> SecurityOrigin::create(const KURL& url)
>         return origin.release();
>     }
> 
>-    if (shouldUseInnerURL(url))
>-        return adoptRef(new SecurityOrigin(extractInnerURL(url)));
>+    if (SecurityOrigin::shouldUseInnerURL(url))
>+        return adoptRef(new SecurityOrigin(SecurityOrigin::extractInnerURL(url)));
> 
>     return adoptRef(new SecurityOrigin(url));
> }
>@@ -233,7 +222,7 @@ bool SecurityOrigin::isSecure(const KURL& url)
>         return true;
> 
>     // URLs that wrap inner URLs are secure if those inner URLs are secure.
>-    if (shouldUseInnerURL(url) && SchemeRegistry::shouldTreatURLSchemeAsSecure(extractInnerURL(url).protocol()))
>+    if (SecurityOrigin::shouldUseInnerURL(url) && SchemeRegistry::shouldTreatURLSchemeAsSecure(SecurityOrigin::extractInnerURL(url).protocol()))
>         return true;
> 
>     return false;
>diff --git a/Source/WebCore/page/SecurityOrigin.h b/Source/WebCore/page/SecurityOrigin.h
>index 2e32cfb69bd7318102c6472d922f1d7c0be4f3fb..3fe1d083c0bbf840ff04580a77c5fdb2d3bbd317 100644
>--- a/Source/WebCore/page/SecurityOrigin.h
>+++ b/Source/WebCore/page/SecurityOrigin.h
>@@ -52,6 +52,19 @@ public:
>     static PassRefPtr<SecurityOrigin> createFromString(const String&);
>     static PassRefPtr<SecurityOrigin> create(const String& protocol, const String& host, int port);
> 
>+    // Some URL schemes use nested URLs for their security context. For example,
>+    // filesystem URLs look like the following:
>+    //
>+    //   filesystem:http://example.com/temporary/path/to/file.png
>+    //
>+    // We're supposed to use "http://example.com" as the origin.
>+    //
>+    // Generally, we add URL schemes to this list when WebKit support them. For
>+    // example, we don't include the "jar" scheme, even though Firefox
>+    // understands that "jar" uses an inner URL for it's security origin.
>+    static bool shouldUseInnerURL(const KURL&);
>+    static KURL extractInnerURL(const KURL&);
>+
>     // Create a deep copy of this SecurityOrigin. This method is useful
>     // when marshalling a SecurityOrigin to another thread.
>     PassRefPtr<SecurityOrigin> isolatedCopy() const;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 0d7cef6cc524bbaba4dd174dff3ea4ac7f65b06a..b917da20a746276ad3ad773626567c959e52fb4c 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,23 @@
>+2012-08-25  Mike West  <mkwst@chromium.org>
>+
>+        'self' in a CSP directive should match blob: and filesystem: URLs.
>+        https://bugs.webkit.org/show_bug.cgi?id=94918
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/tests/security/contentSecurityPolicy/blob-urls-match-self-expected.txt: Added.
>+        * http/tests/security/contentSecurityPolicy/blob-urls-match-self.html: Added.
>+        * http/tests/security/contentSecurityPolicy/filesystem-urls-match-self-expected.txt: Added.
>+        * http/tests/security/contentSecurityPolicy/filesystem-urls-match-self.html: Added.
>+            Test the new functionality.
>+        * http/tests/security/contentSecurityPolicy/resources/multiple-iframe-test.js:
>+        (test):
>+            Adding support for data: URLs.
>+        * http/tests/security/contentSecurityPolicy/source-list-parsing-08-expected.txt: Added.
>+        * http/tests/security/contentSecurityPolicy/source-list-parsing-08.html: Added.
>+            Adding data: URL tests to ensure that grabbing the inner URL of the
>+            URL to test doesn't inadvertently regress that behavior.
>+
> 2012-08-25  Michelangelo De Simone  <michelangelo@webkit.org>
> 
>         [Crash] Null pointer in CSSParser::parseMixFunction()
>diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/blob-urls-match-self-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/blob-urls-match-self-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..8e8082490280efdd823caabaf44cb113db06459d
>--- /dev/null
>+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/blob-urls-match-self-expected.txt
>@@ -0,0 +1,2 @@
>+ALERT: PASS (1/1)!
>+blob: URLs are same-origin with the page in which they were created, and should therefore match the 'self' source in CSP directives.
>diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/blob-urls-match-self.html b/LayoutTests/http/tests/security/contentSecurityPolicy/blob-urls-match-self.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..d7354f230ec643bc2bd750d4264834d446516995
>--- /dev/null
>+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/blob-urls-match-self.html
>@@ -0,0 +1,30 @@
>+<!DOCTYPE html>
>+<html>
>+    <head>
>+        <meta http-equiv="X-WebKit-CSP" content="script-src 'unsafe-inline' 'self'">
>+    </head>
>+    <body>
>+        <p>
>+            blob: URLs are same-origin with the page in which they were created,
>+            and should therefore match the 'self' source in CSP directives.
>+        </p>
>+
>+        <script>
>+            if (window.testRunner) {
>+                testRunner.dumpAsText();
>+                testRunner.waitUntilDone();
>+            }
>+            function pass() {
>+                alert("PASS (1/1)!");
>+                if (window.testRunner)
>+                    testRunner.notifyDone();
>+            }
>+
>+            var b = new Blob(['pass();'], { type: 'application/javascript' });
>+
>+            var script = document.createElement('script');
>+            script.src = URL.createObjectURL(b);
>+            document.body.appendChild(script);
>+        </script>
>+    </body>
>+</html>
>diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/filesystem-urls-match-self-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/filesystem-urls-match-self-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..d739522bb3b578469f056c732964be465d3a70dc
>--- /dev/null
>+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/filesystem-urls-match-self-expected.txt
>@@ -0,0 +1,2 @@
>+ALERT: PASS (1/1)!
>+filesystem: URLs are same-origin with the page in which they were created, and should therefore match the 'self' source in CSP directives.
>diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/filesystem-urls-match-self.html b/LayoutTests/http/tests/security/contentSecurityPolicy/filesystem-urls-match-self.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..ef94bfeac508da34c5f22bec8c3f6f9ff2980f29
>--- /dev/null
>+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/filesystem-urls-match-self.html
>@@ -0,0 +1,41 @@
>+<!DOCTYPE html>
>+<html>
>+    <head>
>+        <meta http-equiv="X-WebKit-CSP" content="script-src 'unsafe-inline' 'self'">
>+    </head>
>+    <body>
>+        <p>
>+            filesystem: URLs are same-origin with the page in which they were
>+            created, and should therefore match the 'self' source in CSP
>+            directives.
>+        </p>
>+
>+        <script>
>+            if (window.testRunner) {
>+                testRunner.dumpAsText();
>+                testRunner.waitUntilDone();
>+            }
>+            function pass() {
>+                alert("PASS (1/1)!");
>+                if (window.testRunner)
>+                    testRunner.notifyDone();
>+            }
>+
>+            window.webkitRequestFileSystem(TEMPORARY, 1024*1024 /*1MB*/, function(fs) {
>+                fs.root.getFile('pass.js', {create: true}, function(fileEntry) {
>+                    fileEntry.createWriter(function(fileWriter) {
>+                        fileWriter.onwriteend = function(e) {
>+                            var script = document.createElement('script');
>+                            script.src =fileEntry.toURL('application/javascript');
>+                            document.body.appendChild(script);
>+                        };
>+
>+                        // Create a new Blob and write it to pass.js.
>+                        var b = new Blob(['pass();'], {type: 'application/javascript'});
>+                        fileWriter.write(b);
>+                    });
>+                });
>+            });
>+        </script>
>+    </body>
>+</html>
>diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/resources/multiple-iframe-test.js b/LayoutTests/http/tests/security/contentSecurityPolicy/resources/multiple-iframe-test.js
>index 7cd4cd0bb83c18a64383387504bface8cf6310b9..d2f165cc907c0f8ca6d68570645e3cd77f52ab8c 100644
>--- a/LayoutTests/http/tests/security/contentSecurityPolicy/resources/multiple-iframe-test.js
>+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/resources/multiple-iframe-test.js
>@@ -14,7 +14,9 @@ function test() {
>     iframe.src = baseURL + "resources/echo-script-src.pl?" +
>                  "should_run=" + escape(current[0]) +
>                  "&csp=" + escape(current[1]) +
>-                 "&q=" + baseURL + escape(current[2]);
>+                 "&q=" + (current[2].match(/^data:/) ?
>+                     escape(current[2]) :
>+                     baseURL + escape(current[2]));
>     if (current[3])
>       iframe.src += "&nonce=" + escape(current[3]);
> 
>diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-08-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-08-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..372b86f12d39a0fb3c25939900793cadf71d483a
>--- /dev/null
>+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-08-expected.txt
>@@ -0,0 +1,22 @@
>+CONSOLE MESSAGE: Refused to load the script 'data:application/javascript;base64,dmFyIHJlc3VsdCA9IGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCdyZXN1bHQnKTtyZXN1bHQuZmlyc3RDaGlsZC5ub2RlVmFsdWUgPSByZXN1bHQuYXR0cmlidXRlcy5nZXROYW1lZEl0ZW0oJ3RleHQnKS52YWx1ZTs=' because it violates the following Content Security Policy directive: "script-src 'self'".
>+
>+CONSOLE MESSAGE: Refused to load the script 'data:application/javascript;base64,dmFyIHJlc3VsdCA9IGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCdyZXN1bHQnKTtyZXN1bHQuZmlyc3RDaGlsZC5ub2RlVmFsdWUgPSByZXN1bHQuYXR0cmlidXRlcy5nZXROYW1lZEl0ZW0oJ3RleHQnKS52YWx1ZTs=' because it violates the following Content Security Policy directive: "script-src https://127.0.0.1:8000".
>+
>+Test proper handling of data: URLs.
>+
>+
>+
>+--------
>+Frame: '-->'
>+--------
>+PASS
>+
>+--------
>+Frame: '-->'
>+--------
>+PASS
>+
>+--------
>+Frame: '-->'
>+--------
>+PASS
>diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-08.html b/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-08.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..c96a94727433c8c5beb34fab3ac2890cc9022e99
>--- /dev/null
>+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-08.html
>@@ -0,0 +1,19 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src='resources/multiple-iframe-test.js'></script>
>+<script>
>+var dataURL = "data:application/javascript;base64," +
>+    btoa("var result = document.getElementById('result');" +
>+         "result.firstChild.nodeValue = result.attributes.getNamedItem('text').value;");
>+var tests = [
>+    ['yes', 'script-src data:', dataURL],
>+    ['no', 'script-src \'self\'', dataURL],
>+    ['no',  'script-src https://127.0.0.1:8000', dataURL],
>+];
>+</script>
>+</head>
>+<body onload="test()">
>+  <p>
>+    Test proper handling of data: URLs.
>+  </p>

Actions: View | Formatted Diff | Diff

Attachments on bug 94918: 160387 | 160572 | 160768