From 3dadc95cd2cff65a74537e20574e73accd371f7b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Sat, 20 Aug 2022 14:15:42 +0200
Subject: [PATCH] src: refactor to avoid using a moved object

KeyPairGenTraits::EncodeKey should not use an object that it previously
moved. Make ManagedEVPPKey::ToEncoded(Public|Private)Key non-static
members of ManagedEVPPKey and call them on the key object instead.

PR-URL: https://github.com/nodejs/node/pull/44269
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Feng Yu <F3n67u@outlook.com>
---
 src/crypto/crypto_keygen.h | 16 ++++++----------
 src/crypto/crypto_keys.cc  | 18 ++++++++----------
 src/crypto/crypto_keys.h   | 16 ++++++----------
 3 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/src/crypto/crypto_keygen.h b/src/crypto/crypto_keygen.h
index 01407de83c7b14..b0743efb38ddce 100644
--- a/src/crypto/crypto_keygen.h
+++ b/src/crypto/crypto_keygen.h
@@ -186,16 +186,12 @@ struct KeyPairGenTraits final {
       AdditionalParameters* params,
       v8::Local<v8::Value>* result) {
     v8::Local<v8::Value> keys[2];
-    if (ManagedEVPPKey::ToEncodedPublicKey(
-            env,
-            std::move(params->key),
-            params->public_key_encoding,
-            &keys[0]).IsNothing() ||
-        ManagedEVPPKey::ToEncodedPrivateKey(
-            env,
-            std::move(params->key),
-            params->private_key_encoding,
-            &keys[1]).IsNothing()) {
+    if (params->key
+            .ToEncodedPublicKey(env, params->public_key_encoding, &keys[0])
+            .IsNothing() ||
+        params->key
+            .ToEncodedPrivateKey(env, params->private_key_encoding, &keys[1])
+            .IsNothing()) {
       return v8::Nothing<bool>();
     }
     *result = v8::Array::New(env->isolate(), keys, arraysize(keys));
diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc
index b874152ad434f4..54d320ecd030f1 100644
--- a/src/crypto/crypto_keys.cc
+++ b/src/crypto/crypto_keys.cc
@@ -633,44 +633,42 @@ Maybe<bool> ExportJWKInner(Environment* env,
 
 Maybe<bool> ManagedEVPPKey::ToEncodedPublicKey(
     Environment* env,
-    ManagedEVPPKey key,
     const PublicKeyEncodingConfig& config,
     Local<Value>* out) {
-  if (!key) return Nothing<bool>();
+  if (!*this) return Nothing<bool>();
   if (config.output_key_object_) {
     // Note that this has the downside of containing sensitive data of the
     // private key.
     std::shared_ptr<KeyObjectData> data =
-          KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key));
+        KeyObjectData::CreateAsymmetric(kKeyTypePublic, *this);
     return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
   } else if (config.format_ == kKeyFormatJWK) {
     std::shared_ptr<KeyObjectData> data =
-        KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key));
+        KeyObjectData::CreateAsymmetric(kKeyTypePublic, *this);
     *out = Object::New(env->isolate());
     return ExportJWKInner(env, data, *out, false);
   }
 
-  return Tristate(WritePublicKey(env, key.get(), config).ToLocal(out));
+  return Tristate(WritePublicKey(env, get(), config).ToLocal(out));
 }
 
 Maybe<bool> ManagedEVPPKey::ToEncodedPrivateKey(
     Environment* env,
-    ManagedEVPPKey key,
     const PrivateKeyEncodingConfig& config,
     Local<Value>* out) {
-  if (!key) return Nothing<bool>();
+  if (!*this) return Nothing<bool>();
   if (config.output_key_object_) {
     std::shared_ptr<KeyObjectData> data =
-        KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key));
+        KeyObjectData::CreateAsymmetric(kKeyTypePrivate, *this);
     return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
   } else if (config.format_ == kKeyFormatJWK) {
     std::shared_ptr<KeyObjectData> data =
-        KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key));
+        KeyObjectData::CreateAsymmetric(kKeyTypePrivate, *this);
     *out = Object::New(env->isolate());
     return ExportJWKInner(env, data, *out, false);
   }
 
-  return Tristate(WritePrivateKey(env, key.get(), config).ToLocal(out));
+  return Tristate(WritePrivateKey(env, get(), config).ToLocal(out));
 }
 
 NonCopyableMaybe<PrivateKeyEncodingConfig>
diff --git a/src/crypto/crypto_keys.h b/src/crypto/crypto_keys.h
index d06565d8ad74a3..3889f3f321da45 100644
--- a/src/crypto/crypto_keys.h
+++ b/src/crypto/crypto_keys.h
@@ -112,17 +112,13 @@ class ManagedEVPPKey : public MemoryRetainer {
       unsigned int* offset,
       bool allow_key_object);
 
-  static v8::Maybe<bool> ToEncodedPublicKey(
-      Environment* env,
-      ManagedEVPPKey key,
-      const PublicKeyEncodingConfig& config,
-      v8::Local<v8::Value>* out);
+  v8::Maybe<bool> ToEncodedPublicKey(Environment* env,
+                                     const PublicKeyEncodingConfig& config,
+                                     v8::Local<v8::Value>* out);
 
-  static v8::Maybe<bool> ToEncodedPrivateKey(
-      Environment* env,
-      ManagedEVPPKey key,
-      const PrivateKeyEncodingConfig& config,
-      v8::Local<v8::Value>* out);
+  v8::Maybe<bool> ToEncodedPrivateKey(Environment* env,
+                                      const PrivateKeyEncodingConfig& config,
+                                      v8::Local<v8::Value>* out);
 
  private:
   size_t size_of_private_key() const;