summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBenoit Lize <lizeb@chromium.org>2023-03-21 11:14:20 +0000
committerMichael BrĂ¼ning <michael.bruning@qt.io>2023-05-02 08:58:17 +0000
commit77e9a6c5028a9712490d65214a9882143c329ec5 (patch)
tree56d393972456cd5dfe9d45ceb5ce23019e05a931
parentd6c41f42c6315694b2aa3f435fd5fc4dac578108 (diff)
downloadqtwebengine-chromium-77e9a6c5028a9712490d65214a9882143c329ec5.tar.gz
[Backport] CVE-2023-1812: Out of bounds memory access in DOM Bindings
Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/4357658: Take encoding into account for ParkableString hashing Hashing is used for string deduplication, must take encoding into account. See linked bug for details. (cherry picked from commit ab66c0409aece5bd57511792a3867920f31c589b) Bug: 1418224 Change-Id: I63c024d0a97e44b1f3323cd1ca4d9e953c2beed1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4328136 Commit-Queue: Benoit Lize <lizeb@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1117528} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4357658 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Owners-Override: Benoit Lize <lizeb@chromium.org> Auto-Submit: Benoit Lize <lizeb@chromium.org> Cr-Commit-Position: refs/branch-heads/5615@{#696} Cr-Branched-From: 9c6408ef696e83a9936b82bbead3d41c93c82ee4-refs/heads/main@{#1109224} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/474365 Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
-rw-r--r--chromium/third_party/blink/renderer/platform/bindings/parkable_string.cc20
-rw-r--r--chromium/third_party/blink/renderer/platform/bindings/parkable_string.h3
2 files changed, 18 insertions, 5 deletions
diff --git a/chromium/third_party/blink/renderer/platform/bindings/parkable_string.cc b/chromium/third_party/blink/renderer/platform/bindings/parkable_string.cc
index afd1b69952b..ee0a48bf2c9 100644
--- a/chromium/third_party/blink/renderer/platform/bindings/parkable_string.cc
+++ b/chromium/third_party/blink/renderer/platform/bindings/parkable_string.cc
@@ -4,6 +4,8 @@
#include "third_party/blink/renderer/platform/bindings/parkable_string.h"
+#include <array>
+
#include "base/allocator/partition_allocator/oom.h"
#include "base/allocator/partition_allocator/partition_alloc.h"
#include "base/bind.h"
@@ -230,18 +232,26 @@ ParkableStringImpl::ParkableMetadata::ParkableMetadata(
is_8bit_(string.Is8Bit()),
length_(string.length()) {}
-// static
+// static2
std::unique_ptr<ParkableStringImpl::SecureDigest>
ParkableStringImpl::HashString(StringImpl* string) {
DigestValue digest_result;
- bool ok = ComputeDigest(kHashAlgorithmSha256,
- static_cast<const char*>(string->Bytes()),
- string->CharactersSizeInBytes(), digest_result);
+
+ Digestor digestor(kHashAlgorithmSha256);
+ digestor.Update(base::make_span(static_cast<const uint8_t*>(string->Bytes()),
+ string->CharactersSizeInBytes()));
+ // Also include encoding in the digest, otherwise two strings with identical
+ // byte content but different encoding will be assumed equal, leading to
+ // crashes when one is replaced by the other one.
+ std::array<uint8_t, 1> is_8bit;
+ is_8bit[0] = string->Is8Bit();
+ digestor.Update(is_8bit);
+ digestor.Finish(digest_result);
// The only case where this can return false in BoringSSL is an allocation
// failure of the temporary data required for hashing. In this case, there
// is nothing better to do than crashing.
- if (!ok) {
+ if (digestor.has_failed()) {
// Don't know the exact size, the SHA256 spec hints at ~64 (block size)
// + 32 (digest) bytes.
base::TerminateBecauseOutOfMemory(64 + kDigestSize);
diff --git a/chromium/third_party/blink/renderer/platform/bindings/parkable_string.h b/chromium/third_party/blink/renderer/platform/bindings/parkable_string.h
index 78e2185cced..0d607577347 100644
--- a/chromium/third_party/blink/renderer/platform/bindings/parkable_string.h
+++ b/chromium/third_party/blink/renderer/platform/bindings/parkable_string.h
@@ -57,6 +57,9 @@ class PLATFORM_EXPORT ParkableStringImpl final
constexpr static size_t kDigestSize = 32; // SHA256.
using SecureDigest = Vector<uint8_t, kDigestSize>;
// Computes a secure hash of a |string|, to be passed to |MakeParkable()|.
+ //
+ // TODO(lizeb): This is the "right" way of hashing a string. Move this code
+ // into WTF, and make sure it's the only way that is used.
static std::unique_ptr<SecureDigest> HashString(StringImpl* string);
// Not all ParkableStringImpls are actually parkable.