From 04cba2782569480bf9984595088841a5df534b13 Mon Sep 17 00:00:00 2001
From: tb <tb@openbsd.org>
Date: Sun, 2 May 2021 15:33:33 +0000
Subject: [PATCH] Make TS_compute_imprint a bit more robust.

Instead of using the output parameters directly, null them out at the
beginning and work with local variables which are only assigned to the
output parameters on success. This way we avoid leaking stale pointers
back to the caller.

requested/ok jsing
---
 lib/libcrypto/ts/ts_rsp_verify.c | 48 +++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/lib/libcrypto/ts/ts_rsp_verify.c b/lib/libcrypto/ts/ts_rsp_verify.c
index 27515adf682..c745a2c51f0 100644
--- a/lib/libcrypto/ts/ts_rsp_verify.c
+++ b/lib/libcrypto/ts/ts_rsp_verify.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ts_rsp_verify.c,v 1.19 2021/05/01 13:13:45 tb Exp $ */
+/* $OpenBSD: ts_rsp_verify.c,v 1.20 2021/05/02 15:33:33 tb Exp $ */
 /* Written by Zoltan Glozik (zglozik@stones.com) for the OpenSSL
  * project 2002.
  */
@@ -593,35 +593,40 @@ TS_check_policy(ASN1_OBJECT *req_oid, TS_TST_INFO *tst_info)
 }
 
 static int
-TS_compute_imprint(BIO *data, TS_TST_INFO *tst_info, X509_ALGOR **md_alg,
-    unsigned char **imprint, unsigned *imprint_len)
+TS_compute_imprint(BIO *data, TS_TST_INFO *tst_info, X509_ALGOR **out_md_alg,
+    unsigned char **out_imprint, unsigned int *out_imprint_len)
 {
-	TS_MSG_IMPRINT *msg_imprint = TS_TST_INFO_get_msg_imprint(tst_info);
-	X509_ALGOR *md_alg_resp = TS_MSG_IMPRINT_get_algo(msg_imprint);
+	TS_MSG_IMPRINT *msg_imprint;
+	X509_ALGOR *md_alg_resp;
+	X509_ALGOR *md_alg = NULL;
+	unsigned char *imprint = NULL;
+	unsigned int imprint_len = 0;
 	const EVP_MD *md;
 	EVP_MD_CTX md_ctx;
 	unsigned char buffer[4096];
 	int length;
 
-	*md_alg = NULL;
-	*imprint = NULL;
+	*out_md_alg = NULL;
+	*out_imprint = NULL;
+	*out_imprint_len = 0;
 
-	/* Return the MD algorithm of the response. */
-	if (!(*md_alg = X509_ALGOR_dup(md_alg_resp)))
+	/* Retrieve the MD algorithm of the response. */
+	msg_imprint = TS_TST_INFO_get_msg_imprint(tst_info);
+	md_alg_resp = TS_MSG_IMPRINT_get_algo(msg_imprint);
+	if ((md_alg = X509_ALGOR_dup(md_alg_resp)) == NULL)
 		goto err;
 
 	/* Getting the MD object. */
-	if (!(md = EVP_get_digestbyobj((*md_alg)->algorithm))) {
+	if ((md = EVP_get_digestbyobj((md_alg)->algorithm)) == NULL) {
 		TSerror(TS_R_UNSUPPORTED_MD_ALGORITHM);
 		goto err;
 	}
 
 	/* Compute message digest. */
-	length = EVP_MD_size(md);
-	if (length < 0)
+	if ((length = EVP_MD_size(md)) < 0)
 		goto err;
-	*imprint_len = length;
-	if (!(*imprint = malloc(*imprint_len))) {
+	imprint_len = length;
+	if ((imprint = malloc(imprint_len)) == NULL) {
 		TSerror(ERR_R_MALLOC_FAILURE);
 		goto err;
 	}
@@ -632,17 +637,20 @@ TS_compute_imprint(BIO *data, TS_TST_INFO *tst_info, X509_ALGOR **md_alg,
 		if (!EVP_DigestUpdate(&md_ctx, buffer, length))
 			goto err;
 	}
-	if (!EVP_DigestFinal(&md_ctx, *imprint, NULL))
+	if (!EVP_DigestFinal(&md_ctx, imprint, NULL))
 		goto err;
 
+	*out_md_alg = md_alg;
+	md_alg = NULL;
+	*out_imprint = imprint;
+	imprint = NULL;
+	*out_imprint_len = imprint_len;
+
 	return 1;
 
 err:
-	X509_ALGOR_free(*md_alg);
-	*md_alg = NULL;
-	free(*imprint);
-	*imprint = NULL;
-	*imprint_len = 0;
+	X509_ALGOR_free(md_alg);
+	free(imprint);
 	return 0;
 }
 
-- 
2.20.1