Title: [139085] trunk
Revision
139085
Author
[email protected]
Date
2013-01-08 11:22:00 -0800 (Tue, 08 Jan 2013)

Log Message

CSP: 'none' should take effect only if no other source _expression_ is present.
https://bugs.webkit.org/show_bug.cgi?id=106314

Reviewed by Adam Barth.

Source/WebCore:

WebKit's handling of 'none' in Content Security Policy source lists
doesn't quite match the spec. Currently, we're treating any source list
that contains 'none' as its first token as an empty list. That is:
"script-src 'none'" is handled in the same way as
"script-src 'none' example.com". Based on a bit of public-webappsec@
discussion[1], the behavior we actually want should treat the first as
an empty list, while treating the second as "script-src example.com". In
other words, 'none' in a source list is a no-op, unless it is the _only_
item in the source list.

This patch adjusts our parsing behavior accordingly, and tweaks the
console log we emit for invalid source expressions to warn specifically
about this case.

[1]: http://lists.w3.org/Archives/Public/public-webappsec/2013Jan/0006.html

Test: http/tests/security/contentSecurityPolicy/source-list-parsing-none.html

* page/ContentSecurityPolicy.cpp:
(WebCore::isSourceListNone):
    A new static method that returns true when given a string that
    contains only 'none' (potentially surrounded by whitespace), and
    false otherwise.
(WebCore):
(WebCore::CSPSourceList::parse):
    Move the 'none' check into the initial layer of parsing, which means
    that we can drop the 'isFirstSourceInList' check entirely.
(WebCore::CSPSourceList::parseSource):
    Since we've already checked for 'none' in ::parse, we can at this
    point safely treat any occurance of 'none' in the source list as an
    invalid _expression_.
(WebCore::ContentSecurityPolicy::reportInvalidSourceExpression):
    If the invalid _expression_ is 'none', add a clarification to the
    console message, noting that 'none' only has effect when it's all
    alone.

LayoutTests:

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

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (139084 => 139085)


--- trunk/LayoutTests/ChangeLog	2013-01-08 19:19:37 UTC (rev 139084)
+++ trunk/LayoutTests/ChangeLog	2013-01-08 19:22:00 UTC (rev 139085)
@@ -1,3 +1,13 @@
+2013-01-08  Mike West  <[email protected]>
+
+        CSP: 'none' should take effect only if no other source _expression_ is present.
+        https://bugs.webkit.org/show_bug.cgi?id=106314
+
+        Reviewed by Adam Barth.
+
+        * http/tests/security/contentSecurityPolicy/source-list-parsing-none-expected.txt: Added.
+        * http/tests/security/contentSecurityPolicy/source-list-parsing-none.html: Added.
+
 2013-01-08  Antoine Quint  <[email protected]>
 
         <track> element's mode set to "disabled" after load although it was explicitly set to "hidden"

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


--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-none-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-none-expected.txt	2013-01-08 19:22:00 UTC (rev 139085)
@@ -0,0 +1,39 @@
+CONSOLE MESSAGE: Refused to load the script 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/script.js' because it violates the following Content Security Policy directive: "script-src 'none'".
+
+CONSOLE MESSAGE: Refused to load the script 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/script.js' because it violates the following Content Security Policy directive: "script-src 'none'".
+
+CONSOLE MESSAGE: The source list for Content Security Policy directive 'script-src' contains an invalid source: ''none''. It will be ignored. Note that 'none' has no effect unless it is the only _expression_ in the source list.
+CONSOLE MESSAGE: The source list for Content Security Policy directive 'script-src' contains an invalid source: ''none''. It will be ignored. Note that 'none' has no effect unless it is the only _expression_ in the source list.
+CONSOLE MESSAGE: The source list for Content Security Policy directive 'script-src' contains an invalid source: ''none''. It will be ignored. Note that 'none' has no effect unless it is the only _expression_ in the source list.
+CONSOLE MESSAGE: The source list for Content Security Policy directive 'script-src' contains an invalid source: ''none''. It will be ignored. Note that 'none' has no effect unless it is the only _expression_ in the source list.
+CONSOLE MESSAGE: The source list for Content Security Policy directive 'script-src' contains an invalid source: ''none''. It will be ignored. Note that 'none' has no effect unless it is the only _expression_ in the source list.
+CONSOLE MESSAGE: Refused to load the script 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/script.js' because it violates the following Content Security Policy directive: "script-src 'none' 'none' 'none'".
+
+'none' should be ignored in source lists, unless it is the only token.
+
+
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame1-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame2-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame3-->-->'
+--------
+PASS
+
+--------
+Frame: '<!--framePath //<!--frame4-->-->'
+--------
+PASS

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


--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-none.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-none.html	2013-01-08 19:22:00 UTC (rev 139085)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=''></script>
+<script>
+var tests = [
+    ['no', 'script-src \'none\'', 'resources/script.js'],
+    ['no', 'script-src      \'none\'     ', 'resources/script.js'],
+    ['yes', 'script-src \'none\' http://127.0.0.1:8000', 'resources/script.js'],
+    ['yes', 'script-src http://127.0.0.1:8000 \'none\'', 'resources/script.js'],
+    ['no', 'script-src \'none\' \'none\' \'none\'', 'resources/script.js'],
+];
+</script>
+</head>
+<body _onload_="test()">
+    <p>'none' should be ignored in source lists, unless it is the only token.</p>
+</body>

Modified: trunk/Source/WebCore/ChangeLog (139084 => 139085)


--- trunk/Source/WebCore/ChangeLog	2013-01-08 19:19:37 UTC (rev 139084)
+++ trunk/Source/WebCore/ChangeLog	2013-01-08 19:22:00 UTC (rev 139085)
@@ -1,3 +1,46 @@
+2013-01-08  Mike West  <[email protected]>
+
+        CSP: 'none' should take effect only if no other source _expression_ is present.
+        https://bugs.webkit.org/show_bug.cgi?id=106314
+
+        Reviewed by Adam Barth.
+
+        WebKit's handling of 'none' in Content Security Policy source lists
+        doesn't quite match the spec. Currently, we're treating any source list
+        that contains 'none' as its first token as an empty list. That is:
+        "script-src 'none'" is handled in the same way as
+        "script-src 'none' example.com". Based on a bit of public-webappsec@
+        discussion[1], the behavior we actually want should treat the first as
+        an empty list, while treating the second as "script-src example.com". In
+        other words, 'none' in a source list is a no-op, unless it is the _only_
+        item in the source list.
+
+        This patch adjusts our parsing behavior accordingly, and tweaks the
+        console log we emit for invalid source expressions to warn specifically
+        about this case.
+
+        [1]: http://lists.w3.org/Archives/Public/public-webappsec/2013Jan/0006.html
+
+        Test: http/tests/security/contentSecurityPolicy/source-list-parsing-none.html
+
+        * page/ContentSecurityPolicy.cpp:
+        (WebCore::isSourceListNone):
+            A new static method that returns true when given a string that
+            contains only 'none' (potentially surrounded by whitespace), and
+            false otherwise.
+        (WebCore):
+        (WebCore::CSPSourceList::parse):
+            Move the 'none' check into the initial layer of parsing, which means
+            that we can drop the 'isFirstSourceInList' check entirely.
+        (WebCore::CSPSourceList::parseSource):
+            Since we've already checked for 'none' in ::parse, we can at this
+            point safely treat any occurance of 'none' in the source list as an
+            invalid _expression_.
+        (WebCore::ContentSecurityPolicy::reportInvalidSourceExpression):
+            If the invalid _expression_ is 'none', add a clarification to the
+            console message, noting that 'none' only has effect when it's all
+            alone.
+
 2013-01-08  Alok Priyadarshi  <[email protected]>
 
         Added OpaqueRegionSkia::currentTrackingOpaqueRect

Modified: trunk/Source/WebCore/page/ContentSecurityPolicy.cpp (139084 => 139085)


--- trunk/Source/WebCore/page/ContentSecurityPolicy.cpp	2013-01-08 19:19:37 UTC (rev 139084)
+++ trunk/Source/WebCore/page/ContentSecurityPolicy.cpp	2013-01-08 19:22:00 UTC (rev 139085)
@@ -202,6 +202,24 @@
         ++position;
 }
 
+static bool isSourceListNone(const String& value)
+{
+    const UChar* begin = value.characters();
+    const UChar* end = value.characters() + value.length();
+    skipWhile<isASCIISpace>(begin, end);
+
+    const UChar* position = begin;
+    skipWhile<isSourceCharacter>(position, end);
+    if (!equalIgnoringCase("'none'", begin, position - begin))
+        return false;
+
+    skipWhile<isASCIISpace>(position, end);
+    if (position != end)
+        return false;
+
+    return true;
+}
+
 class CSPSource {
 public:
     CSPSource(const String& scheme, const String& host, int port, const String& path, bool hostHasWildcard, bool portHasWildcard)
@@ -323,6 +341,9 @@
 
 void CSPSourceList::parse(const String& value)
 {
+    // We represent 'none' as an empty m_list.
+    if (isSourceListNone(value))
+        return;
     parse(value.characters(), value.characters() + value.length());
 }
 
@@ -348,7 +369,6 @@
 {
     const UChar* position = begin;
 
-    bool isFirstSourceInList = true;
     while (position < end) {
         skipWhile<isASCIISpace>(position, end);
         if (position == end)
@@ -357,10 +377,6 @@
         const UChar* beginSource = position;
         skipWhile<isSourceCharacter>(position, end);
 
-        if (isFirstSourceInList && equalIgnoringCase("'none'", beginSource, position - beginSource))
-            return; // We represent 'none' as an empty m_list.
-        isFirstSourceInList = false;
-
         String scheme, host, path;
         int port = 0;
         bool hostHasWildcard = false;
@@ -395,6 +411,9 @@
     if (begin == end)
         return false;
 
+    if (equalIgnoringCase("'none'", begin, end - begin))
+        return false;
+
     if (end - begin == 1 && *begin == '*') {
         addSourceStar();
         return true;
@@ -1709,6 +1728,8 @@
 void ContentSecurityPolicy::reportInvalidSourceExpression(const String& directiveName, const String& source) const
 {
     String message = makeString("The source list for Content Security Policy directive '", directiveName, "' contains an invalid source: '", source, "'. It will be ignored.");
+    if (equalIgnoringCase(source, "'none'"))
+        message = makeString(message, " Note that 'none' has no effect unless it is the only _expression_ in the source list.");
     logToConsole(message);
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to