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
This commit is contained in:
djm@openbsd.org 2018-08-13 02:41:05 +00:00 committed by Damien Miller
parent 1b9dd4aa15
commit c3903c38b0
4 changed files with 34 additions and 56 deletions

View File

@ -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. * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl. All rights reserved.
* *
@ -184,17 +184,13 @@ proto_spec(const char *spec)
} }
char * char *
compat_cipher_proposal(char *cipher_prop, u_int compat) compat_cipher_proposal(char *cipher_prop)
{ {
char *cp; if (!(datafellows & SSH_BUG_BIGENDIANAES))
if (!(compat & SSH_BUG_BIGENDIANAES))
return cipher_prop; return cipher_prop;
debug2("%s: original cipher proposal: %s", __func__, 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"); fatal("match_filter_blacklist failed");
free(cipher_prop);
cipher_prop = cp;
debug2("%s: compat cipher proposal: %s", __func__, cipher_prop); debug2("%s: compat cipher proposal: %s", __func__, cipher_prop);
if (*cipher_prop == '\0') if (*cipher_prop == '\0')
fatal("No supported ciphers found"); fatal("No supported ciphers found");
@ -202,17 +198,13 @@ compat_cipher_proposal(char *cipher_prop, u_int compat)
} }
char * char *
compat_pkalg_proposal(char *pkalg_prop, u_int compat) compat_pkalg_proposal(char *pkalg_prop)
{ {
char *cp; if (!(datafellows & SSH_BUG_RSASIGMD5))
if (!(compat & SSH_BUG_RSASIGMD5))
return pkalg_prop; return pkalg_prop;
debug2("%s: original public key proposal: %s", __func__, 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"); fatal("match_filter_blacklist failed");
free(pkalg_prop);
pkalg_prop = cp;
debug2("%s: compat public key proposal: %s", __func__, pkalg_prop); debug2("%s: compat public key proposal: %s", __func__, pkalg_prop);
if (*pkalg_prop == '\0') if (*pkalg_prop == '\0')
fatal("No supported PK algorithms found"); fatal("No supported PK algorithms found");
@ -220,31 +212,24 @@ compat_pkalg_proposal(char *pkalg_prop, u_int compat)
} }
char * char *
compat_kex_proposal(char *kex_prop, u_int compat) compat_kex_proposal(char *p)
{ {
char *cp; if ((datafellows & (SSH_BUG_CURVE25519PAD|SSH_OLD_DHGEX)) == 0)
return p;
if ((compat & (SSH_BUG_CURVE25519PAD|SSH_OLD_DHGEX)) == 0) debug2("%s: original KEX proposal: %s", __func__, p);
return kex_prop; if ((datafellows & SSH_BUG_CURVE25519PAD) != 0)
debug2("%s: original KEX proposal: %s", __func__, kex_prop); if ((p = match_filter_blacklist(p,
if ((compat & SSH_BUG_CURVE25519PAD) != 0) {
if ((cp = match_filter_blacklist(kex_prop,
"curve25519-sha256@libssh.org")) == NULL) "curve25519-sha256@libssh.org")) == NULL)
fatal("match_filter_blacklist failed"); fatal("match_filter_blacklist failed");
free(kex_prop); if ((datafellows & SSH_OLD_DHGEX) != 0) {
kex_prop = cp; if ((p = match_filter_blacklist(p,
}
if ((compat & SSH_OLD_DHGEX) != 0) {
if ((cp = match_filter_blacklist(kex_prop,
"diffie-hellman-group-exchange-sha256," "diffie-hellman-group-exchange-sha256,"
"diffie-hellman-group-exchange-sha1")) == NULL) "diffie-hellman-group-exchange-sha1")) == NULL)
fatal("match_filter_blacklist failed"); fatal("match_filter_blacklist failed");
free(kex_prop);
kex_prop = cp;
} }
debug2("%s: compat KEX proposal: %s", __func__, kex_prop); debug2("%s: compat KEX proposal: %s", __func__, p);
if (*kex_prop == '\0') if (*p == '\0')
fatal("No supported key exchange algorithms found"); fatal("No supported key exchange algorithms found");
return kex_prop; return p;
} }

View File

@ -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. * Copyright (c) 1999, 2000, 2001 Markus Friedl. All rights reserved.
@ -65,15 +65,9 @@
u_int compat_datafellows(const char *); u_int compat_datafellows(const char *);
int proto_spec(const char *); int proto_spec(const char *);
char *compat_cipher_proposal(char *);
/* char *compat_pkalg_proposal(char *);
* compat_*_proposal will update their respective proposals based on the char *compat_kex_proposal(char *);
* 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);
extern int datafellows; extern int datafellows;
#endif #endif

View File

@ -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) 2000 Markus Friedl. All rights reserved.
* Copyright (c) 2008 Damien Miller. 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) if ((s = kex_names_cat(options.kex_algorithms, "ext-info-c")) == NULL)
fatal("%s: kex_names_cat", __func__); 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] = myproposal[PROPOSAL_ENC_ALGS_CTOS] =
compat_cipher_proposal(options.ciphers, datafellows); compat_cipher_proposal(options.ciphers);
myproposal[PROPOSAL_ENC_ALGS_STOC] = 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_CTOS] =
myproposal[PROPOSAL_COMP_ALGS_STOC] = options.compression ? myproposal[PROPOSAL_COMP_ALGS_STOC] = options.compression ?
"zlib@openssh.com,zlib,none" : "none,zlib@openssh.com,zlib"; "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__); fatal("%s: kex_assemble_namelist", __func__);
free(all_key); free(all_key);
myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] =
compat_pkalg_proposal(options.hostkeyalgorithms, compat_pkalg_proposal(options.hostkeyalgorithms);
datafellows);
} else { } else {
/* Enforce default */ /* Enforce default */
options.hostkeyalgorithms = xstrdup(KEX_DEFAULT_PK_ALG); options.hostkeyalgorithms = xstrdup(KEX_DEFAULT_PK_ALG);
/* Prefer algorithms that we already have keys for */ /* Prefer algorithms that we already have keys for */
myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] =
compat_pkalg_proposal( compat_pkalg_proposal(
order_hostkeyalgs(host, hostaddr, port), datafellows); order_hostkeyalgs(host, hostaddr, port));
} }
if (options.rekey_limit || options.rekey_interval) 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 */ /* remove ext-info from the KEX proposals for rekeying */
myproposal[PROPOSAL_KEX_ALGS] = 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) if ((r = kex_prop2buf(kex->my, myproposal)) != 0)
fatal("kex_prop2buf: %s", ssh_err(r)); fatal("kex_prop2buf: %s", ssh_err(r));

10
sshd.c
View File

@ -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 <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -2268,11 +2268,11 @@ do_ssh2_kex(void)
int r; int r;
myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal( myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal(
options.kex_algorithms, datafellows); options.kex_algorithms);
myproposal[PROPOSAL_ENC_ALGS_CTOS] = compat_cipher_proposal( myproposal[PROPOSAL_ENC_ALGS_CTOS] = compat_cipher_proposal(
options.ciphers, datafellows); options.ciphers);
myproposal[PROPOSAL_ENC_ALGS_STOC] = compat_cipher_proposal( myproposal[PROPOSAL_ENC_ALGS_STOC] = compat_cipher_proposal(
options.ciphers, datafellows); options.ciphers);
myproposal[PROPOSAL_MAC_ALGS_CTOS] = myproposal[PROPOSAL_MAC_ALGS_CTOS] =
myproposal[PROPOSAL_MAC_ALGS_STOC] = options.macs; myproposal[PROPOSAL_MAC_ALGS_STOC] = options.macs;
@ -2286,7 +2286,7 @@ do_ssh2_kex(void)
options.rekey_interval); options.rekey_interval);
myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = compat_pkalg_proposal( myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = compat_pkalg_proposal(
list_hostkey_types(), datafellows); list_hostkey_types());
/* start key exchange */ /* start key exchange */
if ((r = kex_setup(active_state, myproposal)) != 0) if ((r = kex_setup(active_state, myproposal)) != 0)