From e99ab8ffd70c24a68b8e9c46da1d669fe0bed810 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 10 Jul 2016 23:35:04 -0400 Subject: [PATCH] Fix DH error-handling in tls_process_key_exchange. The set0 setters take ownership of their arguments, so the values should be set to NULL to avoid a double-free in the cleanup block should ssl_security(SSL_SECOP_TMP_DH) fail. Found by BoringSSL's WeakDH test. Reviewed-by: Kurt Roeckx Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/1299) --- ssl/statem/statem_clnt.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 9fae19028c..be4ba9cead 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1461,12 +1461,14 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_BN_LIB); goto dherr; } + p = g = NULL; if (!DH_set0_key(dh, bnpub_key, NULL)) { al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_BN_LIB); goto dherr; } + bnpub_key = NULL; if (!ssl_security(s, SSL_SECOP_TMP_DH, DH_security_bits(dh), 0, dh)) { al = SSL_AD_HANDSHAKE_FAILURE; -- 2.25.1