From: tb <tb@openbsd.org>
Date: Sun, 2 May 2021 15:33:33 +0000 (+0000)
Subject: Make TS_compute_imprint a bit more robust.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=04cba2782569480bf9984595088841a5df534b13;p=openbsd

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
---

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;
 }