Commit 641705e0 authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

Reland [wasm] Check the size of a function body before storing it

In the original CL I moved an error check backwards, unfortunately
behind a vector lookup which should not happen when there is an error.
Now I also move the vector lookup backwards.

Original message:
We stored the size of a function body before we check that
these values are valid. This caused a failing DCHECK in the constructor
of WireBytesRef which checked for integer overflows. With this CL we
check the size of the function body before we create the WireBytesRef.

R=clemensh@chromium.org

Bug: chromium:738097
Change-Id: Ie65b3cfcbcd6bdb3f04b0760673d9c7b7a0d1057
Reviewed-on: https://chromium-review.googlesource.com/561519Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46453}
parent e2f297a3
...@@ -665,13 +665,15 @@ class ModuleDecoder : public Decoder { ...@@ -665,13 +665,15 @@ class ModuleDecoder : public Decoder {
errorf(pos, "function body count %u mismatch (%u expected)", errorf(pos, "function body count %u mismatch (%u expected)",
functions_count, module_->num_declared_functions); functions_count, module_->num_declared_functions);
} }
for (uint32_t i = 0; ok() && i < functions_count; ++i) { for (uint32_t i = 0; i < functions_count; ++i) {
WasmFunction* function =
&module_->functions[i + module_->num_imported_functions];
uint32_t size = consume_u32v("body size"); uint32_t size = consume_u32v("body size");
function->code = {pc_offset(), size}; uint32_t offset = pc_offset();
consume_bytes(size, "function body"); consume_bytes(size, "function body");
if (ok() && verify_functions) { if (failed()) break;
WasmFunction* function =
&module_->functions[i + module_->num_imported_functions];
function->code = {offset, size};
if (verify_functions) {
ModuleBytesEnv module_env(module_.get(), nullptr, ModuleBytesEnv module_env(module_.get(), nullptr,
ModuleWireBytes(start_, end_)); ModuleWireBytes(start_, end_));
VerifyFunctionBody(module_->signature_zone->allocator(), VerifyFunctionBody(module_->signature_zone->allocator(),
......
...@@ -1347,6 +1347,19 @@ TEST_F(WasmModuleVerifyTest, Regression_648070) { ...@@ -1347,6 +1347,19 @@ TEST_F(WasmModuleVerifyTest, Regression_648070) {
EXPECT_FAILURE(data); EXPECT_FAILURE(data);
} }
TEST_F(WasmModuleVerifyTest, Regression_738097) {
// The function body size caused an integer overflow in the module decoder.
static const byte data[] = {
SIGNATURES_SECTION(1, SIG_ENTRY_v_v), // --
FUNCTION_SIGNATURES_SECTION(1, 0), // --
SECTION(Code, 1 + 5 + 1), // --
1, // --
U32V_5(0xffffffff), // function size,
0 // No real body
};
EXPECT_FAILURE(data);
}
TEST_F(WasmModuleVerifyTest, FunctionBodies_empty) { TEST_F(WasmModuleVerifyTest, FunctionBodies_empty) {
static const byte data[] = { static const byte data[] = {
EMPTY_SIGNATURES_SECTION, // -- EMPTY_SIGNATURES_SECTION, // --
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment