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,24 @@ 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) {
+  size_t len = EC_POINT_point2oct(group, point, form, nullptr, 0, nullptr);
+  if (len == 0) {
+    env->ThrowError("Failed to get public key length");
+    return MaybeLocal<Object>();
+  }
+  MallocedBuffer<unsigned char> buf(len);
+  len = EC_POINT_point2oct(group, point, form, buf.data, buf.size, nullptr);
+  if (len == 0) {
+    env->ThrowError("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();
@@ -4460,26 +4478,14 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
   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");
-
-  unsigned char* out = node::Malloc<unsigned char>(size);
-
-  int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr);
-  if (r != size) {
-    free(out);
-    return env->ThrowError("Failed to get public key");
-  }
-
-  Local<Object> buf =
-      Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
-  args.GetReturnValue().Set(buf);
+  MaybeLocal<Object> buf =
+      ECPointToBuffer(env, EC_KEY_get0_group(ecdh->key_.get()), pub, form);
+  if (buf.IsEmpty()) return;
+  args.GetReturnValue().Set(buf.ToLocalChecked());
 }
 
 
@@ -5082,23 +5088,9 @@ 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");
-
-  unsigned char* out = node::Malloc<unsigned char>(size);
-
-  int r = EC_POINT_point2oct(group.get(), pub.get(), form, out, size, nullptr);
-  if (r != size) {
-    free(out);
-    return env->ThrowError("Failed to get public key");
-  }
-
-  Local<Object> buf =
-      Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
-  args.GetReturnValue().Set(buf);
+  MaybeLocal<Object> buf = ECPointToBuffer(env, group.get(), pub.get(), form);
+  if (buf.IsEmpty()) return;
+  args.GetReturnValue().Set(buf.ToLocalChecked());
 }