Skip to content

Commit e161a13

Browse files
jakobkummerowmibrunin
authored andcommitted
[Backport] CVE-2024-9602: Type Confusion in V8
Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/5886728: Properly check max module size and allow d8-based tests for it. (cherry picked from commit 9542895cdd3dbd97da3d9032ddb36fd4feb612e4) Fixed: 368241697 Change-Id: Iddc9f7e669de7a1d79dccbc99bcc5fb43dad67a1 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5886728 Reviewed-by: Clemens Backes <[email protected]> Reviewed-by: Matthias Liedtke <[email protected]> Auto-Submit: Jakob Kummerow <[email protected]> Commit-Queue: Jakob Kummerow <[email protected]> Cr-Original-Commit-Position: refs/heads/main@{#96272} Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5905241 Commit-Queue: Gyuyoung Kim (xWF) <[email protected]> Reviewed-by: Jakob Kummerow <[email protected]> Cr-Commit-Position: refs/branch-heads/12.6@{#68} Cr-Branched-From: 3c9fa12db3183a6f4ea53d2675adb66ea1194529-refs/heads/12.6.228@{#2} Cr-Branched-From: 981bb15ba4dbf9e2381dfc94ec2c4af0b9c6a0b6-refs/heads/main@{#93835} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/597919 Reviewed-by: Michal Klocek <[email protected]>
1 parent fb65b88 commit e161a13

File tree

3 files changed

+34
-11
lines changed

3 files changed

+34
-11
lines changed

chromium/v8/src/wasm/streaming-decoder.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,10 @@ void AsyncStreamingDecoder::Finish(bool can_use_compiled_module) {
294294
if (!full_wire_bytes_.back().empty()) {
295295
size_t total_length = 0;
296296
for (auto& bytes : full_wire_bytes_) total_length += bytes.size();
297+
if (ok()) {
298+
// {DecodeSectionLength} enforces this with graceful error reporting.
299+
CHECK_LE(total_length, max_module_size());
300+
}
297301
auto all_bytes = base::OwnedVector<uint8_t>::NewForOverwrite(total_length);
298302
uint8_t* ptr = all_bytes.begin();
299303
for (auto& bytes : full_wire_bytes_) {
@@ -627,6 +631,18 @@ std::unique_ptr<AsyncStreamingDecoder::DecodingState>
627631
AsyncStreamingDecoder::DecodeSectionLength::NextWithValue(
628632
AsyncStreamingDecoder* streaming) {
629633
TRACE_STREAMING("DecodeSectionLength(%zu)\n", value_);
634+
// Check if this section fits into the overall module length limit.
635+
// Note: {this->module_offset_} is the position of the section ID byte,
636+
// {streaming->module_offset_} is the start of the section's payload (i.e.
637+
// right after the just-decoded section length varint).
638+
// The latter can already exceed the max module size, when the previous
639+
// section barely fit into it, and this new section's ID or length crossed
640+
// the threshold.
641+
uint32_t payload_start = streaming->module_offset();
642+
size_t max_size = max_module_size();
643+
if (payload_start > max_size || max_size - payload_start < value_) {
644+
return streaming->ToErrorState();
645+
}
630646
SectionBuffer* buf =
631647
streaming->CreateNewBuffer(module_offset_, section_id_, value_,
632648
buffer().SubVector(0, bytes_consumed_));

chromium/v8/src/wasm/wasm-engine.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2017,10 +2017,11 @@ uint32_t max_table_init_entries() {
20172017

20182018
// {max_module_size} is declared in wasm-limits.h.
20192019
size_t max_module_size() {
2020-
// Clamp the value of --wasm-max-module-size between 16 and just below 2GB.
2020+
// Clamp the value of --wasm-max-module-size between 16 and the maximum
2021+
// that the implementation supports.
20212022
constexpr size_t kMin = 16;
2022-
constexpr size_t kMax = RoundDown<kSystemPointerSize>(size_t{kMaxInt});
2023-
static_assert(kMin <= kV8MaxWasmModuleSize && kV8MaxWasmModuleSize <= kMax);
2023+
constexpr size_t kMax = kV8MaxWasmModuleSize;
2024+
static_assert(kMin <= kV8MaxWasmModuleSize);
20242025
return std::clamp(v8_flags.wasm_max_module_size.value(), kMin, kMax);
20252026
}
20262027

chromium/v8/src/wasm/wasm-js.cc

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,8 @@ GET_FIRST_ARGUMENT_AS(Tag)
194194
#undef GET_FIRST_ARGUMENT_AS
195195

196196
i::wasm::ModuleWireBytes GetFirstArgumentAsBytes(
197-
const v8::FunctionCallbackInfo<v8::Value>& info, ErrorThrower* thrower,
198-
bool* is_shared) {
197+
const v8::FunctionCallbackInfo<v8::Value>& info, size_t max_length,
198+
ErrorThrower* thrower, bool* is_shared) {
199199
DCHECK(i::ValidateCallbackInfo(info));
200200
const uint8_t* start = nullptr;
201201
size_t length = 0;
@@ -226,7 +226,6 @@ i::wasm::ModuleWireBytes GetFirstArgumentAsBytes(
226226
if (length == 0) {
227227
thrower->CompileError("BufferSource argument is empty");
228228
}
229-
size_t max_length = i::wasm::max_module_size();
230229
if (length > max_length) {
231230
// The spec requires a CompileError for implementation-defined limits, see
232231
// https://webassembly.github.io/spec/js-api/index.html#limits.
@@ -609,7 +608,8 @@ void WebAssemblyCompileImpl(const v8::FunctionCallbackInfo<v8::Value>& info) {
609608
new AsyncCompilationResolver(isolate, context, promise_resolver));
610609

611610
bool is_shared = false;
612-
auto bytes = GetFirstArgumentAsBytes(info, &thrower, &is_shared);
611+
auto bytes = GetFirstArgumentAsBytes(info, i::wasm::max_module_size(),
612+
&thrower, &is_shared);
613613
if (thrower.error()) {
614614
resolver->OnCompilationFailed(thrower.Reify());
615615
return;
@@ -641,8 +641,11 @@ void WasmStreamingCallbackForTesting(
641641
v8::WasmStreaming::Unpack(info.GetIsolate(), info.Data());
642642

643643
bool is_shared = false;
644+
// We don't check the buffer length up front, to allow d8 to test that the
645+
// streaming decoder implementation handles overly large inputs correctly.
646+
size_t unlimited = std::numeric_limits<size_t>::max();
644647
i::wasm::ModuleWireBytes bytes =
645-
GetFirstArgumentAsBytes(info, &thrower, &is_shared);
648+
GetFirstArgumentAsBytes(info, unlimited, &thrower, &is_shared);
646649
if (thrower.error()) {
647650
streaming->Abort(Utils::ToLocal(thrower.Reify()));
648651
return;
@@ -744,7 +747,8 @@ void WebAssemblyValidateImpl(const v8::FunctionCallbackInfo<v8::Value>& info) {
744747
ErrorThrower thrower(i_isolate, "WebAssembly.validate()");
745748

746749
bool is_shared = false;
747-
auto bytes = GetFirstArgumentAsBytes(info, &thrower, &is_shared);
750+
auto bytes = GetFirstArgumentAsBytes(info, i::wasm::max_module_size(),
751+
&thrower, &is_shared);
748752

749753
v8::ReturnValue<v8::Value> return_value = info.GetReturnValue();
750754

@@ -823,7 +827,8 @@ void WebAssemblyModuleImpl(const v8::FunctionCallbackInfo<v8::Value>& info) {
823827
}
824828

825829
bool is_shared = false;
826-
auto bytes = GetFirstArgumentAsBytes(info, &thrower, &is_shared);
830+
auto bytes = GetFirstArgumentAsBytes(info, i::wasm::max_module_size(),
831+
&thrower, &is_shared);
827832

828833
if (thrower.error()) {
829834
return;
@@ -1140,7 +1145,8 @@ void WebAssemblyInstantiateImpl(
11401145
}
11411146

11421147
bool is_shared = false;
1143-
auto bytes = GetFirstArgumentAsBytes(info, &thrower, &is_shared);
1148+
auto bytes = GetFirstArgumentAsBytes(info, i::wasm::max_module_size(),
1149+
&thrower, &is_shared);
11441150
if (thrower.error()) {
11451151
resolver->OnInstantiationFailed(thrower.Reify());
11461152
return;

0 commit comments

Comments
 (0)