From 36d01cdb9b39d784c39fdd6ac1aedc2eda1aebdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 29 Jul 2024 08:55:40 +0200 Subject: [PATCH] Fix memory leak in wc_GeneratePreTBS() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the wc_GeneratePreTBS() method (used for WOLFSSL_DUAL_ALG_CERTS support), there was a workaround for alt names in certificates, as the CopyDecodedToX509() method wasn't properly copying them. As a proper copy mechanism is implemented now, we have to remove the workaround as it now causes a memory leak of the copied values. Signed-off-by: Tobias Frauenschläger --- src/x509.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/x509.c b/src/x509.c index 04e2a8be93..035e1b190c 100644 --- a/src/x509.c +++ b/src/x509.c @@ -7521,20 +7521,12 @@ int wolfSSL_i2d_X509(WOLFSSL_X509* x509, unsigned char** out) int wc_GeneratePreTBS(DecodedCert* cert, byte *der, int derSz) { int ret = 0; WOLFSSL_X509 *x = NULL; - byte certOwnsAltNames = 0; byte certIsCSR = 0; if ((cert == NULL) || (der == NULL) || (derSz <= 0)) { return BAD_FUNC_ARG; } - /* The call to CopyDecodedToX509() transfers ownership of the altNames in - * the DecodedCert to the temporary X509 object, causing the list to be - * freed in wolfSSL_X509_free(). As this is an unintended side-effect, we - * have to save the ownerFlag here and transfer ownership back to the - * DecodedCert prior to freeing the X509 object. */ - certOwnsAltNames = cert->weOwnAltNames; - #ifdef WOLFSSL_CERT_REQ certIsCSR = cert->isCSR; #endif @@ -7547,9 +7539,6 @@ int wc_GeneratePreTBS(DecodedCert* cert, byte *der, int derSz) { ret = CopyDecodedToX509(x, cert); } - /* CopyDecodedToX509() clears cert->weOwnAltNames. Restore it. */ - cert->weOwnAltNames = certOwnsAltNames; - if (ret == 0) { /* Remove the altsigval extension. */ XFREE(x->altSigValDer, x->heap, DYNAMIC_TYPE_X509_EXT); @@ -7565,9 +7554,6 @@ int wc_GeneratePreTBS(DecodedCert* cert, byte *der, int derSz) { } if (x != NULL) { - /* Safe the altNames list from being freed unitentionally. */ - x->altNames = NULL; - wolfSSL_X509_free(x); }