[wasm] Avoid integer overflow on function locals check

On 32-bit systems, the computation {count + type_list->size()} can
overflow, leading to memory corruption later on.

R=titzer@chromium.org

Bug: chromium:819869
Change-Id: Ic81d201e58211e3989b4e945cd52e98dc951fbda
Reviewed-on: https://chromium-review.googlesource.com/955025
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51817}
This commit is contained in:
Clemens Hammacher 2018-03-08 15:39:47 +01:00 committed by Commit Bot
parent d4c4345ef8
commit a71e5f9a7b
3 changed files with 23 additions and 3 deletions

View File

@ -41,6 +41,8 @@ class MovableLabel {
Label* get() { return label_.get(); } Label* get() { return label_.get(); }
MovableLabel() : MovableLabel(new Label()) {} MovableLabel() : MovableLabel(new Label()) {}
operator bool() const { return label_ != nullptr; }
static MovableLabel None() { return MovableLabel(nullptr); } static MovableLabel None() { return MovableLabel(nullptr); }
private: private:
@ -53,6 +55,8 @@ class MovableLabel {
public: public:
Label* get() { return &label_; } Label* get() { return &label_; }
operator bool() const { return true; }
static MovableLabel None() { return MovableLabel(); } static MovableLabel None() { return MovableLabel(); }
private: private:
@ -148,6 +152,8 @@ class LiftoffCompiler {
codegen_zone_(codegen_zone), codegen_zone_(codegen_zone),
safepoint_table_builder_(compilation_zone_) {} safepoint_table_builder_(compilation_zone_) {}
~LiftoffCompiler() { BindUnboundLabels(nullptr); }
bool ok() const { return ok_; } bool ok() const { return ok_; }
void unsupported(Decoder* decoder, const char* reason) { void unsupported(Decoder* decoder, const char* reason) {
@ -185,7 +191,8 @@ class LiftoffCompiler {
#ifdef DEBUG #ifdef DEBUG
// Bind all labels now, otherwise their destructor will fire a DCHECK error // Bind all labels now, otherwise their destructor will fire a DCHECK error
// if they where referenced before. // if they where referenced before.
for (uint32_t i = 0, e = decoder->control_depth(); i < e; ++i) { uint32_t control_depth = decoder ? decoder->control_depth() : 0;
for (uint32_t i = 0; i < control_depth; ++i) {
Control* c = decoder->control_at(i); Control* c = decoder->control_at(i);
Label* label = c->label.get(); Label* label = c->label.get();
if (!label->is_bound()) __ bind(label); if (!label->is_bound()) __ bind(label);
@ -258,7 +265,7 @@ class LiftoffCompiler {
OutOfLineCode::StackCheck(position, __ cache_state()->used_registers)); OutOfLineCode::StackCheck(position, __ cache_state()->used_registers));
OutOfLineCode& ool = out_of_line_code_.back(); OutOfLineCode& ool = out_of_line_code_.back();
__ StackCheck(ool.label.get()); __ StackCheck(ool.label.get());
__ bind(ool.continuation.get()); if (ool.continuation) __ bind(ool.continuation.get());
} }
void StartFunctionBody(Decoder* decoder, Control* block) { void StartFunctionBody(Decoder* decoder, Control* block) {

View File

@ -682,7 +682,8 @@ class WasmDecoder : public Decoder {
uint32_t count = decoder->consume_u32v("local count"); uint32_t count = decoder->consume_u32v("local count");
if (decoder->failed()) return false; if (decoder->failed()) return false;
if ((count + type_list->size()) > kV8MaxWasmFunctionLocals) { DCHECK_LE(type_list->size(), kV8MaxWasmFunctionLocals);
if (count > kV8MaxWasmFunctionLocals - type_list->size()) {
decoder->error(decoder->pc() - 1, "local count too large"); decoder->error(decoder->pc() - 1, "local count too large");
return false; return false;
} }

View File

@ -0,0 +1,12 @@
// Copyright 2018 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.
load('test/mjsunit/wasm/wasm-constants.js');
load('test/mjsunit/wasm/wasm-module-builder.js');
var builder = new WasmModuleBuilder();
builder.addFunction(undefined, kSig_i_i)
.addLocals({i32_count: 0xffffffff})
.addBody([]);
assertThrows(() => builder.instantiate(), WebAssembly.CompileError);