src: use BignumPointer and use BN_clear_free

PR-URL: #50454 Reviewed-By: Joyee Cheung <[email protected]>

Author: jasnell

src/crypto/README.md

@@ -82,7 +82,7 @@ using EVPKeyCtxPointer = DeleteFnPtr<EVP_PKEY_CTX, EVP_PKEY_CTX_free>
 using EVPMDPointer = DeleteFnPtr<EVP_MD_CTX, EVP_MD_CTX_free>
 using RSAPointer = DeleteFnPtr<RSA, RSA_free>
 using ECPointer = DeleteFnPtr<EC_KEY, EC_KEY_free>
-using BignumPointer = DeleteFnPtr<BIGNUM, BN_free>
+using BignumPointer = DeleteFnPtr<BIGNUM, BN_clear_free>
 using NetscapeSPKIPointer = DeleteFnPtr<NETSCAPE_SPKI, NETSCAPE_SPKI_free>
 using ECGroupPointer = DeleteFnPtr<EC_GROUP, EC_GROUP_free>
 using ECPointPointer = DeleteFnPtr<EC_POINT, EC_POINT_free>

src/crypto/crypto_dh.cc

@@ -2,6 +2,7 @@
 #include"async_wrap-inl.h"
 #include"base_object-inl.h"
 #include"crypto/crypto_keys.h"
+#include"crypto/crypto_util.h"
 #include"env-inl.h"
 #include"memory_tracker-inl.h"
 #include"threadpoolwork-inl.h"
@@ -162,13 +163,11 @@ bool DiffieHellman::Init(const char* p, int p_len, int g){
  DH_R_BAD_GENERATOR, __FILE__, __LINE__);
 returnfalse;
  }
- BIGNUM* bn_p =
-BN_bin2bn(reinterpret_cast<constunsignedchar*>(p), p_len, nullptr);
- BIGNUM* bn_g = BN_new();
-if (!BN_set_word(bn_g, g) ||
- !DH_set0_pqg(dh_.get(), bn_p, nullptr, bn_g)){
-BN_free(bn_p);
-BN_free(bn_g);
+ BignumPointer bn_p(
+BN_bin2bn(reinterpret_cast<constunsignedchar*>(p), p_len, nullptr));
+ BignumPointer bn_g(BN_new());
+if (bn_p == nullptr || bn_g == nullptr || !BN_set_word(bn_g.get(), g) ||
+ !DH_set0_pqg(dh_.get(), bn_p.release(), nullptr, bn_g.release())){
 returnfalse;
  }
 returnVerifyContext();
@@ -186,21 +185,23 @@ bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len){
  DH_R_BAD_GENERATOR, __FILE__, __LINE__);
 returnfalse;
  }
- BIGNUM* bn_g =
-BN_bin2bn(reinterpret_cast<constunsignedchar*>(g), g_len, nullptr);
-if (BN_is_zero(bn_g) || BN_is_one(bn_g)){
-BN_free(bn_g);
+ BignumPointer bn_g(
+BN_bin2bn(reinterpret_cast<constunsignedchar*>(g), g_len, nullptr));
+if (BN_is_zero(bn_g.get()) || BN_is_one(bn_g.get())){
 ERR_put_error(ERR_LIB_DH, DH_F_DH_BUILTIN_GENPARAMS,
  DH_R_BAD_GENERATOR, __FILE__, __LINE__);
 returnfalse;
  }
- BIGNUM* bn_p =
-BN_bin2bn(reinterpret_cast<constunsignedchar*>(p), p_len, nullptr);
-if (!DH_set0_pqg(dh_.get(), bn_p, nullptr, bn_g)){
-BN_free(bn_p);
-BN_free(bn_g);
+ BignumPointer bn_p(
+BN_bin2bn(reinterpret_cast<constunsignedchar*>(p), p_len, nullptr));
+if (!DH_set0_pqg(dh_.get(), bn_p.get(), nullptr, bn_g.get())){
 returnfalse;
  }
+// The DH_set0_pqg call above takes ownership of the bignums on success,
+// so we should release them here so we don't end with a possible
+// use-after-free or double free.
+ bn_p.release();
+ bn_g.release();
 returnVerifyContext();
 }
 

src/crypto/crypto_util.h

@@ -65,7 +65,7 @@ using EVPKeyCtxPointer = DeleteFnPtr<EVP_PKEY_CTX, EVP_PKEY_CTX_free>
 using EVPMDPointer = DeleteFnPtr<EVP_MD_CTX, EVP_MD_CTX_free>
 using RSAPointer = DeleteFnPtr<RSA, RSA_free>
 using ECPointer = DeleteFnPtr<EC_KEY, EC_KEY_free>
-using BignumPointer = DeleteFnPtr<BIGNUM, BN_free>
+using BignumPointer = DeleteFnPtr<BIGNUM, BN_clear_free>
 using BignumCtxPointer = DeleteFnPtr<BN_CTX, BN_CTX_free>
 using NetscapeSPKIPointer = DeleteFnPtr<NETSCAPE_SPKI, NETSCAPE_SPKI_free>
 using ECGroupPointer = DeleteFnPtr<EC_GROUP, EC_GROUP_free>