github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-04-10 03:01:51 -04:00
Code Issues Packages Projects Releases Wiki Activity

chore: cherry-pick 8af66de55aad from chromium (#31523)

Browse Source 
* chore: cherry-pick 8af66de55aad from chromium

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: Electron Bot <electron@github.com>
This commit is contained in:
Pedro Pontes
2021-10-25 02:47:11 +02:00
committed by GitHub
parent c98059abd9
commit c2023dd9f5
2 changed files with 127 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
patches/chromium/.patches
View File

@@ -107,5 +107,6 @@ fix_media_key_usage_with_globalshortcuts.patch
cherry-pick-ec42dfd3545f.patch
cherry-pick-39090918efac.patch
mas_gate_private_enterprise_APIs
cherry-pick-8af66de55aad.patch
cherry-pick-c69dddfe1cde.patch
cherry-pick-2e7c9b33453b.patch

126
patches/chromium/cherry-pick-8af66de55aad.patch Normal file
View File

@@ -0,0 +1,126 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Antonio Sartori <antoniosartori@chromium.org>
Date: Tue, 24 Aug 2021 15:01:17 +0000
Subject: Limit length of 'csp' attribute
Most servers limit the length of request headers anywhere. 4Kb seems
like a reasonable limit, which some popular http servers have by
default, and which we already enforce for Referer
(https://crrev.com/c/1595872).
I would have liked the constant 4096 to be shared between //content
and blink. This would have required putting it somewhere like in
//services/network or in //third_party/blink/common, creating a new
file for it. I thought it would be easier to avoid that for this
change.
It would be safer to not load the iframe document, or to impose some
very strict CSP like "default-src 'none'", instead than just ignoring
the 'csp' attribute if that's too long. However, ignoring is what we
already do if the attribute contains illegal characters or does not
match the CSP grammary or is not subsumed by the parent iframe's csp
attribute. For this change, I believe it's better to stay consistent
with that, and later change the CSPEE code to block loading in all
those cases.
Bug: 1233067
Change-Id: Ie9cd3db82287a76892cca76a0bf0d4a1613a3055
Fixed: 1233067
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3057048
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/main@{#914730}
diff --git a/content/browser/content_security_policy_browsertest.cc b/content/browser/content_security_policy_browsertest.cc
index 1d0631955600449d142697ce68c474f1957eae75..f95fe16e3c3f8c8b6c603f7cd19dcdb915deacfa 100644
--- a/content/browser/content_security_policy_browsertest.cc
+++ b/content/browser/content_security_policy_browsertest.cc
@@ -225,4 +225,21 @@ IN_PROC_BROWSER_TEST_F(ContentSecurityPolicyBrowserTest, FileURLs) {
}
}
+// Test that a 'csp' attribute longer than 4096 bytes is ignored.
+IN_PROC_BROWSER_TEST_F(ContentSecurityPolicyBrowserTest, CSPAttributeTooLong) {
+ std::string long_csp_attribute = "script-src 'none' ";
+ long_csp_attribute.resize(4097, 'a');
+ std::string page = "data:text/html,<body><iframe csp=\"" +
+ long_csp_attribute + "\"></iframe></body>";
+
+ GURL url(page);
+ WebContentsConsoleObserver console_observer(web_contents());
+ console_observer.SetPattern("'csp' attribute too long*");
+ EXPECT_TRUE(NavigateToURL(shell(), url));
+ console_observer.Wait();
+
+ EXPECT_EQ(current_frame_host()->child_count(), 1u);
+ EXPECT_FALSE(current_frame_host()->child_at(0)->csp_attribute());
+}
+
} // namespace content
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index a0c056f47b9582536f9c8ee93517c872bfadf2e3..72c08a7ac135149b66f6710ff01b4922df5aba24 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -821,9 +821,11 @@ enum class VerifyDidCommitParamsDifference {
};
bool ValidateCSPAttribute(const std::string& value) {
+ static const size_t kMaxLengthCSPAttribute = 4096;
if (!base::IsStringASCII(value))
return false;
- if (value.find('\n') != std::string::npos ||
+ if (value.length() > kMaxLengthCSPAttribute ||
+ value.find('\n') != std::string::npos ||
value.find('\r') != std::string::npos) {
return false;
}
diff --git a/third_party/blink/renderer/core/html/html_iframe_element.cc b/third_party/blink/renderer/core/html/html_iframe_element.cc
index c60e3281ea96ef39e16034c88abf4c6c2a9bd2eb..f9434ae7a2c1066dd2c97f552ee572ca4d808f24 100644
--- a/third_party/blink/renderer/core/html/html_iframe_element.cc
+++ b/third_party/blink/renderer/core/html/html_iframe_element.cc
@@ -208,16 +208,27 @@ void HTMLIFrameElement::ParseAttribute(
UpdateContainerPolicy();
}
} else if (name == html_names::kCspAttr) {
+ static const size_t kMaxLengthCSPAttribute = 4096;
if (value && (value.Contains('\n') || value.Contains('\r') ||
!MatchesTheSerializedCSPGrammar(value.GetString()))) {
+ // TODO(antoniosartori): It would be safer to block loading iframes with
+ // invalid 'csp' attribute.
required_csp_ = g_null_atom;
GetDocument().AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
mojom::blink::ConsoleMessageSource::kOther,
mojom::blink::ConsoleMessageLevel::kError,
"'csp' attribute is invalid: " + value));
- return;
- }
- if (required_csp_ != value) {
+ } else if (value && value.length() > kMaxLengthCSPAttribute) {
+ // TODO(antoniosartori): It would be safer to block loading iframes with
+ // invalid 'csp' attribute.
+ required_csp_ = g_null_atom;
+ GetDocument().AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+ mojom::blink::ConsoleMessageSource::kOther,
+ mojom::blink::ConsoleMessageLevel::kError,
+ String::Format("'csp' attribute too long. The max length for the "
+ "'csp' attribute is %zu bytes.",
+ kMaxLengthCSPAttribute)));
+ } else if (required_csp_ != value) {
required_csp_ = value;
DidChangeAttributes();
UseCounter::Count(GetDocument(), WebFeature::kIFrameCSPAttribute);
diff --git a/third_party/blink/web_tests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header.html b/third_party/blink/web_tests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header.html
index a9ad787408786e594ccb554d2bd9186a9e8e7c1e..e0a31db8e28fb1a9d2884c7677597072d4badba2 100644
--- a/third_party/blink/web_tests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header.html
+++ b/third_party/blink/web_tests/external/wpt/content-security-policy/embedded-enforcement/required_csp-header.html
@@ -59,6 +59,9 @@
{ "name": "Wrong and dangerous value of `csp` should not trigger sending Sec-Required-CSP Header - report-to present",
"csp": "script-src 'unsafe-inline'; report-to resources/dummy-report.php",
"expected": null },
+ { "name": "Sec-Required-CSP is not sent if `csp` attribute is longer than 4096 bytes",
+ "csp": "style-src " + Array.from(Array(2044).keys()).map(i => 'a').join(' '),
+ "expected": null },
];
tests.forEach(test => {