crypto: fix malloc mixing in X509ToObject

EC_KEY_key2buf returns an OPENSSL_malloc'd pointer so it shouldn't be
passed into Buffer::New, which expect a libc malloc'd pointer. Instead,
factor out the ECDH::GetPublicKey code which uses EC_POINT_point2oct.

This preserves the existing behavior where encoding failures are
silently ignored, but it is probably safe to CHECK fail them instead.

PR-URL: nodejs/node#25717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

src/node_crypto.cc

@@ -1556,6 +1556,28 @@ static void AddFingerprintDigest(const unsigned char* md,
   }
 }
 
+
+static MaybeLocal<Object> ECPointToBuffer(Environment* env,
+                                          const EC_GROUP* group,
+                                          const EC_POINT* point,
+                                          point_conversion_form_t form,
+                                          const char** error) {
+  size_t len = EC_POINT_point2oct(group, point, form, nullptr, 0, nullptr);
+  if (len == 0) {
+    if (error != nullptr) *error = "Failed to get public key length";
+    return MaybeLocal<Object>();
+  }
+  MallocedBuffer<unsigned char> buf(len,
+                                    env->isolate()->GetArrayBufferAllocator());
+  len = EC_POINT_point2oct(group, point, form, buf.data, buf.size, nullptr);
+  if (len == 0) {
+    if (error != nullptr) *error = "Failed to get public key";
+    return MaybeLocal<Object>();
+  }
+  return Buffer::New(env, buf.release(), len);
+}
+
+
 static Local<Object> X509ToObject(Environment* env, X509* cert) {
   EscapableHandleScope scope(env->isolate());
   Local<Context> context = env->context();
@@ -4472,31 +4494,19 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
   ECDH* ecdh;
   ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder());
 
+  const EC_GROUP* group = EC_KEY_get0_group(ecdh->key_.get());
   const EC_POINT* pub = EC_KEY_get0_public_key(ecdh->key_.get());
   if (pub == nullptr)
     return env->ThrowError("Failed to get ECDH public key");
 
-  int size;
   CHECK(args[0]->IsUint32());
   uint32_t val = args[0].As<Uint32>()->Value();
   point_conversion_form_t form = static_cast<point_conversion_form_t>(val);
 
-  size = EC_POINT_point2oct(ecdh->group_, pub, form, nullptr, 0, nullptr);
-  if (size == 0)
-    return env->ThrowError("Failed to get public key length");
-
-  auto* allocator = env->isolate()->GetArrayBufferAllocator();
-  unsigned char* out =
-      static_cast<unsigned char*>(allocator->AllocateUninitialized(size));
-
-  int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr);
-  if (r != size) {
-    allocator->Free(out, size);
-    return env->ThrowError("Failed to get public key");
-  }
-
-  Local<Object> buf =
-      Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
+  const char* error;
+  Local<Object> buf;
+  if (!ECPointToBuffer(env, group, pub, form, &error).ToLocal(&buf))
+    return env->ThrowError(error);
   args.GetReturnValue().Set(buf);
 }
 
@@ -5103,24 +5113,10 @@ void ConvertKey(const FunctionCallbackInfo<Value>& args) {
   uint32_t val = args[2].As<Uint32>()->Value();
   point_conversion_form_t form = static_cast<point_conversion_form_t>(val);
 
-  int size = EC_POINT_point2oct(
-      group.get(), pub.get(), form, nullptr, 0, nullptr);
-
-  if (size == 0)
-    return env->ThrowError("Failed to get public key length");
-
-  auto* allocator = env->isolate()->GetArrayBufferAllocator();
-  unsigned char* out =
-      static_cast<unsigned char*>(allocator->AllocateUninitialized(size));
-
-  int r = EC_POINT_point2oct(group.get(), pub.get(), form, out, size, nullptr);
-  if (r != size) {
-    allocator->Free(out, size);
-    return env->ThrowError("Failed to get public key");
-  }
-
-  Local<Object> buf =
-      Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
+  const char* error;
+  Local<Object> buf;
+  if (!ECPointToBuffer(env, group.get(), pub.get(), form, &error).ToLocal(&buf))
+    return env->ThrowError(error);
   args.GetReturnValue().Set(buf);
 }