From 3f009363695acaa814ce4a66fbefb0343b18698f Mon Sep 17 00:00:00 2001 From: ilammy Date: Wed, 4 Sep 2019 11:32:13 +0300 Subject: [PATCH 1/3] Avoid double-free of pub_exp (OpenSSL) We should not free pub_exp after we have successfully passed it into EVP_PKEY_CTX with EVP_PKEY_CTX_ctrl(EVP_PKEY_CTRL_RSA_KEYGEN_PUBEXP) call. Otherwise we'll free the same object twice when calling EVP_PKEY_CTX_free(). --- src/soter/openssl/soter_rsa_key_pair_gen.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/soter/openssl/soter_rsa_key_pair_gen.c b/src/soter/openssl/soter_rsa_key_pair_gen.c index 8b9278fcb..56c074e17 100644 --- a/src/soter/openssl/soter_rsa_key_pair_gen.c +++ b/src/soter/openssl/soter_rsa_key_pair_gen.c @@ -76,9 +76,8 @@ soter_status_t soter_rsa_key_pair_gen_init(soter_rsa_key_pair_gen_t* ctx, const EVP_PKEY_CTRL_RSA_KEYGEN_BITS, rsa_key_length(key_length), NULL)), - (BN_free(pub_exp), EVP_PKEY_CTX_free(ctx->pkey_ctx))); - SOTER_IF_FAIL(EVP_PKEY_keygen(ctx->pkey_ctx, &pkey), - (BN_free(pub_exp), EVP_PKEY_CTX_free(ctx->pkey_ctx))); + (EVP_PKEY_CTX_free(ctx->pkey_ctx))); + SOTER_IF_FAIL(EVP_PKEY_keygen(ctx->pkey_ctx, &pkey), (EVP_PKEY_CTX_free(ctx->pkey_ctx))); return SOTER_SUCCESS; } From 35108fe4858118d9330a381cee55524afd528c80 Mon Sep 17 00:00:00 2001 From: ilammy Date: Wed, 4 Sep 2019 11:34:47 +0300 Subject: [PATCH 2/3] Avoid double-free of pub_exp (BoringSSL) We should not free pub_exp after we have successfully passed it into EVP_PKEY_CTX with EVP_PKEY_CTX_set_rsa_keygen_pubexp() call. Otherwise we'll free the same object twice when calling EVP_PKEY_CTX_free(). --- src/soter/boringssl/soter_rsa_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/soter/boringssl/soter_rsa_common.c b/src/soter/boringssl/soter_rsa_common.c index 75cec2f0c..074b47590 100644 --- a/src/soter/boringssl/soter_rsa_common.c +++ b/src/soter/boringssl/soter_rsa_common.c @@ -74,7 +74,7 @@ soter_status_t soter_rsa_gen_key(EVP_PKEY_CTX* pkey_ctx, const unsigned key_leng /* Override default key size for RSA key. Currently OpenSSL has default key size of 1024. * LibreSSL has 2048. We will put 2048 explicitly */ SOTER_IF_FAIL((1 <= EVP_PKEY_CTX_set_rsa_keygen_bits(pkey_ctx, rsa_key_length(key_length))), - (BN_free(pub_exp), EVP_PKEY_CTX_free(pkey_ctx))); + (EVP_PKEY_CTX_free(pkey_ctx))); if (!EVP_PKEY_keygen(pkey_ctx, &pkey)) { return SOTER_FAIL; From ee4fb1167391f1d1b60abaac6e7ee145e4fe6dc3 Mon Sep 17 00:00:00 2001 From: ilammy Date: Wed, 4 Sep 2019 11:37:56 +0300 Subject: [PATCH 3/3] Avoid double-free of EVP_PKEY_CTX (BoringSSL) We do not own pkey_ctx in soter_rsa_gen_key(). It is owned by the caller: soter_rsa_key_pair_gen_init(). We should not free it ourselves in case of errors. (And we should not use magic macros unless really necessary.) --- src/soter/boringssl/soter_rsa_common.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/soter/boringssl/soter_rsa_common.c b/src/soter/boringssl/soter_rsa_common.c index 074b47590..c047f516d 100644 --- a/src/soter/boringssl/soter_rsa_common.c +++ b/src/soter/boringssl/soter_rsa_common.c @@ -69,12 +69,15 @@ soter_status_t soter_rsa_gen_key(EVP_PKEY_CTX* pkey_ctx, const unsigned key_leng return SOTER_FAIL; } - SOTER_IF_FAIL(1 <= EVP_PKEY_CTX_set_rsa_keygen_pubexp(pkey_ctx, pub_exp), - (BN_free(pub_exp), EVP_PKEY_CTX_free(pkey_ctx))); + if (1 > EVP_PKEY_CTX_set_rsa_keygen_pubexp(pkey_ctx, pub_exp)) { + BN_free(pub_exp); + return SOTER_FAIL; + } /* Override default key size for RSA key. Currently OpenSSL has default key size of 1024. * LibreSSL has 2048. We will put 2048 explicitly */ - SOTER_IF_FAIL((1 <= EVP_PKEY_CTX_set_rsa_keygen_bits(pkey_ctx, rsa_key_length(key_length))), - (EVP_PKEY_CTX_free(pkey_ctx))); + if (1 > EVP_PKEY_CTX_set_rsa_keygen_bits(pkey_ctx, rsa_key_length(key_length))) { + return SOTER_FAIL; + } if (!EVP_PKEY_keygen(pkey_ctx, &pkey)) { return SOTER_FAIL;