From 24452362ebf30bc2c372dd60d3b7f77edb408ce2 Mon Sep 17 00:00:00 2001
From: tb <tb@openbsd.org>
Date: Wed, 18 Oct 2023 07:30:49 +0000
Subject: [PATCH] Use X509_ALGOR_set_evp_md() in CMS_add1_signer()

Contrary to X509_ALGOR_set_md() this allows for error checking. Avoid
local complications by freeing in the exit path and use a const version
of X509_ALGOR for walking a STACK_OF() to avoid a bad free.

Clean up includes

ok jsing
---
 lib/libcrypto/cms/cms_sd.c | 40 +++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/lib/libcrypto/cms/cms_sd.c b/lib/libcrypto/cms/cms_sd.c
index 245822971bf..5f472311d05 100644
--- a/lib/libcrypto/cms/cms_sd.c
+++ b/lib/libcrypto/cms/cms_sd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: cms_sd.c,v 1.28 2023/09/11 09:29:30 tb Exp $ */
+/* $OpenBSD: cms_sd.c,v 1.29 2023/10/18 07:30:49 tb Exp $ */
 /*
  * Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project.
@@ -52,18 +52,22 @@
  * ====================================================================
  */
 
+#include <stdlib.h>
 #include <string.h>
 
-#include "cryptlib.h"
-#include <openssl/asn1t.h>
-#include <openssl/pem.h>
-#include <openssl/x509.h>
-#include <openssl/x509v3.h>
+#include <openssl/asn1.h>
+#include <openssl/bio.h>
 #include <openssl/err.h>
+#include <openssl/evp.h>
 #include <openssl/cms.h>
+#include <openssl/objects.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+
+#include "asn1_local.h"
 #include "cms_local.h"
-#include "asn1/asn1_local.h"
-#include "evp/evp_local.h"
+#include "evp_local.h"
+#include "x509_local.h"
 
 /* CMS SignedData Utilities */
 
@@ -279,7 +283,7 @@ CMS_add1_signer(CMS_ContentInfo *cms, X509 *signer, EVP_PKEY *pk,
 {
 	CMS_SignedData *sd;
 	CMS_SignerInfo *si = NULL;
-	X509_ALGOR *alg;
+	X509_ALGOR *alg = NULL;
 	int i, type;
 
 	if (!X509_check_private_key(signer, pk)) {
@@ -337,26 +341,29 @@ CMS_add1_signer(CMS_ContentInfo *cms, X509 *signer, EVP_PKEY *pk,
 		goto err;
 	}
 
-	X509_ALGOR_set_md(si->digestAlgorithm, md);
+	if (!X509_ALGOR_set_evp_md(si->digestAlgorithm, md))
+		goto err;
 
 	/* See if digest is present in digestAlgorithms */
 	for (i = 0; i < sk_X509_ALGOR_num(sd->digestAlgorithms); i++) {
+		const X509_ALGOR *x509_alg;
 		const ASN1_OBJECT *aoid;
-		alg = sk_X509_ALGOR_value(sd->digestAlgorithms, i);
-		X509_ALGOR_get0(&aoid, NULL, NULL, alg);
+
+		x509_alg = sk_X509_ALGOR_value(sd->digestAlgorithms, i);
+		X509_ALGOR_get0(&aoid, NULL, NULL, x509_alg);
 		if (OBJ_obj2nid(aoid) == EVP_MD_type(md))
 			break;
 	}
 
 	if (i == sk_X509_ALGOR_num(sd->digestAlgorithms)) {
-		alg = X509_ALGOR_new();
-		if (alg == NULL)
+		if ((alg = X509_ALGOR_new()) == NULL)
+			goto merr;
+		if (!X509_ALGOR_set_evp_md(alg, md))
 			goto merr;
-		X509_ALGOR_set_md(alg, md);
 		if (!sk_X509_ALGOR_push(sd->digestAlgorithms, alg)) {
-			X509_ALGOR_free(alg);
 			goto merr;
 		}
+		alg = NULL;
 	}
 
 	if (!(flags & CMS_KEY_PARAM) && !cms_sd_asn1_ctrl(si, 0))
@@ -422,6 +429,7 @@ CMS_add1_signer(CMS_ContentInfo *cms, X509 *signer, EVP_PKEY *pk,
 	CMSerror(ERR_R_MALLOC_FAILURE);
  err:
 	ASN1_item_free((ASN1_VALUE *)si, &CMS_SignerInfo_it);
+	X509_ALGOR_free(alg);
 
 	return NULL;
 }
-- 
2.20.1