Make ssl_create_cipher_list() have a single exit
authortb <tb@openbsd.org>
Sun, 4 Sep 2022 07:55:32 +0000 (07:55 +0000)
committertb <tb@openbsd.org>
Sun, 4 Sep 2022 07:55:32 +0000 (07:55 +0000)
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

index c42c3fd..d304cfe 100644 (file)
@@ -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 *