From a318c5d3937989dc17ab08b5cb0899b2903597b9 Mon Sep 17 00:00:00 2001 From: "Andrew R. Whalley" Date: Thu, 31 May 2018 16:06:40 +0000 Subject: [PATCH] [M68 Merge] Fixed CSP directive value parsing accepted character range TBR=andypaicu@chromium.org (cherry picked from commit 5b8466d84c801a9dfe140cb9a9f6be3a8caba230) Bug: 845961 Change-Id: Ifc9609058cd7cbd268785db46534e3ed09da6ce3 Reviewed-on: https://chromium-review.googlesource.com/1071510 Commit-Queue: Andy Paicu Reviewed-by: Mike West Cr-Original-Commit-Position: refs/heads/master@{#561834} Reviewed-on: https://chromium-review.googlesource.com/1080929 Reviewed-by: Andrew Whalley Cr-Commit-Position: refs/branch-heads/3440@{#62} Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733} --- .../required_csp-header-crlf.html | 144 ++++++++++++++++++ .../support/echo-required-csp.py | 7 +- .../support/testharness-helper.sub.js | 4 + .../core/frame/csp/content_security_policy.cc | 4 + .../frame/csp/content_security_policy_test.cc | 28 ++++ 5 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header-crlf.html diff --git a/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header-crlf.html b/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header-crlf.html new file mode 100644 index 000000000000..87bda1bf502d --- /dev/null +++ b/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header-crlf.html @@ -0,0 +1,144 @@ + + + +Embedded Enforcement: Sec-Required-CSP header. + + + + + + + + diff --git a/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/echo-required-csp.py b/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/echo-required-csp.py index 930a1f608538..6063cc046ba7 100644 --- a/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/echo-required-csp.py +++ b/third_party/WebKit/LayoutTests/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/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/testharness-helper.sub.js b/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/testharness-helper.sub.js index 127a94ba359f..6d2e64f8f7e9 100644 --- a/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/embedded-enforcement/support/testharness-helper.sub.js +++ b/third_party/WebKit/LayoutTests/external/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 index 51399e7d6e08..8f9382c7f67d 100644 --- 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 index e25bfc9de4b1..495b4e4ad502 100644 --- 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