From 0849e44220ec733a3261fe43bd35fd11b9c98036 Mon Sep 17 00:00:00 2001 From: Jon Simons Date: Tue, 30 Apr 2019 12:37:26 -0700 Subject: [PATCH] dh: fix libcrypto leak via ssh_dh_keypair_set_keys Upon SSH_OK, callers of `ssh_dh_keypair_set_keys` expect for ownership of the `priv` and `pub` values to be transferred away and eventually later managed by way of the `struct dh_ctx` at hand. The mbedTLS and gcrypt builds transfer ownership of these values in that way, but the libcrypto `ssh_dh_keypair_set_keys` is copying the given values with `BN_dup`. This causes a memory leak that can be seen with pkd and valgrind: valgrind --leak-check=full \ ./pkd_hello -i1 -t torture_pkd_openssh_dsa_rsa_diffie_hellman_group16_sha512 Fix the leak by replacing the `BN_dup` with direct assignment. Now the bignums will eventually be freed via `ssh_dh_cleanup`. Signed-off-by: Jon Simons Reviewed-by: Andreas Schneider --- src/dh_crypto.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dh_crypto.c b/src/dh_crypto.c index 20d38383..56a76fab 100644 --- a/src/dh_crypto.c +++ b/src/dh_crypto.c @@ -73,10 +73,10 @@ int ssh_dh_keypair_set_keys(struct dh_ctx *ctx, int peer, } if (priv) { - priv_key = BN_dup(priv); + priv_key = priv; } if (pub) { - pub_key = BN_dup(pub); + pub_key = pub; } (void)DH_set0_key(ctx->keypair[peer], pub_key, priv_key);