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

4 files changed, 34 insertions, 56 deletions

diff --git a/compat.c b/compat.c
index 563e1333..0624dc6d 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 e2877737..d611d33e 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 93192d18..10e4f0a0 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 d3bd8fda..a738c3ab 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 <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, 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)