tls: fix assert in context._external accessor

* Restrict the receiver to instances of the FunctionTemplate.
* Use `args.This()` instead of `args.Holder()`.

Fixes: #3682
PR-URL: #5521
Reviewed-By: Fedor Indutny <[email protected]>

Author: bnoordhuis

src/node_crypto.cc

@@ -63,6 +63,7 @@ static const int X509_NAME_FLAGS = ASN1_STRFLGS_ESC_CTRL
 namespace node {
 namespace crypto {
 
+using v8::AccessorSignature;
 using v8::Array;
 using v8::Boolean;
 using v8::Context;
@@ -315,7 +316,8 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
       nullptr,
       env->as_external(),
       DEFAULT,
-      static_cast<PropertyAttribute>(ReadOnly | DontDelete));
+      static_cast<PropertyAttribute>(ReadOnly | DontDelete),
+      AccessorSignature::New(env->isolate(), t));
 
   target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "SecureContext"),
               t->GetFunction());
@@ -1146,9 +1148,7 @@ int SecureContext::TicketKeyCallback(SSL* ssl,
 
 void SecureContext::CtxGetter(Local<String> property,
                               const PropertyCallbackInfo<Value>& info) {
-  HandleScope scope(info.GetIsolate());
-
-  SSL_CTX* ctx = Unwrap<SecureContext>(info.Holder())->ctx_;
+  SSL_CTX* ctx = Unwrap<SecureContext>(info.This())->ctx_;
   Local<External> ext = External::New(info.GetIsolate(), ctx);
   info.GetReturnValue().Set(ext);
 }
@@ -1216,7 +1216,8 @@ void SSLWrap<Base>::AddMethods(Environment* env, Local<FunctionTemplate> t) {
       nullptr,
       env->as_external(),
       DEFAULT,
-      static_cast<PropertyAttribute>(ReadOnly | DontDelete));
+      static_cast<PropertyAttribute>(ReadOnly | DontDelete),
+      AccessorSignature::New(env->isolate(), t));
 }
 
 
@@ -2203,10 +2204,8 @@ void SSLWrap<Base>::CertCbDone(const FunctionCallbackInfo<Value>& args) {
 
 template <class Base>
 void SSLWrap<Base>::SSLGetter(Local<String> property,
-                        const PropertyCallbackInfo<Value>& info) {
-  HandleScope scope(info.GetIsolate());
-
-  SSL* ssl = Unwrap<Base>(info.Holder())->ssl_;
+                              const PropertyCallbackInfo<Value>& info) {
+  SSL* ssl = Unwrap<Base>(info.This())->ssl_;
   Local<External> ext = External::New(info.GetIsolate(), ssl);
   info.GetReturnValue().Set(ext);
 }
@@ -4144,12 +4143,14 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
   env->SetProtoMethod(t, "setPublicKey", SetPublicKey);
   env->SetProtoMethod(t, "setPrivateKey", SetPrivateKey);
 
-  t->InstanceTemplate()->SetAccessor(env->verify_error_string(),
-                                     DiffieHellman::VerifyErrorGetter,
-                                     nullptr,
-                                     env->as_external(),
-                                     DEFAULT,
-                                     attributes);
+  t->InstanceTemplate()->SetAccessor(
+      env->verify_error_string(),
+      DiffieHellman::VerifyErrorGetter,
+      nullptr,
+      env->as_external(),
+      DEFAULT,
+      attributes,
+      AccessorSignature::New(env->isolate(), t));
 
   target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "DiffieHellman"),
               t->GetFunction());
@@ -4164,12 +4165,14 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
   env->SetProtoMethod(t2, "getPublicKey", GetPublicKey);
   env->SetProtoMethod(t2, "getPrivateKey", GetPrivateKey);
 
-  t2->InstanceTemplate()->SetAccessor(env->verify_error_string(),
-                                      DiffieHellman::VerifyErrorGetter,
-                                      nullptr,
-                                      env->as_external(),
-                                      DEFAULT,
-                                      attributes);
+  t2->InstanceTemplate()->SetAccessor(
+      env->verify_error_string(),
+      DiffieHellman::VerifyErrorGetter,
+      nullptr,
+      env->as_external(),
+      DEFAULT,
+      attributes,
+      AccessorSignature::New(env->isolate(), t2));
 
   target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "DiffieHellmanGroup"),
               t2->GetFunction());

test/parallel/test-tls-external-accessor.js

@@ -0,0 +1,24 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+if (!common.hasCrypto) {
+  console.log('1..0 # Skipped: missing crypto');
+  return;
+}
+
+// Ensure accessing ._external doesn't hit an assert in the accessor method.
+const tls = require('tls');
+{
+  const pctx = tls.createSecureContext().context;
+  const cctx = Object.create(pctx);
+  assert.throws(() => cctx._external, /incompatible receiver/);
+  pctx._external;
+}
+{
+  const pctx = tls.createSecurePair().credentials.context;
+  const cctx = Object.create(pctx);
+  assert.throws(() => cctx._external, /incompatible receiver/);
+  pctx._external;
+}