[wasm] Stricter max memory check

If the maximum number of memory pages is raised using
--wasm-max-mem-pages, we might allocate more than kMaxInt bytes for
wasm memory. The byte length is stored as int in JSArrayBuffer, hence
this can lead to failures.
Thus, we now additially check against kMaxInt, and fail instantiation
if this check fails.

Drive-by: Add/fix more bounds checks.

R=ahaas@chromium.org
BUG=chromium:724846

Change-Id: Id8e1a1e13e15f4aa355ab9414b4b950510e5e88a
Reviewed-on: https://chromium-review.googlesource.com/509255Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45465}

src/wasm/wasm-module.cc

@@ -858,6 +858,7 @@ Handle<JSArrayBuffer> wasm::SetupArrayBuffer(Isolate* isolate,
                                              bool is_external,
                                              bool enable_guard_regions) {
   Handle<JSArrayBuffer> buffer = isolate->factory()->NewJSArrayBuffer();
+  DCHECK_GE(kMaxInt, size);
   JSArrayBuffer::Setup(buffer, isolate, is_external, allocation_base,
                        allocation_length, backing_store,
                        static_cast<int>(size));
@@ -869,7 +870,11 @@ Handle<JSArrayBuffer> wasm::SetupArrayBuffer(Isolate* isolate,
 
 Handle<JSArrayBuffer> wasm::NewArrayBuffer(Isolate* isolate, size_t size,
                                            bool enable_guard_regions) {
-  if (size > (FLAG_wasm_max_mem_pages * WasmModule::kPageSize)) {
+  // Check against kMaxInt, since the byte length is stored as int in the
+  // JSArrayBuffer. Note that wasm_max_mem_pages can be raised from the command
+  // line, and we don't want to fail a CHECK then.
+  if (size > FLAG_wasm_max_mem_pages * WasmModule::kPageSize ||
+      size > kMaxInt) {
     // TODO(titzer): lift restriction on maximum memory allocated here.
     return Handle<JSArrayBuffer>::null();
   }
@@ -1302,8 +1307,8 @@ class InstantiationHelper {
     //--------------------------------------------------------------------------
     if (!memory_.is_null()) {
       Address mem_start = static_cast<Address>(memory_->backing_store());
-      uint32_t mem_size =
-          static_cast<uint32_t>(memory_->byte_length()->Number());
+      uint32_t mem_size;
+      CHECK(memory_->byte_length()->ToUint32(&mem_size));
       LoadDataSegments(mem_start, mem_size);
 
       uint32_t old_mem_size = compiled_module_->mem_size();

src/wasm/wasm-objects.cc

@@ -346,15 +346,14 @@ Handle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
   Address old_mem_start = nullptr;
   uint32_t old_size = 0;
   if (!old_buffer.is_null()) {
-    DCHECK(old_buffer->byte_length()->IsNumber());
     old_mem_start = static_cast<Address>(old_buffer->backing_store());
-    old_size = old_buffer->byte_length()->Number();
+    CHECK(old_buffer->byte_length()->ToUint32(&old_size));
   }
+  DCHECK_EQ(0, old_size % WasmModule::kPageSize);
+  uint32_t old_pages = old_size / WasmModule::kPageSize;
   DCHECK_GE(std::numeric_limits<uint32_t>::max(),
             old_size + pages * WasmModule::kPageSize);
-  uint32_t new_size = old_size + pages * WasmModule::kPageSize;
-  if (new_size <= old_size || max_pages * WasmModule::kPageSize < new_size ||
-      FLAG_wasm_max_mem_pages * WasmModule::kPageSize < new_size) {
+  if (old_pages > max_pages || pages > max_pages - old_pages) {
     return Handle<JSArrayBuffer>::null();
   }
 
@@ -363,6 +362,8 @@ Handle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
   const bool enable_guard_regions =
       (old_buffer.is_null() && EnableGuardRegions()) ||
       (!old_buffer.is_null() && old_buffer->has_guard_region());
+  size_t new_size =
+      static_cast<size_t>(old_pages + pages) * WasmModule::kPageSize;
   Handle<JSArrayBuffer> new_buffer =
       NewArrayBuffer(isolate, new_size, enable_guard_regions);
   if (new_buffer.is_null()) return new_buffer;

test/mjsunit/regress/wasm/regression-724846.js

+// Copyright 2017 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');
+
+// Flags: --wasm-max-mem-pages=49152
+
+let builder = new WasmModuleBuilder();
+const num_pages = 49152;
+builder.addMemory(num_pages, num_pages);
+// num_pages * 64k (page size) > kMaxInt.
+assertThrows(() => builder.instantiate(), RangeError);