diff --git a/src/builtins.cc b/src/builtins.cc index 71867e1962..f4d9aa21f8 100644 --- a/src/builtins.cc +++ b/src/builtins.cc @@ -1044,7 +1044,9 @@ MUST_USE_RESULT static MaybeHandle HandleApiCallHelper( DCHECK(!args[0]->IsNull()); if (args[0]->IsUndefined()) args[0] = function->global_proxy(); - Object* raw_holder = fun_data->GetCompatibleReceiver(isolate, args[0]); + Handle receiver(&args[0]); + Handle raw_holder = + fun_data->GetCompatibleReceiver(isolate, receiver, is_construct); if (raw_holder->IsNull()) { // This function cannot be called with the given receiver. Abort! @@ -1066,12 +1068,8 @@ MUST_USE_RESULT static MaybeHandle HandleApiCallHelper( LOG(isolate, ApiObjectAccess("call", JSObject::cast(*args.receiver()))); DCHECK(raw_holder->IsJSObject()); - FunctionCallbackArguments custom(isolate, - data_obj, - *function, - raw_holder, - &args[0] - 1, - args.length() - 1, + FunctionCallbackArguments custom(isolate, data_obj, *function, *raw_holder, + &args[0] - 1, args.length() - 1, is_construct); v8::Handle value = custom.Call(callback); diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 230939fe56..c4b446eb62 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -8646,6 +8646,14 @@ bool HOptimizedGraphBuilder::TryInlineApiCall(Handle function, CallOptimization optimization(function); if (!optimization.is_simple_api_call()) return false; Handle holder_map; + for (int i = 0; i < receiver_maps->length(); ++i) { + auto map = receiver_maps->at(i); + // Don't inline calls to receivers requiring accesschecks. + if (map->is_access_check_needed() && + map->instance_type() != JS_GLOBAL_PROXY_TYPE) { + return false; + } + } if (call_type == kCallApiFunction) { // Cannot embed a direct reference to the global proxy map // as it maybe dropped on deserialization. diff --git a/src/objects.cc b/src/objects.cc index f876cda1fa..1cfdf63fa2 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -241,21 +241,28 @@ bool FunctionTemplateInfo::IsTemplateFor(Map* map) { // TODO(dcarney): CallOptimization duplicates this logic, merge. -Object* FunctionTemplateInfo::GetCompatibleReceiver(Isolate* isolate, - Object* receiver) { +Handle FunctionTemplateInfo::GetCompatibleReceiver( + Isolate* isolate, Handle receiver, bool is_construct) { // API calls are only supported with JSObject receivers. - if (!receiver->IsJSObject()) return isolate->heap()->null_value(); + if (!receiver->IsJSObject()) return isolate->factory()->null_value(); + auto js_receiver = Handle::cast(receiver); + if (!is_construct && js_receiver->IsAccessCheckNeeded() && + !isolate->MayAccess(js_receiver)) { + return isolate->factory()->null_value(); + } Object* recv_type = this->signature(); // No signature, return holder. if (recv_type->IsUndefined()) return receiver; FunctionTemplateInfo* signature = FunctionTemplateInfo::cast(recv_type); // Check the receiver. - for (PrototypeIterator iter(isolate, receiver, + for (PrototypeIterator iter(isolate, *receiver, PrototypeIterator::START_AT_RECEIVER); !iter.IsAtEnd(PrototypeIterator::END_AT_NON_HIDDEN); iter.Advance()) { - if (signature->IsTemplateFor(iter.GetCurrent())) return iter.GetCurrent(); + if (signature->IsTemplateFor(iter.GetCurrent())) { + return Handle(iter.GetCurrent(), isolate); + } } - return isolate->heap()->null_value(); + return isolate->factory()->null_value(); } diff --git a/src/objects.h b/src/objects.h index cf9184ffe1..0fc1d0d808 100644 --- a/src/objects.h +++ b/src/objects.h @@ -10783,7 +10783,9 @@ class FunctionTemplateInfo: public TemplateInfo { // Returns the holder JSObject if the function can legally be called with this // receiver. Returns Heap::null_value() if the call is illegal. - Object* GetCompatibleReceiver(Isolate* isolate, Object* receiver); + Handle GetCompatibleReceiver(Isolate* isolate, + Handle receiver, + bool is_construct); private: // Bit position in the flag, from least significant bit position. diff --git a/test/cctest/test-accessors.cc b/test/cctest/test-accessors.cc index bbb74c0a71..54546993f0 100644 --- a/test/cctest/test-accessors.cc +++ b/test/cctest/test-accessors.cc @@ -605,3 +605,86 @@ THREADED_TEST(Regress433458) { "Object.defineProperty(obj, 'prop', { writable: false });" "Object.defineProperty(obj, 'prop', { writable: true });"); } + + +static bool security_check_value = false; + + +static bool SecurityTestCallback(Local global, Local name, + v8::AccessType type, Local data) { + return security_check_value; +} + + +TEST(PrototypeGetterAccessCheck) { + i::FLAG_allow_natives_syntax = true; + LocalContext env; + v8::Isolate* isolate = env->GetIsolate(); + v8::HandleScope scope(isolate); + auto fun_templ = v8::FunctionTemplate::New(isolate); + auto getter_templ = v8::FunctionTemplate::New(isolate, handle_property); + getter_templ->SetLength(0); + fun_templ->InstanceTemplate()->SetAccessorProperty(v8_str("foo"), + getter_templ); + auto obj_templ = v8::ObjectTemplate::New(isolate); + obj_templ->SetAccessCheckCallbacks(SecurityTestCallback, nullptr); + env->Global()->Set(v8_str("Fun"), fun_templ->GetFunction()); + env->Global()->Set(v8_str("obj"), obj_templ->NewInstance()); + env->Global()->Set(v8_str("obj2"), obj_templ->NewInstance()); + + security_check_value = true; + CompileRun("var proto = new Fun();"); + CompileRun("obj.__proto__ = proto;"); + ExpectInt32("proto.foo", 907); + + // Test direct. + security_check_value = true; + ExpectInt32("obj.foo", 907); + security_check_value = false; + { + v8::TryCatch try_catch(isolate); + CompileRun("obj.foo"); + CHECK(try_catch.HasCaught()); + } + + // Test through call. + security_check_value = true; + ExpectInt32("proto.__lookupGetter__('foo').call(obj)", 907); + security_check_value = false; + { + v8::TryCatch try_catch(isolate); + CompileRun("proto.__lookupGetter__('foo').call(obj)"); + CHECK(try_catch.HasCaught()); + } + + // Test ics. + CompileRun( + "function f() {" + " var x;" + " for (var i = 0; i < 4; i++) {" + " x = obj.foo;" + " }" + " return x;" + "}"); + + security_check_value = true; + ExpectInt32("f()", 907); + security_check_value = false; + { + v8::TryCatch try_catch(isolate); + CompileRun("f();"); + CHECK(try_catch.HasCaught()); + } + + // Test crankshaft. + CompileRun("%OptimizeFunctionOnNextCall(f);"); + + security_check_value = true; + ExpectInt32("f()", 907); + security_check_value = false; + { + v8::TryCatch try_catch(isolate); + CompileRun("f();"); + CHECK(try_catch.HasCaught()); + } +} diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 5edbda777c..3ad0e6c610 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -8279,8 +8279,11 @@ TEST(DetachedAccesses) { CHECK(result.IsEmpty()); result = CompileRun("get_x_w()"); CHECK(result.IsEmpty()); - result = CompileRun("this_x()"); - CHECK(v8_str("env2_x")->Equals(result)); + { + v8::TryCatch try_catch(env1->GetIsolate()); + CompileRun("this_x()"); + CHECK(try_catch.HasCaught()); + } // Reattach env2's proxy env2 = Context::New(env1->GetIsolate(),