- 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);
}