[M68 Merge] Fixed CSP directive value parsing accepted character range

[email protected] (cherry picked from commit 5b8466d) Bug: 845961 Change-Id: Ifc9609058cd7cbd268785db46534e3ed09da6ce3 Reviewed-on: https://chromium-review.googlesource.com/1071510 Commit-Queue: Andy Paicu <[email protected]> Reviewed-by: Mike West <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#561834} Reviewed-on: https://chromium-review.googlesource.com/1080929 Reviewed-by: Andrew Whalley <[email protected]> Cr-Commit-Position: refs/branch-heads/3440@{#62} Cr-Branched-From: 010ddcf-refs/heads/master@{#561733}
nwjs · May 31, 2018 · a318c5d · a318c5d
1 parent 33cf620
commit a318c5d
Show file tree

Hide file tree

Showing 5 changed files with 186 additions and 1 deletion.
diff --git a/...s/external/wpt/content-security-policy/embedded-enforcement/required_csp-header-crlf.html b/...s/external/wpt/content-security-policy/embedded-enforcement/required_csp-header-crlf.html
@@ -0,0 +1,144 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Embedded Enforcement: Sec-Required-CSP header.</title>
+  <script src="/resources/testharness.js"></script>
+  <script src="/resources/testharnessreport.js"></script>
+  <script src="support/testharness-helper.sub.js"></script>
+</head>
+<body>
+  <script>
+    var tests = [
+      // CRLF characters
+      { "name": "\\r\\n character after directive name",
+        "csp": "script-src\r\n'unsafe-inline'",
+        "expected": null },
+      { "name": "\\r\\n character in directive value",
+        "csp": "script-src 'unsafe-inline'\r\n'unsafe-eval'",
+        "expected": null },
+      { "name": "\\n character after directive name",
+        "csp": "script-src\n'unsafe-inline'",
+        "expected": null },
+      { "name": "\\n character in directive value",
+        "csp": "script-src 'unsafe-inline'\n'unsafe-eval'",
+        "expected": null },
+      { "name": "\\r character after directive name",
+        "csp": "script-src\r'unsafe-inline'",
+        "expected": null },
+      { "name": "\\r character in directive value",
+        "csp": "script-src 'unsafe-inline'\r'unsafe-eval'",
+        "expected": null },
+
+      // HTML encoded CRLF characters
+      { "name": "%0D%0A character after directive name",
+        "csp": "script-src%0D%0A'unsafe-inline'",
+        "expected": null },
+      { "name": "%0D%0A character in directive value",
+        "csp": "script-src 'unsafe-inline'%0D%0A'unsafe-eval'",
+        "expected": null },
+      { "name": "%0A character after directive name",
+        "csp": "script-src%0A'unsafe-inline'",
+        "expected": null },
+      { "name": "%0A character in directive value",
+        "csp": "script-src 'unsafe-inline'%0A'unsafe-eval'",
+        "expected": null },
+      { "name": "%0D character after directive name",
+        "csp": "script-src%0D'unsafe-inline'",
+        "expected": null },
+      { "name": "%0D character in directive value",
+        "csp": "script-src 'unsafe-inline'%0D'unsafe-eval'",
+        "expected": null },
+
+      // Attempt HTTP Header injection
+      { "name": "Attempt injecting after directive name using \\r\\n",
+        "csp": "script-src\r\nTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after directive name using \\r",
+        "csp": "script-src\rTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after directive name using \\n",
+        "csp": "script-src\nTest-Header-Injection: dummy",
+        "expected": null },
+
+      { "name": "Attempt injecting after directive value using \\r\\n",
+        "csp": "script-src example.com\r\nTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after directive value using \\r",
+        "csp": "script-src example.com\rTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after directive value using \\n",
+        "csp": "script-src example.com\nTest-Header-Injection: dummy",
+        "expected": null },
+
+      { "name": "Attempt injecting after semicolon using \\r\\n",
+        "csp": "script-src example.com;\r\nTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after semicolon using \\r",
+        "csp": "script-src example.com;\rTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after semicolon using \\n",
+        "csp": "script-src example.com;\nTest-Header-Injection: dummy",
+        "expected": null },
+
+      { "name": "Attempt injecting after space between name and value using \\r\\n",
+        "csp": "script-src \r\nTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after space between name and value using \\r",
+        "csp": "script-src \rTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after space between name and value using \\n",
+        "csp": "script-src \nTest-Header-Injection: dummy",
+        "expected": null },
+
+      // Attempt HTTP Header injection using URL encoded characters
+      { "name": "Attempt injecting after directive name using %0D%0A",
+        "csp": "script-src%0D%0ATest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after directive name using %0D",
+        "csp": "script-src%0DTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after directive name using %0A",
+        "csp": "script-src%0ATest-Header-Injection: dummy",
+        "expected": null },
+
+      { "name": "Attempt injecting after directive value using %0D%0A",
+        "csp": "script-src example.com%0D%0ATest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after directive value using %0D",
+        "csp": "script-src example.com%0DTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after directive value using %0A",
+        "csp": "script-src example.com%0ATest-Header-Injection: dummy",
+        "expected": null },
+
+      { "name": "Attempt injecting after semicolon using %0D%0A",
+        "csp": "script-src example.com;%0D%0ATest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after semicolon using %0D",
+        "csp": "script-src example.com;%0DTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after semicolon using %0A",
+        "csp": "script-src example.com;%0ATest-Header-Injection: dummy",
+        "expected": null },
+
+      { "name": "Attempt injecting after space between name and value using %0D%0A",
+        "csp": "script-src %0D%0ATest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after space between name and value using %0D",
+        "csp": "script-src %0DTest-Header-Injection: dummy",
+        "expected": null },
+      { "name": "Attempt injecting after space between name and value using %0A",
+        "csp": "script-src %0ATest-Header-Injection: dummy",
+        "expected": null },
+
+    ];
+
+    tests.forEach(test => {
+      async_test(t =>  {
+        var url = generateURLString(Host.SAME_ORIGIN, PolicyHeader.REQUIRED_CSP);
+        assert_required_csp(t, url, test.csp, [test.expected]);
+      }, "Test CRLF: " + test.name);
+    });
+  </script>
+</body>
+</html>
diff --git a/...ts/external/wpt/content-security-policy/embedded-enforcement/support/echo-required-csp.py b/...ts/external/wpt/content-security-policy/embedded-enforcement/support/echo-required-csp.py
@@ -1,8 +1,13 @@
 import json
 def main(request, response):
-    header = request.headers.get("Sec-Required-CSP");
     message = {}
+
+    header = request.headers.get("Test-Header-Injection");
+    message['test_header_injection'] = header if header else None
+
+    header = request.headers.get("Sec-Required-CSP");
     message['required_csp'] = header if header else None
+
     second_level_iframe_code = ""
     if "include_second_level_iframe" in request.GET:
        if "second_level_iframe_csp" in request.GET and request.GET["second_level_iframe_csp"] <> "":

diff --git a/...ternal/wpt/content-security-policy/embedded-enforcement/support/testharness-helper.sub.js b/...ternal/wpt/content-security-policy/embedded-enforcement/support/testharness-helper.sub.js
@@ -91,6 +91,10 @@ function assert_required_csp(t, url, csp, expected) {
       assert_unreached('Child iframes have unexpected csp:"' + e.data['required_csp'] + '"');
 
     expected.splice(expected.indexOf(e.data['required_csp']), 1);
+
+    if (e.data['test_header_injection'] != null)
+      assert_unreached('HTTP header injection was successful');
+
     if (expected.length == 0)
       t.done();
   }));

diff --git a/third_party/blink/renderer/core/frame/csp/content_security_policy.cc b/third_party/blink/renderer/core/frame/csp/content_security_policy.cc
@@ -1843,6 +1843,10 @@ bool ContentSecurityPolicy::ShouldBypassContentSecurityPolicy(
 // static
 bool ContentSecurityPolicy::IsValidCSPAttr(const String& attr,
                                            const String& context_required_csp) {
+  // we don't allow any newline characters in the CSP attributes
+  if (attr.Contains('\n') || attr.Contains('\r'))
+    return false;
+
   ContentSecurityPolicy* attr_policy = ContentSecurityPolicy::Create();
   attr_policy->AddPolicyFromHeaderValue(attr,
                                         kContentSecurityPolicyHeaderTypeEnforce,

diff --git a/third_party/blink/renderer/core/frame/csp/content_security_policy_test.cc b/third_party/blink/renderer/core/frame/csp/content_security_policy_test.cc
@@ -1348,6 +1348,34 @@ TEST_F(ContentSecurityPolicyTest, IsValidCSPAttrTest) {
       "report-to relative-path/reporting;"
       "base-uri http://example.com 'self'",
       ""));
+
+  // CRLF should not be allowed
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "base-uri\nhttp://example.com", ""));
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "base-uri http://example.com\nhttp://example2.com", ""));
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "base\n-uri http://example.com", ""));
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "\nbase-uri http://example.com", ""));
+
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "base-uri\r\nhttp://example.com", ""));
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "base-uri http://example.com\r\nhttp://example2.com", ""));
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "base\r\n-uri http://example.com", ""));
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "\r\nbase-uri http://example.com", ""));
+
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "base-uri\rhttp://example.com", ""));
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "base-uri http://example.com\rhttp://example2.com", ""));
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "base\r-uri http://example.com", ""));
+  EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(
+      "\rbase-uri http://example.com", ""));
 }
 
 }  // namespace blink