From a767aa5890f6b383c282f34cf5194ffb67c2dede Mon Sep 17 00:00:00 2001 From: Toddr Bot Date: Sat, 14 Mar 2026 21:42:47 +0000 Subject: [PATCH 1/2] fix: prevent resource leaks in generate_key() and get_public_key_string() generate_key() leaked the BIGNUM exponent (e) and EVP_PKEY_CTX on all OpenSSL version paths when error-path croaks fired before cleanup: - Pre-0.00908: e was never freed - 1.1.x: e and rsa leaked when RSA_generate_key_ex() failed - 3.x: CHECK_OPEN_SSL croaks bypassed BN_free(e) and EVP_PKEY_CTX_free(ctx) Replace CHECK_OPEN_SSL chain with short-circuit conditional that frees all resources before calling croakSsl(). For 1.1.x, free e and rsa before croak() on key generation failure. get_public_key_string() leaked the OSSL_ENCODER_CTX and BIO when the second CHECK_OPEN_SSL croak fired after encoder context allocation. Replace with short-circuit conditional for consistent cleanup. --- RSA.xs | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/RSA.xs b/RSA.xs index 4b54c93..1a583e6 100644 --- a/RSA.xs +++ b/RSA.xs @@ -474,9 +474,13 @@ get_public_key_string(p_rsa) ctx = OSSL_ENCODER_CTX_new_for_pkey(p_rsa->rsa, OSSL_KEYMGMT_SELECT_PUBLIC_KEY, "PEM", "PKCS1", NULL); - CHECK_OPEN_SSL(ctx != NULL && OSSL_ENCODER_CTX_get_num_encoders(ctx)); - - CHECK_OPEN_SSL(OSSL_ENCODER_to_bio(ctx, stringBIO) == 1); + if (!ctx || !OSSL_ENCODER_CTX_get_num_encoders(ctx) + || OSSL_ENCODER_to_bio(ctx, stringBIO) != 1) + { + OSSL_ENCODER_CTX_free(ctx); + BIO_free(stringBIO); + croakSsl(__FILE__, __LINE__); + } OSSL_ENCODER_CTX_free(ctx); #else @@ -522,22 +526,30 @@ generate_key(proto, bitsSV, exponent = 65537) #if OPENSSL_VERSION_NUMBER >= 0x00908000L && OPENSSL_VERSION_NUMBER < 0x30000000L rsa = RSA_new(); if (!RSA_generate_key_ex(rsa, SvIV(bitsSV), e, NULL)) + { + BN_free(e); + RSA_free(rsa); croak("Unable to generate a key"); + } BN_free(e); CHECK_OPEN_SSL(rsa != NULL); #endif #if OPENSSL_VERSION_NUMBER >= 0x30000000L ctx = EVP_PKEY_CTX_new_from_name(NULL, "RSA", NULL); - CHECK_OPEN_SSL(ctx); - CHECK_OPEN_SSL(EVP_PKEY_keygen_init(ctx) == 1); - CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, SvIV(bitsSV)) > 0); - CHECK_OPEN_SSL(EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx, e) >0); - CHECK_OPEN_SSL(EVP_PKEY_generate(ctx, &rsa) == 1); - CHECK_OPEN_SSL(rsa != NULL); - - if (e != NULL) + if (!ctx + || EVP_PKEY_keygen_init(ctx) != 1 + || EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, SvIV(bitsSV)) <= 0 + || EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx, e) <= 0 + || EVP_PKEY_generate(ctx, &rsa) != 1 + || rsa == NULL) + { BN_free(e); + EVP_PKEY_CTX_free(ctx); + croakSsl(__FILE__, __LINE__); + } + + BN_free(e); EVP_PKEY_CTX_free(ctx); #endif CHECK_OPEN_SSL(rsa); From 5ae2ee7da2772b3fc7a8101753590f15faa281cf Mon Sep 17 00:00:00 2001 From: Toddr Bot Date: Mon, 16 Mar 2026 23:47:22 +0000 Subject: [PATCH 2/2] rebase: apply review feedback on #101 --- RSA.xs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/RSA.xs b/RSA.xs index 1a583e6..8293650 100644 --- a/RSA.xs +++ b/RSA.xs @@ -527,12 +527,11 @@ generate_key(proto, bitsSV, exponent = 65537) rsa = RSA_new(); if (!RSA_generate_key_ex(rsa, SvIV(bitsSV), e, NULL)) { - BN_free(e); - RSA_free(rsa); - croak("Unable to generate a key"); + BN_free(e); + RSA_free(rsa); + croak("Unable to generate a key"); } BN_free(e); - CHECK_OPEN_SSL(rsa != NULL); #endif #if OPENSSL_VERSION_NUMBER >= 0x30000000L ctx = EVP_PKEY_CTX_new_from_name(NULL, "RSA", NULL);