Free ciphers before assigning to them
authortb <tb@openbsd.org>
Tue, 28 Jun 2022 14:51:37 +0000 (14:51 +0000)
committertb <tb@openbsd.org>
Tue, 28 Jun 2022 14:51:37 +0000 (14:51 +0000)
While this is not a leak currently, it definitely looks like one.
Pointed out by jsing on review of a diff that touched the vicinity
a while ago.

ok jsing

lib/libssl/ssl_srvr.c

index 35f3d58..20660cb 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.142 2022/06/07 17:14:17 tb Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.143 2022/06/28 14:51:37 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1074,6 +1074,7 @@ ssl3_get_client_hello(SSL *s)
                s->internal->hit = 1;
                s->session->verify_result = X509_V_OK;
 
+               sk_SSL_CIPHER_free(s->session->ciphers);
                s->session->ciphers = ciphers;
                ciphers = NULL;
 
@@ -1098,18 +1099,17 @@ ssl3_get_client_hello(SSL *s)
         */
 
        if (!s->internal->hit) {
-               sk_SSL_CIPHER_free(s->session->ciphers);
-               s->session->ciphers = ciphers;
                if (ciphers == NULL) {
                        al = SSL_AD_ILLEGAL_PARAMETER;
                        SSLerror(s, SSL_R_NO_CIPHERS_PASSED);
                        goto fatal_err;
                }
+               sk_SSL_CIPHER_free(s->session->ciphers);
+               s->session->ciphers = ciphers;
                ciphers = NULL;
-               c = ssl3_choose_cipher(s, s->session->ciphers,
-               SSL_get_ciphers(s));
 
-               if (c == NULL) {
+               if ((c = ssl3_choose_cipher(s, s->session->ciphers,
+                   SSL_get_ciphers(s))) == NULL) {
                        al = SSL_AD_HANDSHAKE_FAILURE;
                        SSLerror(s, SSL_R_NO_SHARED_CIPHER);
                        goto fatal_err;