AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[wasm] Use size over capacity for types OOB checks

Browse Source 
This fixes a bug where the {types} vector automatically reserved
additional space, and by comparing with its capacity we failed to
register an out-of-bounds error.
Using capacity over size has led to bugs before, and using it correctly
(reserving as much space as needed manually) prevents vectors from
reserving space exponentially. Therefore we are switching to using size
for bounds checks instead.

Bug: v8:7748, chromium:1388942
Change-Id: I3cb8de4f113aaa6d70e45557161fd4c268861f1f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4046221
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84419}
This commit is contained in:
Manos Koukoutos 2022-11-22 09:57:24 +01:00 committed by V8 LUCI CQ
parent 858e87894a
commit dbe5434a36
6 changed files with 81 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/wasm/canonical-types.cc
View File

@ -15,11 +15,16 @@ TypeCanonicalizer* GetTypeCanonicalizer() {
}
void TypeCanonicalizer::AddRecursiveGroup(WasmModule* module, uint32_t size) {
AddRecursiveGroup(module, size,
static_cast<uint32_t>(module->types.size() - size));
}
void TypeCanonicalizer::AddRecursiveGroup(WasmModule* module, uint32_t size,
uint32_t start_index) {
// Multiple threads could try to register recursive groups concurrently.
// TODO(manoskouk): Investigate if we can fine-grain the synchronization.
base::MutexGuard mutex_guard(&mutex_);
DCHECK_GE(module->types.size(), size);
uint32_t start_index = static_cast<uint32_t>(module->types.size()) - size;
DCHECK_GE(module->types.size(), start_index + size);
CanonicalGroup group;
group.types.resize(size);
for (uint32_t i = 0; i < size; i++) {

10
src/wasm/canonical-types.h
View File

@ -38,9 +38,13 @@ class TypeCanonicalizer {
TypeCanonicalizer(TypeCanonicalizer&& other) = delete;
TypeCanonicalizer& operator=(TypeCanonicalizer&& other) = delete;
// Registers the last {size} types of {module} as a recursive group, and
// possibly canonicalizes it if an identical one has been found.
// Modifies {module->isorecursive_canonical_type_ids}.
// Registers {size} types of {module} as a recursive group, starting at
// {start_index}, and possibly canonicalizes it if an identical one has been
// found. Modifies {module->isorecursive_canonical_type_ids}.
V8_EXPORT_PRIVATE void AddRecursiveGroup(WasmModule* module, uint32_t size,
uint32_t start_index);
// Same as above, except it registers the last {size} types in the module.
V8_EXPORT_PRIVATE void AddRecursiveGroup(WasmModule* module, uint32_t size);
// Adds a module-independent signature as a recursive group, and canonicalizes

5
src/wasm/function-body-decoder-impl.h
View File

@ -215,8 +215,6 @@ void DecodeError(Decoder* decoder, const char* str) {
namespace value_type_reader {
// If {module} is not null, the read index will be checked against the module's
// type capacity.
template <typename ValidationTag>
HeapType read_heap_type(Decoder* decoder, const byte* pc,
uint32_t* const length, const WasmFeatures& enabled) {
@ -392,8 +390,7 @@ bool ValidateHeapType(Decoder* decoder, const byte* pc,
// A {nullptr} module is accepted if we are not validating anyway (e.g. for
// opcode length computation).
if (!ValidationTag::validate && module == nullptr) return true;
// We use capacity over size so this works mid-DecodeTypeSection.
if (!VALIDATE(type.ref_index() < module->types.capacity())) {
if (!VALIDATE(type.ref_index() < module->types.size())) {
DecodeError<ValidationTag>(decoder, pc, "Type index %u is out of bounds",
type.ref_index());
return false;

47
src/wasm/module-decoder-impl.h
View File

@ -605,15 +605,6 @@ class ModuleDecoderTemplate : public Decoder {
}
}
bool check_supertype(uint32_t supertype) {
if (V8_UNLIKELY(supertype >= module_->types.size())) {
errorf(pc(), "type %zu: forward-declared supertype %u",
module_->types.size(), supertype);
return false;
}
return true;
}
TypeDefinition consume_subtype_definition() {
DCHECK(enabled_features_.has_gc());
uint8_t kind = read_u8<Decoder::FullValidationTag>(pc(), "type kind");
@ -629,7 +620,6 @@ class ModuleDecoderTemplate : public Decoder {
tracer_.Description(supertype);
tracer_.NextLine();
}
if (supertype != kNoSuperType && !check_supertype(supertype)) return {};
TypeDefinition type = consume_base_type_definition();
type.supertype = supertype;
return type;
@ -644,7 +634,8 @@ class ModuleDecoderTemplate : public Decoder {
// Non wasm-gc type section decoding.
if (!enabled_features_.has_gc()) {
module_->types.reserve(types_count);
module_->types.resize(types_count);
module_->isorecursive_canonical_type_ids.resize(types_count);
for (uint32_t i = 0; i < types_count; ++i) {
TRACE("DecodeSignature[%d] module+%d\n", i,
static_cast<int>(pc_ - start_));
@ -660,8 +651,8 @@ class ModuleDecoderTemplate : public Decoder {
consume_bytes(1, "function");
const FunctionSig* sig = consume_sig(module_->signature_zone.get());
if (!ok()) break;
module_->add_signature(sig, kNoSuperType);
type_canon->AddRecursiveGroup(module_.get(), 1);
module_->types[i] = {sig, kNoSuperType};
type_canon->AddRecursiveGroup(module_.get(), 1, i);
break;
}
case kWasmArrayTypeCode:
@ -687,22 +678,26 @@ class ModuleDecoderTemplate : public Decoder {
if (kind == kWasmRecursiveTypeGroupCode) {
consume_bytes(1, "rec. group definition", tracer_);
tracer_.NextLine();
size_t initial_size = module_->types.size();
uint32_t group_size =
consume_count("recursive group size", kV8MaxWasmTypes);
if (module_->types.size() + group_size > kV8MaxWasmTypes) {
if (initial_size + group_size > kV8MaxWasmTypes) {
errorf(pc(), "Type definition count exceeds maximum %zu",
kV8MaxWasmTypes);
return;
}
// Reserve space for the current recursive group, so we are
// allowed to reference its elements.
module_->types.reserve(module_->types.size() + group_size);
module_->types.resize(initial_size + group_size);
module_->isorecursive_canonical_type_ids.resize(initial_size +
group_size);
for (uint32_t j = 0; j < group_size; j++) {
tracer_.TypeOffset(pc_offset());
TypeDefinition type = consume_subtype_definition();
if (ok()) module_->add_type(type);
if (ok()) module_->types[initial_size + j] = type;
}
if (ok()) {
type_canon->AddRecursiveGroup(module_.get(), group_size,
static_cast<uint32_t>(initial_size));
}
if (ok()) type_canon->AddRecursiveGroup(module_.get(), group_size);
} else {
tracer_.TypeOffset(pc_offset());
TypeDefinition type = consume_subtype_definition();
@ -718,16 +713,22 @@ class ModuleDecoderTemplate : public Decoder {
for (uint32_t i = 0; ok() && i < module_->types.size(); ++i) {
uint32_t explicit_super = module_->supertype(i);
if (explicit_super == kNoSuperType) continue;
// {consume_super_type} has checked this.
DCHECK_LT(explicit_super, module_->types.size());
if (explicit_super >= module_->types.size()) {
errorf("type %u: supertype %u out of bounds", i, explicit_super);
continue;
}
if (explicit_super >= i) {
errorf("type %u: forward-declared supertype %u", i, explicit_super);
continue;
}
int depth = GetSubtypingDepth(module, i);
DCHECK_GE(depth, 0);
if (depth > static_cast<int>(kV8MaxRttSubtypingDepth)) {
errorf("type %d: subtyping depth is greater than allowed", i);
errorf("type %u: subtyping depth is greater than allowed", i);
continue;
}
if (!ValidSubtypeDefinition(i, explicit_super, module, module)) {
errorf("type %d has invalid explicit supertype %d", i, explicit_super);
errorf("type %u has invalid explicit supertype %u", i, explicit_super);
continue;
}
}

17
test/mjsunit/regress/wasm/regress-1388942.js Normal file
View File

@ -0,0 +1,17 @@
// Copyright 2022 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --experimental-wasm-gc
d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
let builder = new WasmModuleBuilder();
builder.addType(kSig_v_v);
builder.addType(kSig_v_i);
builder.addType(kSig_i_v);
builder.addGlobal(wasmRefNullType(3), true, [kExprRefNull, 3]);
assertThrows(() => builder.instantiate(), WebAssembly.CompileError);

27
test/unittests/wasm/module-decoder-unittest.cc
View File

@ -3345,13 +3345,36 @@ TEST_F(WasmModuleVerifyTest, OutOfBoundsTypeInType) {
EXPECT_NOT_OK(result, "Type index 1 is out of bounds");
}
TEST_F(WasmModuleVerifyTest, ForwardSupertype) {
TEST_F(WasmModuleVerifyTest, OutOfBoundsSupertype) {
WASM_FEATURE_SCOPE(typed_funcref);
WASM_FEATURE_SCOPE(gc);
static const byte data[] = {
SECTION(Type, ENTRY_COUNT(1), kWasmRecursiveTypeGroupCode, ENTRY_COUNT(1),
kWasmSubtypeCode, ENTRY_COUNT(1), 1,
WASM_STRUCT_DEF(FIELD_COUNT(1), STRUCT_FIELD(kI32Code, true)))};
ModuleResult result = DecodeModule(base::ArrayVector(data));
EXPECT_NOT_OK(result, "type 0: supertype 1 out of bounds");
}
TEST_F(WasmModuleVerifyTest, ForwardSupertypeSameType) {
WASM_FEATURE_SCOPE(typed_funcref);
WASM_FEATURE_SCOPE(gc);
static const byte data[] = {
SECTION(Type, ENTRY_COUNT(1), kWasmRecursiveTypeGroupCode, ENTRY_COUNT(1),
kWasmSubtypeCode, ENTRY_COUNT(1), 0,
WASM_STRUCT_DEF(FIELD_COUNT(1), STRUCT_FIELD(kRefCode, true)))};
WASM_STRUCT_DEF(FIELD_COUNT(1), STRUCT_FIELD(kI32Code, true)))};
ModuleResult result = DecodeModule(base::ArrayVector(data));
EXPECT_NOT_OK(result, "type 0: forward-declared supertype 0");
}
TEST_F(WasmModuleVerifyTest, ForwardSupertypeSameRecGroup) {
WASM_FEATURE_SCOPE(typed_funcref);
WASM_FEATURE_SCOPE(gc);
static const byte data[] = {
SECTION(Type, ENTRY_COUNT(1), kWasmRecursiveTypeGroupCode, ENTRY_COUNT(2),
kWasmSubtypeCode, ENTRY_COUNT(1), 0,
WASM_STRUCT_DEF(FIELD_COUNT(1), STRUCT_FIELD(kI32Code, true)),
WASM_STRUCT_DEF(FIELD_COUNT(1), STRUCT_FIELD(kI32Code, true)))};
ModuleResult result = DecodeModule(base::ArrayVector(data));
EXPECT_NOT_OK(result, "type 0: forward-declared supertype 0");
}