From 65f3838cf1d1fae3660e9effe98b3431aa849af3 Mon Sep 17 00:00:00 2001
From: Toddr Bot <toddbot@rinaldo.us>
Date: Fri, 20 Mar 2026 02:11:49 +0000
Subject: [PATCH] fix: free BIGNUM parameters in _new_key_from_parameters() on
 OpenSSL 3.x

On pre-3.x, RSA_set0_key() and RSA_set0_factors() take ownership of
the BIGNUM parameters (n, e, d, p, q), so the caller must not free
them.  On 3.x, OSSL_PARAM_BLD_push_BN() copies the values internally,
leaving the original BIGNUMs orphaned.

Every call to new_key_from_parameters() on OpenSSL 3.x leaked 3-5
BIGNUMs (n, e, d, and optionally p, q).  Add BN_clear_free() calls
on both the success and error paths, guarded by the 3.x version check.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 RSA.xs | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/RSA.xs b/RSA.xs
index 65c8078..8841800 100644
--- a/RSA.xs
+++ b/RSA.xs
@@ -719,6 +719,14 @@ _new_key_from_parameters(proto, n, e, d, p, q)
 #endif
 #endif
 #if OPENSSL_VERSION_NUMBER >= 0x30000000L
+        /* OSSL_PARAM_BLD_push_BN() copies the value, so the original
+           BIGNUMs (from pointer_copy or BN_new) must be freed here.
+           On pre-3.x, RSA_set0_key/RSA_set0_factors took ownership. */
+        BN_clear_free(n);
+        BN_clear_free(e);
+        BN_clear_free(d);
+        BN_clear_free(p);
+        BN_clear_free(q);
         BN_clear_free(dmp1);
         BN_clear_free(dmq1);
         BN_clear_free(iqmp);
@@ -760,6 +768,9 @@ _new_key_from_parameters(proto, n, e, d, p, q)
         THROW( status > 0 && rsa != NULL );
         EVP_PKEY_CTX_free(pctx);
         pctx = NULL;
+        BN_clear_free(n);
+        BN_clear_free(e);
+        BN_clear_free(d);
 #else
         CHECK_OPEN_SSL(RSA_set0_key(rsa, n, e, d));
 #endif
@@ -771,10 +782,17 @@ _new_key_from_parameters(proto, n, e, d, p, q)
         goto end;
 
     err:
-        //if (p) BN_clear_free(p);
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+        /* On 3.x, push_BN copies, so originals are always ours to free.
+           On pre-3.x, RSA_set0_key/set0_factors may have taken ownership,
+           so these are intentionally skipped (risk of double-free). */
+        BN_clear_free(n);
+        BN_clear_free(e);
+        BN_clear_free(d);
+        BN_clear_free(p);
+        BN_clear_free(q);
+#endif
         if (p_minus_1) BN_clear_free(p_minus_1);
-        //if (q) BN_clear_free(q);
-        //if (d) BN_clear_free(d);
         if (q_minus_1) BN_clear_free(q_minus_1);
         if (dmp1) BN_clear_free(dmp1);
         if (dmq1) BN_clear_free(dmq1);