From: Theo Buehler <tb@theobuehler.org>
Subject: Re: [PATCH] libressl: fix memory leak in CMS_EncryptedData_encrypt on error
To: Niels Dossche <niels.dossche@ugent.be>
Cc: tech@openbsd.org, jsing@openbsd.org, kenjiro@openbsd.org, beck@openbsd.org
Date: Mon, 3 Nov 2025 15:53:52 +0100

On Mon, Nov 03, 2025 at 02:38:15PM +0100, Niels Dossche wrote:
> Hi
> 
> This patch fixes a memory leak when an error occurs in
> CMS_EncryptedData_encrypt when calling CMS_EncryptedData_set1_key in
> libressl.
> 
> This issue was found via an experimental static analyzer I'm working on,
> and I manually read the code to verify whether this is a real bug or not.

Committed, thanks. Is that all your analyzer flagged in this file?
Surely there's more... :)

Concerning CMS_EncryptedData_encrypt() there's more that is wrong here
such as missing error checks and otherwise confusing logic, so I think
we're better off cleaning that up while we're there:

Index: cms/cms_smime.c
===================================================================
RCS file: /cvs/src/lib/libcrypto/cms/cms_smime.c,v
diff -u -p -r1.30 cms_smime.c
--- cms/cms_smime.c	3 Nov 2025 14:29:50 -0000	1.30
+++ cms/cms_smime.c	3 Nov 2025 14:35:52 -0000
@@ -277,27 +277,32 @@ CMS_ContentInfo *
 CMS_EncryptedData_encrypt(BIO *in, const EVP_CIPHER *cipher,
     const unsigned char *key, size_t keylen, unsigned int flags)
 {
-	CMS_ContentInfo *cms;
+	CMS_ContentInfo *cms = NULL;
 
-	if (!cipher) {
+	if (cipher == NULL) {
 		CMSerror(CMS_R_NO_CIPHER);
-		return NULL;
+		goto err;
 	}
-	cms = CMS_ContentInfo_new();
-	if (cms == NULL)
-		return NULL;
-	if (!CMS_EncryptedData_set1_key(cms, cipher, key, keylen)) {
-		CMS_ContentInfo_free(cms);
-		return NULL;
+
+	if ((cms = CMS_ContentInfo_new()) == NULL)
+		goto err;
+
+	if (!CMS_EncryptedData_set1_key(cms, cipher, key, keylen))
+		goto err;
+
+	if ((flags & CMS_DETACHED) == 0) {
+		if (!CMS_set_detached(cms, 0))
+			goto err;
 	}
 
-	if (!(flags & CMS_DETACHED))
-		CMS_set_detached(cms, 0);
+	if ((flags & (CMS_STREAM | CMS_PARTIAL)) == 0) {
+		if (!CMS_final(cms, in, NULL, flags))
+			goto err;
+	}
 
-	if ((flags & (CMS_STREAM | CMS_PARTIAL)) ||
-	    CMS_final(cms, in, NULL, flags))
-		return cms;
+	return cms;
 
+ err:
 	CMS_ContentInfo_free(cms);
 
 	return NULL;