Don't double free a DH object
authorMatt Caswell <matt@openssl.org>
Mon, 30 Mar 2020 13:15:06 +0000 (14:15 +0100)
committerMatt Caswell <matt@openssl.org>
Wed, 1 Apr 2020 16:16:58 +0000 (17:16 +0100)
Having created a DH object and assigned it to an EVP_PKEY - we should
not free both the EVP_PKEY and the original DH. This will lead to a
double free occurring.

This issue was discovered and reported by GitHub Security Lab team member
Agustin Gianni.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/11441)

ssl/statem/statem_clnt.c

index 534902f..4c4e6cb 100644 (file)
@@ -2152,9 +2152,10 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey)
                  ERR_R_EVP_LIB);
         goto err;
     }
                  ERR_R_EVP_LIB);
         goto err;
     }
+    dh = NULL;
 
     if (!ssl_security(s, SSL_SECOP_TMP_DH, EVP_PKEY_security_bits(peer_tmp),
 
     if (!ssl_security(s, SSL_SECOP_TMP_DH, EVP_PKEY_security_bits(peer_tmp),
-                      0, dh)) {
+                      0, EVP_PKEY_get0_DH(peer_tmp))) {
         SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_TLS_PROCESS_SKE_DHE,
                  SSL_R_DH_KEY_TOO_SMALL);
         goto err;
         SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_TLS_PROCESS_SKE_DHE,
                  SSL_R_DH_KEY_TOO_SMALL);
         goto err;