Source/WebCore: Content Security Policy directives that begin with an invalid charact...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2012 20:08:23 +0000 (20:08 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2012 20:08:23 +0000 (20:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=93622

Patch by Mike West <mkwst@chromium.org> on 2012-08-09
Reviewed by Adam Barth.

CSP directives containing unrecognized characters somewhere in the
middle were caught and logged by the current algorithm. We additionally
caught the case in which the directive begins with an invalid character,
but we silently ignored it. Now we're slightly more vocal.

This change also exits 'parseDirective' early in the case where the
directive text is empty, or filled only with whitespace. There doesn't
seem to be any value in flagging that case, as it has no impact on the
way the policy would have been interpreted (that is, 'img-src *;;...'
doesn't change in meaning by ignoring the empty directive between the
semicolons).

Test: http/tests/security/contentSecurityPolicy/directive-parsing-05.html

* page/ContentSecurityPolicy.cpp:
(WebCore::CSPDirectiveList::parseDirective):
    Two changes: first, we now exit early if the entire directive text
    is empty (e.g. ';;;' or ';      ;'); second, if the directive begins
    with a character that doesn't match 'isDirectiveNameCharacter', then
    we advance either to the next space, or the end of the value, and
    report an unrecognized directive to the policy.

LayoutTests: Content Security Policy directives with an invalid character should log a console warning.
https://bugs.webkit.org/show_bug.cgi?id=93622

Patch by Mike West <mkwst@chromium.org> on 2012-08-09
Reviewed by Adam Barth.

* http/tests/security/contentSecurityPolicy/directive-parsing-05-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/directive-parsing-05.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/security/contentSecurityPolicy/directive-parsing-05-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/contentSecurityPolicy/directive-parsing-05.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/ContentSecurityPolicy.cpp

index 67193cb..5dfd2e5 100644 (file)
@@ -1,3 +1,13 @@
+2012-08-09  Mike West  <mkwst@chromium.org>
+
+        Content Security Policy directives with an invalid character should log a console warning.
+        https://bugs.webkit.org/show_bug.cgi?id=93622
+
+        Reviewed by Adam Barth.
+
+        * http/tests/security/contentSecurityPolicy/directive-parsing-05-expected.txt: Added.
+        * http/tests/security/contentSecurityPolicy/directive-parsing-05.html: Added.
+
 2012-08-09  Ryosuke Niwa  <rniwa@webkit.org>
 
         Fix a test after r125178.
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/directive-parsing-05-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/directive-parsing-05-expected.txt
new file mode 100644 (file)
index 0000000..dc9c3c2
--- /dev/null
@@ -0,0 +1,10 @@
+CONSOLE MESSAGE: Unrecognized Content-Security-Policy directive ':script-src'.
+
+Directives starting with an invalid character should be logged and ignored.
+
+
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+PASS
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/directive-parsing-05.html b/LayoutTests/http/tests/security/contentSecurityPolicy/directive-parsing-05.html
new file mode 100644 (file)
index 0000000..5339887
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+  testRunner.dumpAsText();
+  testRunner.dumpChildFramesAsText();
+}
+</script>
+</head>
+<body>
+  <p>
+    Directives starting with an invalid character should be logged and ignored.
+  </p>
+  <iframe src="http://127.0.0.1:8000/security/contentSecurityPolicy/resources/echo-script-src.pl?should_run=yes&q=http://127.0.0.1:8000/security/contentSecurityPolicy/resources/script.js&csp=:script-src%20'none'"></iframe>
+</body>
+</html>
+
index 30031c1..f0897a9 100644 (file)
@@ -1,3 +1,32 @@
+2012-08-09  Mike West  <mkwst@chromium.org>
+
+        Content Security Policy directives that begin with an invalid character should log a console warning.
+        https://bugs.webkit.org/show_bug.cgi?id=93622
+
+        Reviewed by Adam Barth.
+
+        CSP directives containing unrecognized characters somewhere in the
+        middle were caught and logged by the current algorithm. We additionally
+        caught the case in which the directive begins with an invalid character,
+        but we silently ignored it. Now we're slightly more vocal.
+
+        This change also exits 'parseDirective' early in the case where the
+        directive text is empty, or filled only with whitespace. There doesn't
+        seem to be any value in flagging that case, as it has no impact on the
+        way the policy would have been interpreted (that is, 'img-src *;;...'
+        doesn't change in meaning by ignoring the empty directive between the
+        semicolons).
+
+        Test: http/tests/security/contentSecurityPolicy/directive-parsing-05.html
+
+        * page/ContentSecurityPolicy.cpp:
+        (WebCore::CSPDirectiveList::parseDirective):
+            Two changes: first, we now exit early if the entire directive text
+            is empty (e.g. ';;;' or ';      ;'); second, if the directive begins
+            with a character that doesn't match 'isDirectiveNameCharacter', then
+            we advance either to the next space, or the end of the value, and
+            report an unrecognized directive to the policy.
+
 2012-08-09  Shawn Singh  <shawnsingh@chromium.org>
 
         [chromium] Pass mask scale and offset to shaders for correct masking
index a0a6e24..078a5f7 100644 (file)
@@ -874,12 +874,19 @@ bool CSPDirectiveList::parseDirective(const UChar* begin, const UChar* end, Stri
     const UChar* position = begin;
     skipWhile<isASCIISpace>(position, end);
 
+    // Empty directive (e.g. ";;;"). Exit early.
+    if (position == end)
+        return false;
+
     const UChar* nameBegin = position;
     skipWhile<isDirectiveNameCharacter>(position, end);
 
     // The directive-name must be non-empty.
-    if (nameBegin == position)
+    if (nameBegin == position) {
+        skipWhile<isNotASCIISpace>(position, end);
+        m_policy->reportUnrecognizedDirective(String(nameBegin, position - nameBegin));
         return false;
+    }
 
     name = String(nameBegin, position - nameBegin);