diff options
author | Xiaocheng Hu <xiaochengh@chromium.org> | 2022-05-18 03:21:02 +0000 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2022-06-16 19:34:12 +0000 |
commit | 6279f48432e70f32f58f350ce3b87f454fe4bad0 (patch) | |
tree | d724d4cc11360a9aeca9c7423e79da55a6258be9 | |
parent | b3062c911051cda52487cb6e34dbcf3c6260b8c6 (diff) | |
download | qtwebengine-chromium-6279f48432e70f32f58f350ce3b87f454fe4bad0.tar.gz |
[Backport] CVE-2022-1867: Insufficient validation of untrusted input in Data Transfer
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/3623277:
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
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#997032}
Commit-Queue: Zakhar Voit <voit@google.com>
Reviewed-by: Jana Grill <janagrill@google.com>
Owners-Override: Jana Grill <janagrill@google.com>
Cr-Commit-Position: refs/branch-heads/4664@{#1632}
Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r-- | chromium/third_party/blink/renderer/core/editing/serializers/serialization.cc | 82 |
1 files changed, 51 insertions, 31 deletions
diff --git a/chromium/third_party/blink/renderer/core/editing/serializers/serialization.cc b/chromium/third_party/blink/renderer/core/editing/serializers/serialization.cc index e7a2d10108e..1ff2fe76550 100644 --- a/chromium/third_party/blink/renderer/core/editing/serializers/serialization.cc +++ b/chromium/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); |