Title: [120540] trunk
Revision
120540
Author
[email protected]
Date
2012-06-16 17:35:48 -0700 (Sat, 16 Jun 2012)

Log Message

Ignore paths in Content Security Policy sources rather than failing to parse them.
https://bugs.webkit.org/show_bug.cgi?id=89281

Patch by Mike West <[email protected]> on 2012-06-16
Reviewed by Adam Barth.

Source/WebCore:

In short: `script-src http://example.com/` should allow scripts from
http://example.com. Currently, it allows no scripts at all, as the
terminal `/` isn't accepted as part of a hostname.

This patch adjusts CSPSourceList::parseSource to accept paths (and
discard them). Once this lands, the next step will be to keep the
path, and use it when comparing source origins in the various
allowXXXFromSource methods.

Tests: http/tests/security/contentSecurityPolicy/source-list-parsing-05.html
       http/tests/security/contentSecurityPolicy/source-list-parsing-06.html

* page/ContentSecurityPolicy.cpp:
(CSPSourceList):
(WebCore):
(WebCore::CSPSourceList::parseSource):
    Reworked this method entirely to support paths.
(WebCore::CSPSourceList::parsePath):
    More or less a no-op at the moment.
(WebCore::CSPSourceList::parsePort):
    Moved the `:` assertion here from parseSource.

LayoutTests:

* http/tests/security/contentSecurityPolicy/source-list-parsing-05-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/source-list-parsing-05.html: Added.
* http/tests/security/contentSecurityPolicy/source-list-parsing-06-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/source-list-parsing-06.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (120539 => 120540)


--- trunk/LayoutTests/ChangeLog	2012-06-17 00:26:26 UTC (rev 120539)
+++ trunk/LayoutTests/ChangeLog	2012-06-17 00:35:48 UTC (rev 120540)
@@ -1,5 +1,17 @@
 2012-06-16  Mike West  <[email protected]>
 
+        Ignore paths in Content Security Policy sources rather than failing to parse them.
+        https://bugs.webkit.org/show_bug.cgi?id=89281
+
+        Reviewed by Adam Barth.
+
+        * http/tests/security/contentSecurityPolicy/source-list-parsing-05-expected.txt: Added.
+        * http/tests/security/contentSecurityPolicy/source-list-parsing-05.html: Added.
+        * http/tests/security/contentSecurityPolicy/source-list-parsing-06-expected.txt: Added.
+        * http/tests/security/contentSecurityPolicy/source-list-parsing-06.html: Added.
+
+2012-06-16  Mike West  <[email protected]>
+
         Fix test cases for `header`/`footer` AXRoleDescription
         https://bugs.webkit.org/show_bug.cgi?id=88911
 

Added: trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-05-expected.txt (0 => 120540)


--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-05-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-05-expected.txt	2012-06-17 00:35:48 UTC (rev 120540)
@@ -0,0 +1,55 @@
+CONSOLE MESSAGE: Unrecognized Content-Security-Policy directive 'pathwithasemicolon'.
+
+Paths should be ignored when evaluating sources.
+
+
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame1-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame2-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame3-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame4-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame5-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame6-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame7-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame8-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame9-->-->'
+--------
+PASS

Added: trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-05.html (0 => 120540)


--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-05.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-05.html	2012-06-17 00:35:48 UTC (rev 120540)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=''></script>
+<script>
+var tests = [
+    ['yes', 'script-src 127.0.0.1:*/', 'resources/script.js'],
+    ['yes', 'script-src 127.0.0.1:*/path', 'resources/script.js'],
+    ['yes', 'script-src 127.0.0.1:*/path?query=string', 'resources/script.js'],
+    ['yes', 'script-src 127.0.0.1:*/path#anchor', 'resources/script.js'],
+    ['yes', 'script-src 127.0.0.1:8000/', 'resources/script.js'],
+    ['yes', 'script-src 127.0.0.1:8000/path', 'resources/script.js'],
+    ['yes', 'script-src 127.0.0.1:8000/path?query=string', 'resources/script.js'],
+    ['yes', 'script-src 127.0.0.1:8000/path#anchor', 'resources/script.js'],
+    ['yes', 'script-src 127.0.0.1:8000/thisisa;pathwithasemicolon', 'resources/script.js'],
+    ['yes', 'script-src 127.0.0.1:8000/this is a path with spaces', 'resources/script.js'],
+];
+</script>
+</head>
+<body _onload_="test()">
+  <p>
+    Paths should be ignored when evaluating sources.
+  </p>

Added: trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-06-expected.txt (0 => 120540)


--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-06-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-06-expected.txt	2012-06-17 00:35:48 UTC (rev 120540)
@@ -0,0 +1,55 @@
+CONSOLE MESSAGE: Unrecognized Content-Security-Policy directive 'pathwithasemicolon'.
+
+Paths should be ignored when evaluating sources.
+
+
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame1-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame2-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame3-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame4-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame5-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame6-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame7-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame8-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame9-->-->'
+--------
+PASS

Added: trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-06.html (0 => 120540)


--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-06.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-06.html	2012-06-17 00:35:48 UTC (rev 120540)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=''></script>
+<script>
+var tests = [
+    ['yes', 'script-src http://127.0.0.1:*/', 'resources/script.js'],
+    ['yes', 'script-src http://127.0.0.1:*/path', 'resources/script.js'],
+    ['yes', 'script-src http://127.0.0.1:*/path?query=string', 'resources/script.js'],
+    ['yes', 'script-src http://127.0.0.1:*/path#anchor', 'resources/script.js'],
+    ['yes', 'script-src http://127.0.0.1:8000/', 'resources/script.js'],
+    ['yes', 'script-src http://127.0.0.1:8000/path', 'resources/script.js'],
+    ['yes', 'script-src http://127.0.0.1:8000/path?query=string', 'resources/script.js'],
+    ['yes', 'script-src http://127.0.0.1:8000/path#anchor', 'resources/script.js'],
+    ['yes', 'script-src http://127.0.0.1:8000/thisisa;pathwithasemicolon', 'resources/script.js'],
+    ['yes', 'script-src http://127.0.0.1:8000/this is a path with spaces', 'resources/script.js'],
+];
+</script>
+</head>
+<body _onload_="test()">
+  <p>
+    Paths should be ignored when evaluating sources.
+  </p>

Modified: trunk/Source/WebCore/ChangeLog (120539 => 120540)


--- trunk/Source/WebCore/ChangeLog	2012-06-17 00:26:26 UTC (rev 120539)
+++ trunk/Source/WebCore/ChangeLog	2012-06-17 00:35:48 UTC (rev 120540)
@@ -1,3 +1,32 @@
+2012-06-16  Mike West  <[email protected]>
+
+        Ignore paths in Content Security Policy sources rather than failing to parse them.
+        https://bugs.webkit.org/show_bug.cgi?id=89281
+
+        Reviewed by Adam Barth.
+
+        In short: `script-src http://example.com/` should allow scripts from
+        http://example.com. Currently, it allows no scripts at all, as the
+        terminal `/` isn't accepted as part of a hostname.
+
+        This patch adjusts CSPSourceList::parseSource to accept paths (and
+        discard them). Once this lands, the next step will be to keep the
+        path, and use it when comparing source origins in the various
+        allowXXXFromSource methods.
+
+        Tests: http/tests/security/contentSecurityPolicy/source-list-parsing-05.html
+               http/tests/security/contentSecurityPolicy/source-list-parsing-06.html
+
+        * page/ContentSecurityPolicy.cpp:
+        (CSPSourceList):
+        (WebCore):
+        (WebCore::CSPSourceList::parseSource):
+            Reworked this method entirely to support paths.
+        (WebCore::CSPSourceList::parsePath):
+            More or less a no-op at the moment.
+        (WebCore::CSPSourceList::parsePort):
+            Moved the `:` assertion here from parseSource.
+
 2012-06-16  Zeev Lieber  <[email protected]>
 
         [Chromium] Compositor should avoid drawing quads when cached textures are available and contents unchanged

Modified: trunk/Source/WebCore/page/ContentSecurityPolicy.cpp (120539 => 120540)


--- trunk/Source/WebCore/page/ContentSecurityPolicy.cpp	2012-06-17 00:26:26 UTC (rev 120539)
+++ trunk/Source/WebCore/page/ContentSecurityPolicy.cpp	2012-06-17 00:35:48 UTC (rev 120540)
@@ -77,6 +77,11 @@
     return !isASCIISpace(c);
 }
 
+bool isNotColonOrSlash(UChar c)
+{
+    return c != ':' && c != '/';
+}
+
 } // namespace
 
 static bool skipExactly(const UChar*& position, const UChar* end, UChar delimiter)
@@ -98,7 +103,7 @@
     return false;
 }
 
-static void skipUtil(const UChar*& position, const UChar* end, UChar delimiter)
+static void skipUntil(const UChar*& position, const UChar* end, UChar delimiter)
 {
     while (position < end && *position != delimiter)
         ++position;
@@ -191,6 +196,7 @@
     bool parseScheme(const UChar* begin, const UChar* end, String& scheme);
     bool parseHost(const UChar* begin, const UChar* end, String& host, bool& hostHasWildcard);
     bool parsePort(const UChar* begin, const UChar* end, int& port, bool& portHasWildcard);
+    bool parsePath(const UChar* begin, const UChar* end, String& path);
 
     void addSourceSelf();
     void addSourceStar();
@@ -263,13 +269,15 @@
 }
 
 // source            = scheme ":"
-//                   / ( [ scheme "://" ] host [ port ] )
+//                   / ( [ scheme "://" ] host [ port ] [ path ] )
 //                   / "'self'"
 //
 bool CSPSourceList::parseSource(const UChar* begin, const UChar* end,
                                 String& scheme, String& host, int& port,
                                 bool& hostHasWildcard, bool& portHasWildcard)
 {
+    String path; // FIXME: We're ignoring the path component for now.
+
     if (begin == end)
         return false;
 
@@ -294,53 +302,81 @@
     }
 
     const UChar* position = begin;
-
     const UChar* beginHost = begin;
-    skipUtil(position, end, ':');
+    const UChar* beginPath = end;
+    const UChar* beginPort = 0;
 
+    skipWhile<isNotColonOrSlash>(position, end);
+
     if (position == end) {
-        // This must be a host-only source.
-        if (!parseHost(beginHost, position, host, hostHasWildcard))
-            return false;
-        return true;
+        // host
+        //     ^
+        return parseHost(beginHost, position, host, hostHasWildcard);
     }
 
-    if (end - position == 1) {
-        ASSERT(*position == ':');
-        // This must be a scheme-only source.
-        if (!parseScheme(begin, position, scheme))
+    if (*position == '/') {
+        // host/path || host/ || /
+        //     ^            ^    ^
+        if (!parseHost(beginHost, position, host, hostHasWildcard)
+            || !parsePath(position, end, path)
+            || position != end)
             return false;
         return true;
     }
 
-    ASSERT(end - position >= 2);
-    if (position[1] == '/') {
-        if (!parseScheme(begin, position, scheme)
-            || !skipExactly(position, end, ':')
-            || !skipExactly(position, end, '/')
-            || !skipExactly(position, end, '/'))
+    if (*position == ':') {
+        if (end - position == 1) {
+            // scheme:
+            //       ^
+            return parseScheme(begin, position, scheme);
+        }
+
+        if (position[1] == '/') {
+            // scheme://host || scheme://
+            //       ^                ^
+            if (!parseScheme(begin, position, scheme)
+                || !skipExactly(position, end, ':')
+                || !skipExactly(position, end, '/')
+                || !skipExactly(position, end, '/'))
+                return false;
+            if (position == end)
+                return true;
+            beginHost = position;
+            skipWhile<isNotColonOrSlash>(position, end);
+        }
+
+        if (*position == ':') {
+            // host:port || scheme://host:port
+            //     ^                     ^
+            beginPort = position;
+            skipUntil(position, end, '/');
+        }
+    }
+    
+    if (*position == '/') {
+        // scheme://host/path || scheme://host:port/path
+        //              ^                          ^
+        if (position == beginHost)
             return false;
-        beginHost = position;
-        skipUtil(position, end, ':');
+
+        beginPath = position;
     }
 
-    if (position == beginHost)
+    if (!parseHost(beginHost, beginPort ? beginPort : beginPath, host, hostHasWildcard))
         return false;
 
-    if (!parseHost(beginHost, position, host, hostHasWildcard))
-        return false;
-
-    if (position == end) {
+    if (beginPort) {
+        if (!parsePort(beginPort, beginPath, port, portHasWildcard))
+            return false;
+    } else {
         port = 0;
-        return true;
     }
 
-    if (!skipExactly(position, end, ':'))
-        ASSERT_NOT_REACHED();
+    if (beginPath != end) {
+        if (!parsePath(beginPath, end, path))
+            return false;
+    }
 
-    if (!parsePort(position, end, port, portHasWildcard))
-        return false;
-
     return true;
 }
 
@@ -411,6 +447,19 @@
     return true;
 }
 
+// FIXME: Deal with an actual path. This just sucks up everything to the end of the string.
+bool CSPSourceList::parsePath(const UChar* begin, const UChar* end, String& path)
+{
+    ASSERT(begin <= end);
+    ASSERT(path.isEmpty());
+
+    if (begin == end)
+        return false;
+
+    path = String(begin, end - begin);
+    return true;
+}
+
 // port              = ":" ( 1*DIGIT / "*" )
 //
 bool CSPSourceList::parsePort(const UChar* begin, const UChar* end, int& port, bool& portHasWildcard)
@@ -419,6 +468,9 @@
     ASSERT(!port);
     ASSERT(!portHasWildcard);
 
+    if (!skipExactly(begin, end, ':'))
+        ASSERT_NOT_REACHED();
+
     if (begin == end)
         return false;
 
@@ -767,7 +819,7 @@
 
     while (position < end) {
         const UChar* directiveBegin = position;
-        skipUtil(position, end, ';');
+        skipUntil(position, end, ';');
 
         String name, value;
         if (parseDirective(directiveBegin, position, name, value)) {
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to