[wasm] Fix insufficient bounds check in WebAssembly.get

According to the wasm js-spec, the table index can be uint32. The
implementation in our implementation expected an int though. We did not
check for the int overflow.

I replaced the throwing of the exception in WasmTableObject::Get to use
the ErrorThrower instead of throwing the exception with Isolate::Throw
directly. The reason is that I see with other CL's that I have to throw
several errors, and I don't want to introduce a new message and
MessageId for every error. Moreover, the ErrorThrower is a standard way
in wasm to throw errors. It feels right to throw the error the same way
here.

R=mstarzinger@chromium.org

Bug: chromium:940296
Change-Id: Idb77c813506fe66a3192b66fe0e8e807b80580ab
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1514496
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60181}

src/wasm/wasm-js.cc

@@ -1378,20 +1378,16 @@ void WebAssemblyTableGet(const v8::FunctionCallbackInfo<v8::Value>& args) {
   if (!EnforceUint32("Argument 0", args[0], context, &thrower, &index)) {
     return;
   }
-
-  i::MaybeHandle<i::Object> maybe_result =
-      i::WasmTableObject::Get(i_isolate, receiver, index);
-
-  if (maybe_result.is_null()) {
-    // No result was produced, which means that an exception should have been
-    // thrown. We can just return in that case, the ScheduledErrorThrower will
-    // take care of transforming the exception into a scheduled exception.
-    CHECK(i_isolate->has_pending_exception());
+  if (!i::WasmTableObject::IsInBounds(i_isolate, receiver, index)) {
+    thrower.RangeError("invalid index %u into function table", index);
     return;
   }
 
+  i::Handle<i::Object> result =
+      i::WasmTableObject::Get(i_isolate, receiver, index);
+
   v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
-  return_value.Set(Utils::ToLocal(maybe_result.ToHandleChecked()));
+  return_value.Set(Utils::ToLocal(result));
 }
 
 // WebAssembly.Table.set(num, JSFunction)

src/wasm/wasm-objects.cc

@@ -849,6 +849,14 @@ void WasmTableObject::Grow(Isolate* isolate, uint32_t count) {
   }
 }
 
+bool WasmTableObject::IsInBounds(Isolate* isolate,
+                                 Handle<WasmTableObject> table,
+                                 uint32_t entry_index) {
+  return (entry_index <
+              static_cast<uint32_t>(std::numeric_limits<int>::max()) &&
+          static_cast<int>(entry_index) < table->elements()->length());
+}
+
 void WasmTableObject::Set(Isolate* isolate, Handle<WasmTableObject> table,
                           uint32_t table_index, Handle<JSFunction> function) {
   Handle<FixedArray> array(table->elements(), isolate);
@@ -871,16 +879,16 @@ void WasmTableObject::Set(Isolate* isolate, Handle<WasmTableObject> table,
   array->set(table_index, *function);
 }
 
-MaybeHandle<Object> WasmTableObject::Get(Isolate* isolate,
-                                         Handle<WasmTableObject> table,
-                                         int table_index) {
+Handle<Object> WasmTableObject::Get(Isolate* isolate,
+                                    Handle<WasmTableObject> table,
+                                    uint32_t index) {
   Handle<FixedArray> elements(table->elements(), isolate);
-  if (table_index >= elements->length()) {
-    isolate->Throw(*isolate->factory()->NewRangeError(
-        MessageTemplate::kWasmTrapFuncInvalid));
-    return MaybeHandle<Object>();
-  }
-  Handle<Object> element(elements->get(table_index), isolate);
+  DCHECK(IsInBounds(isolate, table, index));
+
+  // The FixedArray is addressed with int's.
+  int entry_index = static_cast<int>(index);
+
+  Handle<Object> element(elements->get(entry_index), isolate);
   if (WasmExportedFunction::IsWasmExportedFunction(*element)) {
     return element;
   }
@@ -901,7 +909,7 @@ MaybeHandle<Object> WasmTableObject::Get(Isolate* isolate,
       WasmInstanceObject::GetWasmExportedFunction(isolate, instance,
                                                   function_index);
   if (maybe_element.ToHandle(&element)) {
-    elements->set(table_index, *element);
+    elements->set(entry_index, *element);
     return element;
   }
 
@@ -920,7 +928,7 @@ MaybeHandle<Object> WasmTableObject::Get(Isolate* isolate,
       isolate, instance, function_name, function_index,
       static_cast<int>(function.sig->parameter_count()), wrapper_code);
 
-  elements->set(table_index, *result);
+  elements->set(entry_index, *result);
   WasmInstanceObject::SetWasmExportedFunction(isolate, instance, function_index,
                                               result);
   return result;

src/wasm/wasm-objects.h

@@ -279,12 +279,14 @@ class WasmTableObject : public JSObject {
                                Handle<WasmInstanceObject> instance,
                                int table_index);
 
+  static bool IsInBounds(Isolate* isolate, Handle<WasmTableObject> table,
+                         uint32_t entry_index);
+
   static void Set(Isolate* isolate, Handle<WasmTableObject> table,
                   uint32_t index, Handle<JSFunction> function);
 
-  static MaybeHandle<Object> Get(Isolate* isolate,
-                                 Handle<WasmTableObject> table,
-                                 int table_index);
+  static Handle<Object> Get(Isolate* isolate, Handle<WasmTableObject> table,
+                            uint32_t index);
 
   static void UpdateDispatchTables(Isolate* isolate,
                                    Handle<WasmTableObject> table,

test/mjsunit/regress/wasm/regress-940296.js

+// Copyright 2019 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.
+
+let table = new WebAssembly.Table({element: "anyfunc", initial: 1});
+assertThrows(() => table.get(3612882876), RangeError);

test/mjsunit/wasm/js-api.js

@@ -641,9 +641,9 @@ assertEq(get.call(tbl1, 0), null);
 assertEq(get.call(tbl1, 0, Infinity), null);
 assertEq(get.call(tbl1, 1), null);
 assertEq(get.call(tbl1, 1.5), null);
-assertThrows(() => get.call(tbl1, 2), RangeError, /invalid index into function table/);
+assertThrows(() => get.call(tbl1, 2), RangeError, /invalid index \d* into function table/);
 assertThrows(
-    () => get.call(tbl1, 2.5), RangeError, /invalid index into function table/);
+    () => get.call(tbl1, 2.5), RangeError, /invalid index \d* into function table/);
 assertThrows(() => get.call(tbl1, -1), TypeError, /must be non-negative/);
 assertThrows(
     () => get.call(tbl1, Math.pow(2, 33)), TypeError,