From c3903c38b0fd168ab3d925c2b129d1a599593426 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Mon, 13 Aug 2018 02:41:05 +0000 Subject: [PATCH] upstream: revert compat.[ch] section of the following change. It causes double-free under some circumstances. -- date: 2018/07/31 03:07:24; author: djm; state: Exp; lines: +33 -18; commitid: f7g4UI8eeOXReTPh; fix some memory leaks spotted by Coverity via Jakub Jelen in bz#2366 feedback and ok dtucker@ OpenBSD-Commit-ID: 1e77547f60fdb5e2ffe23e2e4733c54d8d2d1137 --- compat.c | 51 ++++++++++++++++++--------------------------------- compat.h | 14 ++++---------- sshconnect2.c | 15 +++++++-------- sshd.c | 10 +++++----- 4 files changed, 34 insertions(+), 56 deletions(-) diff --git a/compat.c b/compat.c index 563e13331..0624dc6de 100644 --- a/compat.c +++ b/compat.c @@ -1,4 +1,4 @@ -/* $OpenBSD: compat.c,v 1.112 2018/07/31 03:07:24 djm Exp $ */ +/* $OpenBSD: compat.c,v 1.113 2018/08/13 02:41:05 djm Exp $ */ /* * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl. All rights reserved. * @@ -184,17 +184,13 @@ proto_spec(const char *spec) } char * -compat_cipher_proposal(char *cipher_prop, u_int compat) +compat_cipher_proposal(char *cipher_prop) { - char *cp; - - if (!(compat & SSH_BUG_BIGENDIANAES)) + if (!(datafellows & SSH_BUG_BIGENDIANAES)) return cipher_prop; debug2("%s: original cipher proposal: %s", __func__, cipher_prop); - if ((cp = match_filter_blacklist(cipher_prop, "aes*")) == NULL) + if ((cipher_prop = match_filter_blacklist(cipher_prop, "aes*")) == NULL) fatal("match_filter_blacklist failed"); - free(cipher_prop); - cipher_prop = cp; debug2("%s: compat cipher proposal: %s", __func__, cipher_prop); if (*cipher_prop == '\0') fatal("No supported ciphers found"); @@ -202,17 +198,13 @@ compat_cipher_proposal(char *cipher_prop, u_int compat) } char * -compat_pkalg_proposal(char *pkalg_prop, u_int compat) +compat_pkalg_proposal(char *pkalg_prop) { - char *cp; - - if (!(compat & SSH_BUG_RSASIGMD5)) + if (!(datafellows & SSH_BUG_RSASIGMD5)) return pkalg_prop; debug2("%s: original public key proposal: %s", __func__, pkalg_prop); - if ((cp = match_filter_blacklist(pkalg_prop, "ssh-rsa")) == NULL) + if ((pkalg_prop = match_filter_blacklist(pkalg_prop, "ssh-rsa")) == NULL) fatal("match_filter_blacklist failed"); - free(pkalg_prop); - pkalg_prop = cp; debug2("%s: compat public key proposal: %s", __func__, pkalg_prop); if (*pkalg_prop == '\0') fatal("No supported PK algorithms found"); @@ -220,31 +212,24 @@ compat_pkalg_proposal(char *pkalg_prop, u_int compat) } char * -compat_kex_proposal(char *kex_prop, u_int compat) +compat_kex_proposal(char *p) { - char *cp; - - if ((compat & (SSH_BUG_CURVE25519PAD|SSH_OLD_DHGEX)) == 0) - return kex_prop; - debug2("%s: original KEX proposal: %s", __func__, kex_prop); - if ((compat & SSH_BUG_CURVE25519PAD) != 0) { - if ((cp = match_filter_blacklist(kex_prop, + if ((datafellows & (SSH_BUG_CURVE25519PAD|SSH_OLD_DHGEX)) == 0) + return p; + debug2("%s: original KEX proposal: %s", __func__, p); + if ((datafellows & SSH_BUG_CURVE25519PAD) != 0) + if ((p = match_filter_blacklist(p, "curve25519-sha256@libssh.org")) == NULL) fatal("match_filter_blacklist failed"); - free(kex_prop); - kex_prop = cp; - } - if ((compat & SSH_OLD_DHGEX) != 0) { - if ((cp = match_filter_blacklist(kex_prop, + if ((datafellows & SSH_OLD_DHGEX) != 0) { + if ((p = match_filter_blacklist(p, "diffie-hellman-group-exchange-sha256," "diffie-hellman-group-exchange-sha1")) == NULL) fatal("match_filter_blacklist failed"); - free(kex_prop); - kex_prop = cp; } - debug2("%s: compat KEX proposal: %s", __func__, kex_prop); - if (*kex_prop == '\0') + debug2("%s: compat KEX proposal: %s", __func__, p); + if (*p == '\0') fatal("No supported key exchange algorithms found"); - return kex_prop; + return p; } diff --git a/compat.h b/compat.h index e2877737b..d611d33e7 100644 --- a/compat.h +++ b/compat.h @@ -1,4 +1,4 @@ -/* $OpenBSD: compat.h,v 1.53 2018/07/31 03:07:24 djm Exp $ */ +/* $OpenBSD: compat.h,v 1.54 2018/08/13 02:41:05 djm Exp $ */ /* * Copyright (c) 1999, 2000, 2001 Markus Friedl. All rights reserved. @@ -65,15 +65,9 @@ u_int compat_datafellows(const char *); int proto_spec(const char *); - -/* - * compat_*_proposal will update their respective proposals based on the - * active compat flags. The replacement is performed in-place - i.e. they - * will free their argument and return a new heap-allocated string. - */ -char *compat_cipher_proposal(char *, u_int compat); -char *compat_pkalg_proposal(char *, u_int compat); -char *compat_kex_proposal(char *, u_int compat); +char *compat_cipher_proposal(char *); +char *compat_pkalg_proposal(char *); +char *compat_kex_proposal(char *); extern int datafellows; #endif diff --git a/sshconnect2.c b/sshconnect2.c index 93192d186..10e4f0a08 100644 --- a/sshconnect2.c +++ b/sshconnect2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshconnect2.c,v 1.283 2018/07/31 03:07:24 djm Exp $ */ +/* $OpenBSD: sshconnect2.c,v 1.284 2018/08/13 02:41:05 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2008 Damien Miller. All rights reserved. @@ -167,11 +167,11 @@ ssh_kex2(char *host, struct sockaddr *hostaddr, u_short port) if ((s = kex_names_cat(options.kex_algorithms, "ext-info-c")) == NULL) fatal("%s: kex_names_cat", __func__); - myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal(s, datafellows); + myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal(s); myproposal[PROPOSAL_ENC_ALGS_CTOS] = - compat_cipher_proposal(options.ciphers, datafellows); + compat_cipher_proposal(options.ciphers); myproposal[PROPOSAL_ENC_ALGS_STOC] = - compat_cipher_proposal(options.ciphers, datafellows); + compat_cipher_proposal(options.ciphers); myproposal[PROPOSAL_COMP_ALGS_CTOS] = myproposal[PROPOSAL_COMP_ALGS_STOC] = options.compression ? "zlib@openssh.com,zlib,none" : "none,zlib@openssh.com,zlib"; @@ -184,15 +184,14 @@ ssh_kex2(char *host, struct sockaddr *hostaddr, u_short port) fatal("%s: kex_assemble_namelist", __func__); free(all_key); myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = - compat_pkalg_proposal(options.hostkeyalgorithms, - datafellows); + compat_pkalg_proposal(options.hostkeyalgorithms); } else { /* Enforce default */ options.hostkeyalgorithms = xstrdup(KEX_DEFAULT_PK_ALG); /* Prefer algorithms that we already have keys for */ myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = compat_pkalg_proposal( - order_hostkeyalgs(host, hostaddr, port), datafellows); + order_hostkeyalgs(host, hostaddr, port)); } if (options.rekey_limit || options.rekey_interval) @@ -224,7 +223,7 @@ ssh_kex2(char *host, struct sockaddr *hostaddr, u_short port) /* remove ext-info from the KEX proposals for rekeying */ myproposal[PROPOSAL_KEX_ALGS] = - compat_kex_proposal(options.kex_algorithms, datafellows); + compat_kex_proposal(options.kex_algorithms); if ((r = kex_prop2buf(kex->my, myproposal)) != 0) fatal("kex_prop2buf: %s", ssh_err(r)); diff --git a/sshd.c b/sshd.c index d3bd8fdaa..a738c3ab6 100644 --- a/sshd.c +++ b/sshd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshd.c,v 1.513 2018/07/31 03:07:24 djm Exp $ */ +/* $OpenBSD: sshd.c,v 1.514 2018/08/13 02:41:05 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -2268,11 +2268,11 @@ do_ssh2_kex(void) int r; myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal( - options.kex_algorithms, datafellows); + options.kex_algorithms); myproposal[PROPOSAL_ENC_ALGS_CTOS] = compat_cipher_proposal( - options.ciphers, datafellows); + options.ciphers); myproposal[PROPOSAL_ENC_ALGS_STOC] = compat_cipher_proposal( - options.ciphers, datafellows); + options.ciphers); myproposal[PROPOSAL_MAC_ALGS_CTOS] = myproposal[PROPOSAL_MAC_ALGS_STOC] = options.macs; @@ -2286,7 +2286,7 @@ do_ssh2_kex(void) options.rekey_interval); myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = compat_pkalg_proposal( - list_hostkey_types(), datafellows); + list_hostkey_types()); /* start key exchange */ if ((r = kex_setup(active_state, myproposal)) != 0)