Skip to content

Commit

Permalink
src: use BignumPointer and use BN_clear_free
Browse files Browse the repository at this point in the history
PR-URL: #50454
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
jasnell authored and RafaelGSS committed Jan 2, 2024
1 parent e5a8fa0 commit 96d64ed
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/crypto/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
33 changes: 17 additions & 16 deletions src/crypto/crypto_dh.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -162,13 +163,11 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) {
DH_R_BAD_GENERATOR, __FILE__, __LINE__);
return false;
}
BIGNUM* bn_p =
BN_bin2bn(reinterpret_cast<const unsigned char*>(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<const unsigned char*>(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())) {
return false;
}
return VerifyContext();
Expand All @@ -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__);
return false;
}
BIGNUM* bn_g =
BN_bin2bn(reinterpret_cast<const unsigned char*>(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<const unsigned char*>(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__);
return false;
}
BIGNUM* bn_p =
BN_bin2bn(reinterpret_cast<const unsigned char*>(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<const unsigned char*>(p), p_len, nullptr));
if (!DH_set0_pqg(dh_.get(), bn_p.get(), nullptr, bn_g.get())) {
return false;
}
// 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();
return VerifyContext();
}

Expand Down
2 changes: 1 addition & 1 deletion src/crypto/crypto_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down

0 comments on commit 96d64ed

Please sign in to comment.