fix some memory leaks spotted by Coverity via Jakub Jelen in bz#2366

feedback and ok dtucker@

diff --git a/usr.bin/ssh/addrmatch.c b/usr.bin/ssh/addrmatch.c
index 452aa4e..ee1f170 100644 (file)
--- a/usr.bin/ssh/addrmatch.c
+++ b/usr.bin/ssh/addrmatch.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: addrmatch.c,v 1.13 2016/09/21 16:55:42 djm Exp $ */
+/*     $OpenBSD: addrmatch.c,v 1.14 2018/07/31 03:07:24 djm Exp $ */
 
 /*
  * Copyright (c) 2004-2008 Damien Miller <djm@mindrot.org>
@@ -201,25 +201,24 @@ addr_cmp(const struct xaddr *a, const struct xaddr *b)
 static int
 addr_pton(const char *p, struct xaddr *n)
 {
-       struct addrinfo hints, *ai;
+       struct addrinfo hints, *ai = NULL;
+       int ret = -1;
 
        memset(&hints, '\0', sizeof(hints));
        hints.ai_flags = AI_NUMERICHOST;
 
        if (p == NULL || getaddrinfo(p, NULL, &hints, &ai) != 0)
-               return -1;
-
+               goto out;
        if (ai == NULL || ai->ai_addr == NULL)
-               return -1;
-
-       if (n != NULL &&
-           addr_sa_to_xaddr(ai->ai_addr, ai->ai_addrlen, n) == -1) {
+               goto out;
+       if (n != NULL && addr_sa_to_xaddr(ai->ai_addr, ai->ai_addrlen, n) == -1)
+               goto out;
+       /* success */
+       ret = 0;
+ out:
+       if (ai != NULL)
                freeaddrinfo(ai);
-               return -1;
-       }
-
-       freeaddrinfo(ai);
-       return 0;
+       return ret;
 }
 
 /*

diff --git a/usr.bin/ssh/compat.c b/usr.bin/ssh/compat.c
index 9347aed..71a7317 100644 (file)
--- a/usr.bin/ssh/compat.c
+++ b/usr.bin/ssh/compat.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: compat.c,v 1.111 2018/07/09 21:03:30 markus Exp $ */
+/* $OpenBSD: compat.c,v 1.112 2018/07/31 03:07:24 djm Exp $ */
 /*
  * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl.  All rights reserved.
  *
@@ -182,13 +182,17 @@ proto_spec(const char *spec)
 }
 
 char *
-compat_cipher_proposal(char *cipher_prop)
+compat_cipher_proposal(char *cipher_prop, u_int compat)
 {
-       if (!(datafellows & SSH_BUG_BIGENDIANAES))
+       char *cp;
+
+       if (!(compat & SSH_BUG_BIGENDIANAES))
                return cipher_prop;
        debug2("%s: original cipher proposal: %s", __func__, cipher_prop);
-       if ((cipher_prop = match_filter_blacklist(cipher_prop, "aes*")) == NULL)
+       if ((cp = 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");
@@ -196,13 +200,17 @@ compat_cipher_proposal(char *cipher_prop)
 }
 
 char *
-compat_pkalg_proposal(char *pkalg_prop)
+compat_pkalg_proposal(char *pkalg_prop, u_int compat)
 {
-       if (!(datafellows & SSH_BUG_RSASIGMD5))
+       char *cp;
+
+       if (!(compat & SSH_BUG_RSASIGMD5))
                return pkalg_prop;
        debug2("%s: original public key proposal: %s", __func__, pkalg_prop);
-       if ((pkalg_prop = match_filter_blacklist(pkalg_prop, "ssh-rsa")) == NULL)
+       if ((cp = 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");
@@ -210,24 +218,31 @@ compat_pkalg_proposal(char *pkalg_prop)
 }
 
 char *
-compat_kex_proposal(char *p)
+compat_kex_proposal(char *kex_prop, u_int compat)
 {
-       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,
+       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,
                    "curve25519-sha256@libssh.org")) == NULL)
                        fatal("match_filter_blacklist failed");
-       if ((datafellows & SSH_OLD_DHGEX) != 0) {
-               if ((p = match_filter_blacklist(p,
+               free(kex_prop);
+               kex_prop = cp;
+       }
+       if ((compat & SSH_OLD_DHGEX) != 0) {
+               if ((cp = match_filter_blacklist(kex_prop,
                    "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__, p);
-       if (*p == '\0')
+       debug2("%s: compat KEX proposal: %s", __func__, kex_prop);
+       if (*kex_prop == '\0')
                fatal("No supported key exchange algorithms found");
-       return p;
+       return kex_prop;
 }
 

diff --git a/usr.bin/ssh/compat.h b/usr.bin/ssh/compat.h
index 28d2c81..e287773 100644 (file)
--- a/usr.bin/ssh/compat.h
+++ b/usr.bin/ssh/compat.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: compat.h,v 1.52 2018/07/03 11:39:54 djm Exp $ */
+/* $OpenBSD: compat.h,v 1.53 2018/07/31 03:07:24 djm Exp $ */
 
 /*
  * Copyright (c) 1999, 2000, 2001 Markus Friedl.  All rights reserved.
@@ -65,9 +65,15 @@
 
 u_int    compat_datafellows(const char *);
 int     proto_spec(const char *);
-char   *compat_cipher_proposal(char *);
-char   *compat_pkalg_proposal(char *);
-char   *compat_kex_proposal(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);
 
 extern int datafellows;
 #endif

diff --git a/usr.bin/ssh/mux.c b/usr.bin/ssh/mux.c
index 91b292c..512cc6e 100644 (file)
--- a/usr.bin/ssh/mux.c
+++ b/usr.bin/ssh/mux.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: mux.c,v 1.74 2018/07/11 18:53:29 markus Exp $ */
+/* $OpenBSD: mux.c,v 1.75 2018/07/31 03:07:24 djm Exp $ */
 /*
  * Copyright (c) 2002-2008 Damien Miller <djm@openbsd.org>
  *
@@ -1029,6 +1029,7 @@ process_mux_stdio_fwd(struct ssh *ssh, u_int rid,
                set_nonblock(new_fd[1]);
 
        nc = channel_connect_stdio_fwd(ssh, chost, cport, new_fd[0], new_fd[1]);
+       free(chost);
 
        nc->ctl_chan = c->self;         /* link session -> control channel */
        c->remote_id = nc->self;        /* link control -> session channel */

diff --git a/usr.bin/ssh/sftp-client.c b/usr.bin/ssh/sftp-client.c
index d66eb48..a7f6353 100644 (file)
--- a/usr.bin/ssh/sftp-client.c
+++ b/usr.bin/ssh/sftp-client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp-client.c,v 1.129 2018/05/25 04:25:46 djm Exp $ */
+/* $OpenBSD: sftp-client.c,v 1.130 2018/07/31 03:07:24 djm Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
  *
@@ -1443,7 +1443,7 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
 {
        int i, ret = 0;
        SFTP_DIRENT **dir_entries;
-       char *filename, *new_src, *new_dst;
+       char *filename, *new_src = NULL, *new_dst = NULL;
        mode_t mode = 0777;
 
        if (depth >= MAX_DIR_DEPTH) {
@@ -1481,8 +1481,10 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
        }
 
        for (i = 0; dir_entries[i] != NULL && !interrupted; i++) {
-               filename = dir_entries[i]->filename;
+               free(new_dst);
+               free(new_src);
 
+               filename = dir_entries[i]->filename;
                new_dst = path_append(dst, filename);
                new_src = path_append(src, filename);
 
@@ -1505,9 +1507,9 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
                } else
                        logit("%s: not a regular file\n", new_src);
 
-               free(new_dst);
-               free(new_src);
        }
+       free(new_dst);
+       free(new_src);
 
        if (preserve_flag) {
                if (dirattrib->flags & SSH2_FILEXFER_ATTR_ACMODTIME) {
@@ -1774,7 +1776,7 @@ upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
        int ret = 0;
        DIR *dirp;
        struct dirent *dp;
-       char *filename, *new_src, *new_dst;
+       char *filename, *new_src = NULL, *new_dst = NULL;
        struct stat sb;
        Attrib a, *dirattrib;
 
@@ -1825,6 +1827,8 @@ upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
        while (((dp = readdir(dirp)) != NULL) && !interrupted) {
                if (dp->d_ino == 0)
                        continue;
+               free(new_dst);
+               free(new_src);
                filename = dp->d_name;
                new_dst = path_append(dst, filename);
                new_src = path_append(src, filename);
@@ -1851,9 +1855,9 @@ upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
                        }
                } else
                        logit("%s: not a regular file\n", filename);
-               free(new_dst);
-               free(new_src);
        }
+       free(new_dst);
+       free(new_src);
 
        do_setstat(conn, dst, &a);
 

diff --git a/usr.bin/ssh/sshconnect2.c b/usr.bin/ssh/sshconnect2.c
index 4e5e95d..c309ab7 100644 (file)
--- a/usr.bin/ssh/sshconnect2.c
+++ b/usr.bin/ssh/sshconnect2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshconnect2.c,v 1.282 2018/07/18 11:34:04 dtucker Exp $ */
+/* $OpenBSD: sshconnect2.c,v 1.283 2018/07/31 03:07:24 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Damien Miller.  All rights reserved.
@@ -161,11 +161,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);
+       myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal(s, datafellows);
        myproposal[PROPOSAL_ENC_ALGS_CTOS] =
-           compat_cipher_proposal(options.ciphers);
+           compat_cipher_proposal(options.ciphers, datafellows);
        myproposal[PROPOSAL_ENC_ALGS_STOC] =
-           compat_cipher_proposal(options.ciphers);
+           compat_cipher_proposal(options.ciphers, datafellows);
        myproposal[PROPOSAL_COMP_ALGS_CTOS] =
            myproposal[PROPOSAL_COMP_ALGS_STOC] = options.compression ?
            "zlib@openssh.com,zlib,none" : "none,zlib@openssh.com,zlib";
@@ -178,14 +178,15 @@ 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);
+                   compat_pkalg_proposal(options.hostkeyalgorithms,
+                   datafellows);
        } 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));
+                   order_hostkeyalgs(host, hostaddr, port), datafellows);
        }
 
        if (options.rekey_limit || options.rekey_interval)
@@ -215,7 +216,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);
+           compat_kex_proposal(options.kex_algorithms, datafellows);
        if ((r = kex_prop2buf(kex->my, myproposal)) != 0)
                fatal("kex_prop2buf: %s", ssh_err(r));
 

diff --git a/usr.bin/ssh/sshd.c b/usr.bin/ssh/sshd.c
index 2719e10..a125f4a 100644 (file)
--- a/usr.bin/ssh/sshd.c
+++ b/usr.bin/ssh/sshd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.512 2018/07/11 18:53:29 markus Exp $ */
+/* $OpenBSD: sshd.c,v 1.513 2018/07/31 03:07:24 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -2082,11 +2082,11 @@ do_ssh2_kex(void)
        int r;
 
        myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal(
-           options.kex_algorithms);
+           options.kex_algorithms, datafellows);
        myproposal[PROPOSAL_ENC_ALGS_CTOS] = compat_cipher_proposal(
-           options.ciphers);
+           options.ciphers, datafellows);
        myproposal[PROPOSAL_ENC_ALGS_STOC] = compat_cipher_proposal(
-           options.ciphers);
+           options.ciphers, datafellows);
        myproposal[PROPOSAL_MAC_ALGS_CTOS] =
            myproposal[PROPOSAL_MAC_ALGS_STOC] = options.macs;
 
@@ -2100,7 +2100,7 @@ do_ssh2_kex(void)
                    options.rekey_interval);
 
        myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = compat_pkalg_proposal(
-           list_hostkey_types());
+           list_hostkey_types(), datafellows);
 
        /* start key exchange */
        if ((r = kex_setup(active_state, myproposal)) != 0)