From 5ddd6115c60536037fe4546c05643dc88cfb3545 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 5 Aug 2020 19:59:09 +0200 Subject: [PATCH] [api] Fix empty Maybe crash in GetRealNamedPropertyAttributes `Object::GetRealNamedPropertyAttributes()` can crash if an empty `Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because it was not checking for that. Fix that. Refs: https://github.com/nodejs/node/issues/34606 Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057 Commit-Queue: Leszek Swirski Reviewed-by: Leszek Swirski Cr-Commit-Position: refs/heads/master@{#69258} --- src/api/api.cc | 12 +++++++----- test/cctest/test-api.cc | 42 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/api/api.cc b/src/api/api.cc index 93780bceec4..0097683120a 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -4653,9 +4653,9 @@ Maybe v8::Object::GetRealNamedPropertyAttributesInPrototypeChain( Local context, Local key) { auto isolate = reinterpret_cast(context->GetIsolate()); - ENTER_V8_NO_SCRIPT(isolate, context, Object, - GetRealNamedPropertyAttributesInPrototypeChain, - Nothing(), i::HandleScope); + ENTER_V8(isolate, context, Object, + GetRealNamedPropertyAttributesInPrototypeChain, + Nothing(), i::HandleScope); i::Handle self = Utils::OpenHandle(this); if (!self->IsJSObject()) return Nothing(); i::Handle key_obj = Utils::OpenHandle(*key); @@ -4668,6 +4668,7 @@ v8::Object::GetRealNamedPropertyAttributesInPrototypeChain( i::LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR); Maybe result = i::JSReceiver::GetPropertyAttributes(&it); + has_pending_exception = result.IsNothing(); RETURN_ON_FAILED_EXECUTION_PRIMITIVE(PropertyAttribute); if (!it.IsFound()) return Nothing(); if (result.FromJust() == i::ABSENT) return Just(None); @@ -4692,14 +4693,15 @@ MaybeLocal v8::Object::GetRealNamedProperty(Local context, Maybe v8::Object::GetRealNamedPropertyAttributes( Local context, Local key) { auto isolate = reinterpret_cast(context->GetIsolate()); - ENTER_V8_NO_SCRIPT(isolate, context, Object, GetRealNamedPropertyAttributes, - Nothing(), i::HandleScope); + ENTER_V8(isolate, context, Object, GetRealNamedPropertyAttributes, + Nothing(), i::HandleScope); i::Handle self = Utils::OpenHandle(this); i::Handle key_obj = Utils::OpenHandle(*key); i::LookupIterator::Key lookup_key(isolate, key_obj); i::LookupIterator it(isolate, self, lookup_key, self, i::LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR); auto result = i::JSReceiver::GetPropertyAttributes(&it); + has_pending_exception = result.IsNothing(); RETURN_ON_FAILED_EXECUTION_PRIMITIVE(PropertyAttribute); if (!it.IsFound()) return Nothing(); if (result.FromJust() == i::ABSENT) { diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 151076296b4..18f7738033f 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -11959,6 +11959,48 @@ THREADED_TEST(VariousGetPropertiesAndThrowingCallbacks) { CHECK(result.IsEmpty()); } +THREADED_TEST(GetRealNamedPropertyAttributes_With_Proxy) { + LocalContext context; + HandleScope scope(context->GetIsolate()); + + { + Local proxy = + CompileRun( + "new Proxy({ p: 1 }, { getOwnPropertyDescriptor: _ => { " + " throw new Error('xyz'); } });") + .As(); + TryCatch try_catch(context->GetIsolate()); + v8::Maybe result = + proxy->GetRealNamedPropertyAttributes(context.local(), v8_str("p")); + CHECK(result.IsNothing()); + CHECK(try_catch.HasCaught()); + CHECK(try_catch.Exception() + .As() + ->Get(context.local(), v8_str("message")) + .ToLocalChecked() + ->StrictEquals(v8_str("xyz"))); + } + + { + Local proxy = + CompileRun( + "Object.create(" + " new Proxy({ p: 1 }, { getOwnPropertyDescriptor: _ => { " + " throw new Error('abc'); } }))") + .As(); + TryCatch try_catch(context->GetIsolate()); + v8::Maybe result = + proxy->GetRealNamedPropertyAttributesInPrototypeChain(context.local(), + v8_str("p")); + CHECK(result.IsNothing()); + CHECK(try_catch.HasCaught()); + CHECK(try_catch.Exception() + .As() + ->Get(context.local(), v8_str("message")) + .ToLocalChecked() + ->StrictEquals(v8_str("abc"))); + } +} static void ThrowingCallbackWithTryCatch( const v8::FunctionCallbackInfo& args) {