Check CallSite arguments more rigorously

Before, it was possible to construct invalid CallSite objects, which
would trigger a runtime assert when any function is called on it.
This check ensures to throw a TypeError when invalid information is
passed to the CallSite constructor.

This reverts part of this CL: https://codereview.chromium.org/2006603002

R=ishell@chromium.org, titzer@chromium.org, yangguo@chromium.org
BUG=chromium:614295

Review-Url: https://codereview.chromium.org/2010493002
Cr-Commit-Position: refs/heads/master@{#36578}

src/js/messages.js

@@ -275,8 +275,8 @@ function GetStackTraceLine(recv, fun, pos, isGlobal) {
 function CallSite(receiver, fun, pos, strict_mode) {
   // For wasm frames, receiver is the wasm object and fun is the function index
   // instead of an actual function.
-  if (!IS_FUNCTION(fun) && !IS_NUMBER(fun)) {
-    throw MakeTypeError(kCallSiteExpectsFunction, typeof fun);
+  if (!IS_FUNCTION(fun) && !%IsWasmObject(receiver)) {
+    throw MakeTypeError(kCallSiteExpectsFunction, typeof receiver, typeof fun);
   }
 
   if (IS_UNDEFINED(new.target)) {

src/messages.cc

@@ -182,14 +182,9 @@ CallSite::CallSite(Isolate* isolate, Handle<JSObject> call_site_obj)
       // invalid: neither javascript nor wasm
       return;
     }
-    Handle<Object> maybe_wasm_obj = JSObject::GetDataProperty(
-        call_site_obj, isolate->factory()->call_site_wasm_obj_symbol());
-    if (!maybe_wasm_obj->IsJSObject()) {
-      // invalid: neither javascript nor wasm
-      return;
-    }
     // wasm
-    wasm_obj_ = Handle<JSObject>::cast(maybe_wasm_obj);
+    wasm_obj_ = Handle<JSObject>::cast(JSObject::GetDataProperty(
+        call_site_obj, isolate->factory()->call_site_wasm_obj_symbol()));
     wasm_func_index_ = Smi::cast(*maybe_wasm_func_index)->value();
     DCHECK(static_cast<int>(wasm_func_index_) >= 0);
   }
@@ -210,8 +205,8 @@ Handle<Object> CallSite::GetFileName() {
 
 Handle<Object> CallSite::GetFunctionName() {
   if (IsWasm()) {
-    MaybeHandle<String> name = wasm::GetWasmFunctionName(
-        Handle<JSObject>::cast(wasm_obj_), wasm_func_index_);
+    MaybeHandle<String> name =
+        wasm::GetWasmFunctionName(wasm_obj_, wasm_func_index_);
     if (name.is_null()) return isolate_->factory()->null_value();
     return name.ToHandleChecked();
   }

src/messages.h

@@ -97,7 +97,8 @@ class CallSite {
   T(CalledOnNonObject, "% called on non-object")                               \
   T(CalledOnNullOrUndefined, "% called on null or undefined")                  \
   T(CallSiteExpectsFunction,                                                   \
-    "CallSite expects function or number as second argument, got %")           \
+    "CallSite expects wasm object as first or function as second argument, "   \
+    "got <%, %>")                                                              \
   T(CallSiteMethod, "CallSite method % expects CallSite as receiver")          \
   T(CannotConvertToPrimitive, "Cannot convert object to primitive value")      \
   T(CannotPreventExt, "Cannot prevent extensions")                             \

src/runtime/runtime-internal.cc

@@ -13,6 +13,7 @@
 #include "src/isolate-inl.h"
 #include "src/messages.h"
 #include "src/parsing/parser.h"
+#include "src/wasm/wasm-module.h"
 
 namespace v8 {
 namespace internal {
@@ -595,5 +596,14 @@ RUNTIME_FUNCTION(Runtime_OrdinaryHasInstance) {
       isolate, Object::OrdinaryHasInstance(isolate, callable, object));
 }
 
+RUNTIME_FUNCTION(Runtime_IsWasmObject) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(1, args.length());
+  CONVERT_ARG_HANDLE_CHECKED(Object, object, 0);
+  bool is_wasm_object = object->IsJSObject() &&
+                        wasm::IsWasmObject(Handle<JSObject>::cast(object));
+  return *isolate->factory()->ToBoolean(is_wasm_object);
+}
+
 }  // namespace internal
 }  // namespace v8

src/runtime/runtime.h

@@ -324,7 +324,8 @@ namespace internal {
   F(GetAndResetRuntimeCallStats, -1 /* <= 2 */, 1)  \
   F(EnqueueMicrotask, 1, 1)                         \
   F(RunMicrotasks, 0, 1)                            \
-  F(OrdinaryHasInstance, 2, 1)
+  F(OrdinaryHasInstance, 2, 1)                      \
+  F(IsWasmObject, 1, 1)
 
 #define FOR_EACH_INTRINSIC_JSON(F) \
   F(ParseJson, 1, 1)

src/wasm/wasm-module.cc

@@ -961,12 +961,22 @@ int32_t CompileAndRunWasmModule(Isolate* isolate, const WasmModule* module) {
 
 MaybeHandle<String> GetWasmFunctionName(Handle<JSObject> wasm,
                                         uint32_t func_index) {
+  DCHECK(IsWasmObject(wasm));
   Object* func_names_arr_obj = wasm->GetInternalField(kWasmFunctionNamesArray);
   if (func_names_arr_obj->IsUndefined()) return Handle<String>::null();
   return GetWasmFunctionNameFromTable(
       handle(ByteArray::cast(func_names_arr_obj)), func_index);
 }
 
+bool IsWasmObject(Handle<JSObject> object) {
+  // TODO(clemensh): Check wasm byte header once we store a copy of the bytes.
+  return object->GetInternalFieldCount() == kWasmModuleInternalFieldCount &&
+         object->GetInternalField(kWasmModuleCodeTable)->IsFixedArray() &&
+         object->GetInternalField(kWasmMemArrayBuffer)->IsJSArrayBuffer() &&
+         (object->GetInternalField(kWasmFunctionNamesArray)->IsByteArray() ||
+          object->GetInternalField(kWasmFunctionNamesArray)->IsUndefined());
+}
+
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8

src/wasm/wasm-module.h

@@ -320,6 +320,13 @@ int32_t CompileAndRunWasmModule(Isolate* isolate, const WasmModule* module);
 MaybeHandle<String> GetWasmFunctionName(Handle<JSObject> wasm,
                                         uint32_t func_index);
 
+// Check whether the given object is a wasm object.
+// This checks the number and type of internal fields, so it's not 100 percent
+// secure. If it turns out that we need more complete checks, we could add a
+// special marker as internal field, which will definitely never occur anywhere
+// else.
+bool IsWasmObject(Handle<JSObject> object);
+
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8

test/mjsunit/regress/regress-crbug-615774.js

+// Copyright 2016 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.
+
+Error.prepareStackTrace = (e,s) => s;
+var CallSiteConstructor = Error().stack[0].constructor;
+
+try {
+  (new CallSiteConstructor(CallSiteConstructor, 6)).toString();
+} catch (e) {
+}