From 600de79768de60ca14422b12d72411b1100652ff Mon Sep 17 00:00:00 2001
From: tb <tb@openbsd.org>
Date: Sun, 4 Sep 2022 07:55:32 +0000
Subject: [PATCH] Make ssl_create_cipher_list() have a single exit

This simplifies memory management and makes it easier to see the leak
that were introduced in the previous commit. Sprinkle a few malloc
errors for consistency.

CID 278396

with/ok jsing
---
 lib/libssl/ssl_ciph.c | 46 +++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/lib/libssl/ssl_ciph.c b/lib/libssl/ssl_ciph.c
index c42c3fd22db..d304cfe6ec1 100644
--- a/lib/libssl/ssl_ciph.c
+++ b/lib/libssl/ssl_ciph.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_ciph.c,v 1.131 2022/09/01 15:19:16 tb Exp $ */
+/* $OpenBSD: ssl_ciph.c,v 1.132 2022/09/04 07:55:32 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1209,7 +1209,7 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method,
 {
 	int ok, num_of_ciphers, num_of_alias_max, num_of_group_aliases;
 	unsigned long disabled_mkey, disabled_auth, disabled_enc, disabled_mac, disabled_ssl;
-	STACK_OF(SSL_CIPHER) *cipherstack;
+	STACK_OF(SSL_CIPHER) *cipherstack = NULL, *ret = NULL;
 	const char *rule_p;
 	CIPHER_ORDER *co_list = NULL, *head = NULL, *tail = NULL, *curr;
 	const SSL_CIPHER **ca_list = NULL;
@@ -1222,7 +1222,7 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method,
 	 * Return with error if nothing to do.
 	 */
 	if (rule_str == NULL || cipher_list == NULL)
-		return NULL;
+		goto err;
 
 	/*
 	 * To reduce the work to do we only want to process the compiled
@@ -1239,7 +1239,7 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method,
 	co_list = reallocarray(NULL, num_of_ciphers, sizeof(CIPHER_ORDER));
 	if (co_list == NULL) {
 		SSLerrorx(ERR_R_MALLOC_FAILURE);
-		return(NULL);	/* Failure */
+		goto err;
 	}
 
 	ssl_cipher_collect_ciphers(ssl_method, num_of_ciphers,
@@ -1292,10 +1292,8 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method,
 
 	/* Now sort by symmetric encryption strength.  The above ordering remains
 	 * in force within each class */
-	if (!ssl_cipher_strength_sort(&head, &tail)) {
-		free(co_list);
-		return NULL;
-	}
+	if (!ssl_cipher_strength_sort(&head, &tail))
+		goto err;
 
 	/* Now disable everything (maintaining the ordering!) */
 	ssl_cipher_apply_rule(0, 0, 0, 0, 0, 0, 0, CIPHER_DEL, -1, &head, &tail);
@@ -1316,9 +1314,8 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method,
 	num_of_alias_max = num_of_ciphers + num_of_group_aliases + 1;
 	ca_list = reallocarray(NULL, num_of_alias_max, sizeof(SSL_CIPHER *));
 	if (ca_list == NULL) {
-		free(co_list);
 		SSLerrorx(ERR_R_MALLOC_FAILURE);
-		return(NULL);	/* Failure */
+		goto err;
 	}
 	ssl_cipher_collect_aliases(ca_list, num_of_group_aliases, disabled_mkey,
 	    disabled_auth, disabled_enc, disabled_mac, disabled_ssl, head);
@@ -1341,12 +1338,9 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method,
 		ok = ssl_cipher_process_rulestr(rule_p, &head, &tail, ca_list,
 		    cert, &tls13_seen);
 
-	free((void *)ca_list);	/* Not needed anymore */
-
 	if (!ok) {
 		/* Rule processing failure */
-		free(co_list);
-		return (NULL);
+		goto err;
 	}
 
 	/*
@@ -1354,8 +1348,8 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method,
 	 * if we cannot get one.
 	 */
 	if ((cipherstack = sk_SSL_CIPHER_new_null()) == NULL) {
-		free(co_list);
-		return (NULL);
+		SSLerrorx(ERR_R_MALLOC_FAILURE);
+		goto err;
 	}
 
 	/* Prefer TLSv1.3 cipher suites. */
@@ -1363,8 +1357,8 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method,
 		for (i = 0; i < sk_SSL_CIPHER_num(cipher_list_tls13); i++) {
 			cipher = sk_SSL_CIPHER_value(cipher_list_tls13, i);
 			if (!sk_SSL_CIPHER_push(cipherstack, cipher)) {
-				free(co_list);
-				return (NULL);
+				SSLerrorx(ERR_R_MALLOC_FAILURE);
+				goto err;
 			}
 		}
 		tls13_seen = 1;
@@ -1386,8 +1380,8 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method,
 		if (curr->active ||
 		    (!tls13_seen && curr->cipher->algorithm_ssl == SSL_TLSV1_3)) {
 			if (!sk_SSL_CIPHER_push(cipherstack, curr->cipher)) {
-				free(co_list);
-				return (NULL);
+				SSLerrorx(ERR_R_MALLOC_FAILURE);
+				goto err;
 			}
 		}
 		any_active |= curr->active;
@@ -1395,12 +1389,18 @@ ssl_create_cipher_list(const SSL_METHOD *ssl_method,
 	if (!any_active)
 		sk_SSL_CIPHER_zero(cipherstack);
 
-	free(co_list);	/* Not needed any longer */
-
 	sk_SSL_CIPHER_free(*cipher_list);
 	*cipher_list = cipherstack;
+	cipherstack = NULL;
+
+	ret = *cipher_list;
+
+ err:
+	sk_SSL_CIPHER_free(cipherstack);
+	free((void *)ca_list);
+	free(co_list);
 
-	return (cipherstack);
+	return ret;
 }
 
 const SSL_CIPHER *
-- 
2.20.1