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 b03797bdb1df from chromium (#34631)

Browse Source 
* chore: cherry-pick b03797bdb1df from chromium

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
This commit is contained in:
Pedro Pontes
2022-06-20 02:33:19 +02:00
committed by GitHub
parent 4f70332460
commit 977dc2527e
2 changed files with 196 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
patches/chromium/.patches
View File

@@ -127,3 +127,4 @@ cherry-pick-2782c7bc5bbe.patch
cherry-pick-f3d01ff794dc.patch
cherry-pick-919b1ffe1fe7.patch
cherry-pick-f1dd785e021e.patch
cherry-pick-b03797bdb1df.patch

195
patches/chromium/cherry-pick-b03797bdb1df.patch Normal file
View File

@@ -0,0 +1,195 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon, 2 May 2022 23:43:40 +0000
Subject: Markup sanitization should iterate until markup is stable
There are cases where parsing a markup and then serializing it does not
round trip, which can be used to inject XSS. Current sanitization code
only does one round of parsing and serializing, which does not remove
XSS injections that hide deeper.
Hence this patch makes sanitization algorithm iterate until the markup
is stable, or declares failure if it doesn't stabilize after many tries.
(cherry picked from commit 19280353fb92d0ff7d048d7cec28d6a23befbce0)
Fixed: 1315563
Change-Id: I4a3ebe1fda6df0e04a24d863b2b48df2110af209
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3611826
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#997032}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3621618
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/branch-heads/5005@{#363}
Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738}
diff --git a/third_party/blink/renderer/core/editing/serializers/serialization.cc b/third_party/blink/renderer/core/editing/serializers/serialization.cc
index 4ee332fa16a1bda54e6ba99c8063876fb046adaa..463fd9a37671b92c9af68ff5f3c0e8ddd5f1c62c 100644
--- a/third_party/blink/renderer/core/editing/serializers/serialization.cc
+++ b/third_party/blink/renderer/core/editing/serializers/serialization.cc
@@ -835,6 +835,12 @@ static bool StripSVGUseDataURLs(Node& node) {
return stripped;
}
+namespace {
+
+constexpr unsigned kMaxSanitizationIterations = 16;
+
+} // namespace
+
DocumentFragment* CreateSanitizedFragmentFromMarkupWithContext(
Document& document,
const String& raw_markup,
@@ -844,42 +850,56 @@ DocumentFragment* CreateSanitizedFragmentFromMarkupWithContext(
if (raw_markup.IsEmpty())
return nullptr;
- Document* staging_document = CreateStagingDocumentForMarkupSanitization(
- *document.GetFrame()->GetFrameScheduler()->GetAgentGroupScheduler());
- Element* body = staging_document->body();
-
- DocumentFragment* fragment = CreateFragmentFromMarkupWithContext(
- *staging_document, raw_markup, fragment_start, fragment_end, KURL(),
- kDisallowScriptingAndPluginContent);
- if (!fragment) {
- staging_document->GetPage()->WillBeDestroyed();
- return nullptr;
- }
+ // Iterate on parsing, sanitization and serialization until the markup is
+ // stable, or if we have exceeded the maximum allowed number of iterations.
+ String last_markup;
+ String markup = raw_markup;
+ for (unsigned iteration = 0;
+ iteration < kMaxSanitizationIterations && last_markup != markup;
+ ++iteration) {
+ last_markup = markup;
+
+ Document* staging_document = CreateStagingDocumentForMarkupSanitization(
+ *document.GetFrame()->GetFrameScheduler()->GetAgentGroupScheduler());
+ Element* body = staging_document->body();
+
+ DocumentFragment* fragment = CreateFragmentFromMarkupWithContext(
+ *staging_document, last_markup, fragment_start, fragment_end, KURL(),
+ kDisallowScriptingAndPluginContent);
+ if (!fragment) {
+ staging_document->GetPage()->WillBeDestroyed();
+ return nullptr;
+ }
- bool needs_sanitization = false;
- if (ContainsStyleElements(*fragment))
- needs_sanitization = true;
- if (StripSVGUseDataURLs(*fragment))
- needs_sanitization = true;
+ bool needs_sanitization = false;
+ if (ContainsStyleElements(*fragment))
+ needs_sanitization = true;
+ if (StripSVGUseDataURLs(*fragment))
+ needs_sanitization = true;
- if (!needs_sanitization) {
+ if (!needs_sanitization) {
+ markup = CreateMarkup(fragment);
+ } else {
+ body->appendChild(fragment);
+ staging_document->UpdateStyleAndLayout(DocumentUpdateReason::kEditing);
+
+ // This sanitizes stylesheets in the markup into element inline styles
+ markup = CreateMarkup(Position::FirstPositionInNode(*body),
+ Position::LastPositionInNode(*body),
+ CreateMarkupOptions::Builder()
+ .SetShouldAnnotateForInterchange(true)
+ .SetIsForMarkupSanitization(true)
+ .Build());
+ }
staging_document->GetPage()->WillBeDestroyed();
- return CreateFragmentFromMarkupWithContext(
- document, raw_markup, fragment_start, fragment_end, base_url,
- kDisallowScriptingAndPluginContent);
+
+ fragment_start = 0;
+ fragment_end = markup.length();
}
- body->appendChild(fragment);
- staging_document->UpdateStyleAndLayout(DocumentUpdateReason::kEditing);
-
- // This sanitizes stylesheets in the markup into element inline styles
- String markup = CreateMarkup(Position::FirstPositionInNode(*body),
- Position::LastPositionInNode(*body),
- CreateMarkupOptions::Builder()
- .SetShouldAnnotateForInterchange(true)
- .SetIsForMarkupSanitization(true)
- .Build());
- staging_document->GetPage()->WillBeDestroyed();
+ // Sanitization failed because markup can't stabilize.
+ if (last_markup != markup)
+ return nullptr;
return CreateFragmentFromMarkup(document, markup, base_url,
kDisallowScriptingAndPluginContent);
diff --git a/third_party/blink/web_tests/editing/pasteboard/paste-svg-use.html b/third_party/blink/web_tests/editing/pasteboard/paste-svg-use.html
index c03ca90d492a5e2af883f68811baa7660ef1cdc2..1af77f8faa4501f577e28297f013e170775bc86a 100644
--- a/third_party/blink/web_tests/editing/pasteboard/paste-svg-use.html
+++ b/third_party/blink/web_tests/editing/pasteboard/paste-svg-use.html
@@ -32,6 +32,6 @@ selection_test(
<use href=data:application/xml;base64,PHN2ZyBpZD0neCcgeG1sbnM9J2h0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnJz4KPGEgaHJlZj0namF2YXNjcmlwdDphbGVydCgxMjMpJz4KICAgIDxyZWN0IHdpZHRoPScxMDAlJyBoZWlnaHQ9JzEwMCUnIGZpbGw9J2xpZ2h0Ymx1ZScgLz4KICAgICA8dGV4dCB4PScwJyB5PScwJyBmaWxsPSdibGFjayc+CiAgICAgICA8dHNwYW4geD0nMCcgZHk9JzEuMmVtJz5Pb3BzLCB0aGVyZSdzIHNvbWV0aGluZyB3cm9uZyB3aXRoIHRoZSBwYWdlITwvdHNwYW4+CiAgICAgPHRzcGFuIHg9JzAnIGR5PScxLjJlbSc+UGxlYXNlIGNsaWNrIGhlcmUgdG8gcmVsb2FkLjwvdHNwYW4+Cjwvc3ZnPg==#x>"></noscript>asdasd`);
selection.document.execCommand('paste');
},
- '<div contenteditable>|<noscript>&lt;u title="</noscript><div contenteditable="false"><svg></svg></div></div>',
+ '<div contenteditable><div><br></div><div>      <u title="</div><div>      </div><div>      ">asdasd|</div></div>',
'Paste blocks data URI in SVG use element injection via <noscript>');
</script>
diff --git a/third_party/blink/web_tests/external/wpt/clipboard-apis/async-navigator-clipboard-read-sanitize.https.html b/third_party/blink/web_tests/external/wpt/clipboard-apis/async-navigator-clipboard-read-sanitize.https.html
new file mode 100644
index 0000000000000000000000000000000000000000..9e0ab2ee740f85ea4e9de9d3f1d2ac43bee5a985
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/clipboard-apis/async-navigator-clipboard-read-sanitize.https.html
@@ -0,0 +1,44 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Async Clipboard.read() should sanitize text/html</title>
+<link rel="help" href="https://w3c.github.io/clipboard-apis/#dom-clipboard-read">
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1315563">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+
+<p><button id="button">Put payload in the clipboard</button></p>
+<div id="output"></div>
+
+<script>
+let testFailed = false;
+function fail() {
+ testFailed = true;
+}
+
+button.onclick = () => document.execCommand('copy');
+document.oncopy = ev => {
+ ev.preventDefault();
+ ev.clipboardData.setData(
+ 'text/html',
+ `<form><math><mtext></form><form><mglyph><xmp></math><img src=invalid onerror=fail()></xmp>`);
+};
+
+promise_test(async test => {
+ await test_driver.set_permission({name: 'clipboard-read'}, 'granted');
+ await test_driver.click(button);
+
+ const items = await navigator.clipboard.read();
+ const htmlBlob = await items[0].getType("text/html");
+ const html = await htmlBlob.text();
+
+ // This inserts an image with `onerror` handler if `html` is not properly sanitized
+ output.innerHTML = html;
+
+ // Allow the 'error' event to be dispatched asynchronously
+ await new Promise(resolve => test.step_timeout(resolve, 100));
+
+ assert_false(testFailed);
+});
+</script>