summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJon Simons <jon@jonsimons.org>2019-04-30 12:37:26 -0700
committerAndreas Schneider <asn@cryptomilk.org>2019-05-09 17:37:33 +0200
commit0849e44220ec733a3261fe43bd35fd11b9c98036 (patch)
treeb6ce02e2399dc6f48f240155bd402bc456063ff9
parentee42e3badba48083b5dd06bc3eb2111b46120723 (diff)
downloadlibssh-master-fix.tar.gz
libssh-master-fix.tar.xz
libssh-master-fix.zip
dh: fix libcrypto leak via ssh_dh_keypair_set_keysmaster-fix
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 <jon@jonsimons.org> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
-rw-r--r--src/dh_crypto.c4
1 files 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);