From 24638211da59aaea93f3f85d8dd6ef0a36a8644e Mon Sep 17 00:00:00 2001 From: Todd Short Date: Fri, 26 May 2017 08:42:21 -0400 Subject: [PATCH] Fix ex_data memory leak Code was added in commit 62f488d that overwrite the last ex_data valye using CRYPTO_dup_ex_data() causing a memory leak and potentially confusing the ex_data dup() callback. In ssl_session_dup(), new-up the ex_data before calling CRYPTO_dup_ex_data(); all the other structures that dup ex_data have the destination ex_data new'd before the dup. Reviewed-by: Andy Polyakov Reviewed-by: Matt Caswell Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3568) --- crypto/ex_data.c | 9 ++++++++- ssl/ssl_sess.c | 6 ++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crypto/ex_data.c b/crypto/ex_data.c index 108a1959ea..723b21b3d2 100644 --- a/crypto/ex_data.c +++ b/crypto/ex_data.c @@ -473,7 +473,14 @@ static int int_dup_ex_data(int class_index, CRYPTO_EX_DATA *to, if (j < mx) mx = j; if (mx > 0) { - if (!CRYPTO_set_ex_data(to, mx - 1, NULL)) + /* + * Make sure the ex_data stack is at least |mx| elements long to avoid + * issues in the for loop that follows; so go get the |mx|'th element + * (if it does not exist CRYPTO_get_ex_data() returns NULL), and assign + * to itself. This is normally a no-op; but ensures the stack is the + * proper size + */ + if (!CRYPTO_set_ex_data(to, mx - 1, CRYPTO_get_ex_data(to, mx - 1))) goto skip; storage = OPENSSL_malloc(mx * sizeof(CRYPTO_EX_DATA_FUNCS *)); if (!storage) diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index f50f514212..23dd3e7a01 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -261,7 +261,6 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket) #ifndef OPENSSL_NO_SRP dest->srp_username = NULL; #endif - memset(&dest->ex_data, 0, sizeof(dest->ex_data)); /* We deliberately don't copy the prev and next pointers */ dest->prev = NULL; @@ -275,6 +274,9 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket) if (src->peer != NULL) CRYPTO_add(&src->peer->references, 1, CRYPTO_LOCK_X509); + if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_SSL_SESSION, dest, &dest->ex_data)) + goto err; + #ifndef OPENSSL_NO_PSK if (src->psk_identity_hint) { dest->psk_identity_hint = BUF_strdup(src->psk_identity_hint); @@ -325,7 +327,7 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket) } # endif - if (ticket != 0) { + if (ticket != 0 && src->tlsext_tick != NULL) { dest->tlsext_tick = BUF_memdup(src->tlsext_tick, src->tlsext_ticklen); if(dest->tlsext_tick == NULL) goto err; -- 2.25.1