upstream commit

fix signed/unsigned errors reported by clang-3.7; add
 sshbuf_dup_string() to replace a common idiom of strdup(sshbuf_ptr()) with
 better safety checking; feedback and ok markus@

Upstream-ID: 71f926d9bb3f1efed51319a6daf37e93d57c8820

diff --git a/auth2-chall.c b/auth2-chall.c
index 4aff09d80595b3659524fe4726e58f9a7165ef17..ead48031808d650f7ed1817bacf37287adc2bd58 100644 (file)
--- a/auth2-chall.c
+++ b/auth2-chall.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2-chall.c,v 1.43 2015/07/18 07:57:14 djm Exp $ */
+/* $OpenBSD: auth2-chall.c,v 1.44 2016/05/02 08:49:03 djm Exp $ */
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2001 Per Allansson.  All rights reserved.
@@ -122,8 +122,8 @@ kbdint_alloc(const char *devs)
                        buffer_append(&b, devices[i]->name,
                            strlen(devices[i]->name));
                }
-               buffer_append(&b, "\0", 1);
-               kbdintctxt->devices = xstrdup(buffer_ptr(&b));
+               if ((kbdintctxt->devices = sshbuf_dup_string(&b)) == NULL)
+                       fatal("%s: sshbuf_dup_string failed", __func__);
                buffer_free(&b);
        } else {
                kbdintctxt->devices = xstrdup(devs);

diff --git a/auth2.c b/auth2.c
index 717796228a862b440ae59541818115d7877ccc44..9108b8612bea215c3f22a7c275803e5760b922a4 100644 (file)
--- a/auth2.c
+++ b/auth2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2.c,v 1.135 2015/01/19 20:07:45 markus Exp $ */
+/* $OpenBSD: auth2.c,v 1.136 2016/05/02 08:49:03 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -424,8 +424,8 @@ authmethods_get(Authctxt *authctxt)
                buffer_append(&b, authmethods[i]->name,
                    strlen(authmethods[i]->name));
        }
-       buffer_append(&b, "\0", 1);
-       list = xstrdup(buffer_ptr(&b));
+       if ((list = sshbuf_dup_string(&b)) == NULL)
+               fatal("%s: sshbuf_dup_string failed", __func__);
        buffer_free(&b);
        return list;
 }

diff --git a/kex.h b/kex.h
index 1c589660505941fad416b89fb3e2e356de4d3185..131b8d93d3058057c2253343e4c014be076f9a79 100644 (file)
--- a/kex.h
+++ b/kex.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: kex.h,v 1.76 2016/02/08 10:57:07 djm Exp $ */
+/* $OpenBSD: kex.h,v 1.77 2016/05/02 08:49:03 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -205,8 +205,9 @@ int kex_ecdh_hash(int, const EC_GROUP *, const char *, const char *,
     const u_char *, size_t, const u_char *, size_t, const u_char *, size_t,
     const EC_POINT *, const EC_POINT *, const BIGNUM *, u_char *, size_t *);
 
-int     kex_c25519_hash(int, const char *, const char *, const char *, size_t,
-    const char *, size_t, const u_char *, size_t, const u_char *, const u_char *,
+int     kex_c25519_hash(int, const char *, const char *,
+    const u_char *, size_t, const u_char *, size_t,
+    const u_char *, size_t, const u_char *, const u_char *,
     const u_char *, size_t, u_char *, size_t *);
 
 void   kexc25519_keygen(u_char key[CURVE25519_SIZE], u_char pub[CURVE25519_SIZE])

diff --git a/kexc25519.c b/kexc25519.c
index 8d8cd4a2bae953dde7931dbc7429b84524a3a718..0897b8c5190ac1aa0a75bfa4365a7459f57b5ff7 100644 (file)
--- a/kexc25519.c
+++ b/kexc25519.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kexc25519.c,v 1.9 2015/03/26 07:00:04 djm Exp $ */
+/* $OpenBSD: kexc25519.c,v 1.10 2016/05/02 08:49:03 djm Exp $ */
 /*
  * Copyright (c) 2001, 2013 Markus Friedl.  All rights reserved.
  * Copyright (c) 2010 Damien Miller.  All rights reserved.
@@ -86,8 +86,8 @@ kex_c25519_hash(
     int hash_alg,
     const char *client_version_string,
     const char *server_version_string,
-    const char *ckexinit, size_t ckexinitlen,
-    const char *skexinit, size_t skexinitlen,
+    const u_char *ckexinit, size_t ckexinitlen,
+    const u_char *skexinit, size_t skexinitlen,
     const u_char *serverhostkeyblob, size_t sbloblen,
     const u_char client_dh_pub[CURVE25519_SIZE],
     const u_char server_dh_pub[CURVE25519_SIZE],

diff --git a/monitor.c b/monitor.c
index 6b780e48020bbfdf5aa95685ac8ba9c072781b0b..dce920c23c28334fbbde04ad55e3315a762d3808 100644 (file)
--- a/monitor.c
+++ b/monitor.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor.c,v 1.158 2016/03/07 19:02:43 djm Exp $ */
+/* $OpenBSD: monitor.c,v 1.159 2016/05/02 08:49:03 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -34,6 +34,7 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #ifdef HAVE_PATHS_H
 #include <paths.h>
 #endif
@@ -688,7 +689,8 @@ mm_answer_sign(int sock, Buffer *m)
        u_char *p = NULL, *signature = NULL;
        char *alg = NULL;
        size_t datlen, siglen, alglen;
-       int r, keyid, is_proof = 0;
+       int r, is_proof = 0;
+       u_int keyid;
        const char proof_req[] = "hostkeys-prove-00@openssh.com";
 
        debug3("%s", __func__);
@@ -697,6 +699,8 @@ mm_answer_sign(int sock, Buffer *m)
            (r = sshbuf_get_string(m, &p, &datlen)) != 0 ||
            (r = sshbuf_get_cstring(m, &alg, &alglen)) != 0)
                fatal("%s: buffer error: %s", __func__, ssh_err(r));
+       if (keyid > INT_MAX)
+               fatal("%s: invalid key ID", __func__);
 
        /*
         * Supported KEX types use SHA1 (20 bytes), SHA256 (32 bytes),
@@ -1289,7 +1293,8 @@ static int
 monitor_valid_userblob(u_char *data, u_int datalen)
 {
        Buffer b;
-       char *p, *userstyle;
+       u_char *p;
+       char *userstyle, *cp;
        u_int len;
        int fail = 0;
 
@@ -1314,26 +1319,26 @@ monitor_valid_userblob(u_char *data, u_int datalen)
        }
        if (buffer_get_char(&b) != SSH2_MSG_USERAUTH_REQUEST)
                fail++;
-       p = buffer_get_cstring(&b, NULL);
+       cp = buffer_get_cstring(&b, NULL);
        xasprintf(&userstyle, "%s%s%s", authctxt->user,
            authctxt->style ? ":" : "",
            authctxt->style ? authctxt->style : "");
-       if (strcmp(userstyle, p) != 0) {
-               logit("wrong user name passed to monitor: expected %s != %.100s",
-                   userstyle, p);
+       if (strcmp(userstyle, cp) != 0) {
+               logit("wrong user name passed to monitor: "
+                   "expected %s != %.100s", userstyle, cp);
                fail++;
        }
        free(userstyle);
-       free(p);
+       free(cp);
        buffer_skip_string(&b);
        if (datafellows & SSH_BUG_PKAUTH) {
                if (!buffer_get_char(&b))
                        fail++;
        } else {
-               p = buffer_get_cstring(&b, NULL);
-               if (strcmp("publickey", p) != 0)
+               cp = buffer_get_cstring(&b, NULL);
+               if (strcmp("publickey", cp) != 0)
                        fail++;
-               free(p);
+               free(cp);
                if (!buffer_get_char(&b))
                        fail++;
                buffer_skip_string(&b);

diff --git a/servconf.c b/servconf.c
index ba39dce1dffe894cdbd4faf7617c8f1e71f312cd..6111c5a94ff00e84456f81ae38f877a06df2e257 100644 (file)
--- a/servconf.c
+++ b/servconf.c
@@ -1,5 +1,5 @@
 
-/* $OpenBSD: servconf.c,v 1.286 2016/03/07 19:02:43 djm Exp $ */
+/* $OpenBSD: servconf.c,v 1.287 2016/05/02 08:49:03 djm Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -2059,7 +2059,8 @@ parse_server_config(ServerOptions *options, const char *filename, Buffer *conf,
 
        debug2("%s: config %s len %d", __func__, filename, buffer_len(conf));
 
-       obuf = cbuf = xstrdup(buffer_ptr(conf));
+       if ((obuf = cbuf = sshbuf_dup_string(conf)) == NULL)
+               fatal("%s: sshbuf_dup_string failed", __func__);
        active = connectinfo ? 0 : 1;
        linenum = 1;
        while ((cp = strsep(&cbuf, "\n")) != NULL) {

diff --git a/sftp-client.c b/sftp-client.c
index cd990579e93ad6e2c2a75834877281449b1b291a..faf14684ca47c3d8875d2e43d9fb5237718a7ac5 100644 (file)
--- a/sftp-client.c
+++ b/sftp-client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp-client.c,v 1.122 2016/04/08 08:19:17 djm Exp $ */
+/* $OpenBSD: sftp-client.c,v 1.123 2016/05/02 08:49:03 djm Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
  *
@@ -515,8 +515,7 @@ do_lsreaddir(struct sftp_conn *conn, const char *path, int print_flag,
        struct sshbuf *msg;
        u_int count, id, i, expected_id, ents = 0;
        size_t handle_len;
-       u_char type;
-       char *handle;
+       u_char type, *handle;
        int status = SSH2_FX_FAILURE;
        int r;
 

diff --git a/ssh-agent.c b/ssh-agent.c
index c38906d94278e2388cabb621472b01d7ca88a00e..8aa25b30d22f6856150c4bdf40e8612a07ce9b65 100644 (file)
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-agent.c,v 1.212 2016/02/15 09:47:49 dtucker Exp $ */
+/* $OpenBSD: ssh-agent.c,v 1.213 2016/05/02 08:49:03 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -144,8 +144,8 @@ char socket_dir[PATH_MAX];
 #define LOCK_SALT_SIZE 16
 #define LOCK_ROUNDS    1
 int locked = 0;
-char lock_passwd[LOCK_SIZE];
-char lock_salt[LOCK_SALT_SIZE];
+u_char lock_pwhash[LOCK_SIZE];
+u_char lock_salt[LOCK_SALT_SIZE];
 
 extern char *__progname;
 
@@ -677,7 +677,8 @@ static void
 process_lock_agent(SocketEntry *e, int lock)
 {
        int r, success = 0, delay;
-       char *passwd, passwdhash[LOCK_SIZE];
+       char *passwd;
+       u_char passwdhash[LOCK_SIZE];
        static u_int fail_count = 0;
        size_t pwlen;
 
@@ -689,11 +690,11 @@ process_lock_agent(SocketEntry *e, int lock)
                if (bcrypt_pbkdf(passwd, pwlen, lock_salt, sizeof(lock_salt),
                    passwdhash, sizeof(passwdhash), LOCK_ROUNDS) < 0)
                        fatal("bcrypt_pbkdf");
-               if (timingsafe_bcmp(passwdhash, lock_passwd, LOCK_SIZE) == 0) {
+               if (timingsafe_bcmp(passwdhash, lock_pwhash, LOCK_SIZE) == 0) {
                        debug("agent unlocked");
                        locked = 0;
                        fail_count = 0;
-                       explicit_bzero(lock_passwd, sizeof(lock_passwd));
+                       explicit_bzero(lock_pwhash, sizeof(lock_pwhash));
                        success = 1;
                } else {
                        /* delay in 0.1s increments up to 10s */
@@ -710,7 +711,7 @@ process_lock_agent(SocketEntry *e, int lock)
                locked = 1;
                arc4random_buf(lock_salt, sizeof(lock_salt));
                if (bcrypt_pbkdf(passwd, pwlen, lock_salt, sizeof(lock_salt),
-                   lock_passwd, sizeof(lock_passwd), LOCK_ROUNDS) < 0)
+                   lock_pwhash, sizeof(lock_pwhash), LOCK_ROUNDS) < 0)
                        fatal("bcrypt_pbkdf");
                success = 1;
        }

diff --git a/ssh-keygen.c b/ssh-keygen.c
index 478520123e10b2c9f278751f16776ad828285cea..079f10321b1abec381bd412ecdd3bb649bc6c778 100644 (file)
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.288 2016/02/15 09:47:49 dtucker Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.289 2016/05/02 08:49:03 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -883,7 +883,7 @@ do_fingerprint(struct passwd *pw)
        char *comment = NULL, *cp, *ep, line[SSH_MAX_PUBKEY_BYTES];
        int i, invalid = 1;
        const char *path;
-       long int lnum = 0;
+       u_long lnum = 0;
 
        if (!have_identity)
                ask_filename(pw, "Enter file in which the key is");
@@ -946,7 +946,7 @@ do_fingerprint(struct passwd *pw)
                }
                /* Retry after parsing leading hostname/key options */
                if (public == NULL && (public = try_read_key(&cp)) == NULL) {
-                       debug("%s:%ld: not a public key", path, lnum);
+                       debug("%s:%lu: not a public key", path, lnum);
                        continue;
                }
 
@@ -1920,7 +1920,7 @@ do_show_cert(struct passwd *pw)
        FILE *f;
        char *cp, line[SSH_MAX_PUBKEY_BYTES];
        const char *path;
-       long int lnum = 0;
+       u_long lnum = 0;
 
        if (!have_identity)
                ask_filename(pw, "Enter file in which the key is");

diff --git a/sshbuf-misc.c b/sshbuf-misc.c
index 3da4b80e765880b0065f1d181af2dc7d62467232..15dcfbc791966005d4e196daba0d57096610fd93 100644 (file)
--- a/sshbuf-misc.c
+++ b/sshbuf-misc.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: sshbuf-misc.c,v 1.5 2015/10/05 17:11:21 djm Exp $     */
+/*     $OpenBSD: sshbuf-misc.c,v 1.6 2016/05/02 08:49:03 djm Exp $     */
 /*
  * Copyright (c) 2011 Damien Miller
  *
@@ -136,3 +136,26 @@ sshbuf_b64tod(struct sshbuf *buf, const char *b64)
        return 0;
 }
 
+char *
+sshbuf_dup_string(struct sshbuf *buf)
+{
+       const u_char *p = NULL, *s = sshbuf_ptr(buf);
+       size_t l = sshbuf_len(buf);
+       char *r;
+
+       if (s == NULL || l > SIZE_MAX)
+               return NULL;
+       /* accept a nul only as the last character in the buffer */
+       if (l > 0 && (p = memchr(s, '\0', l)) != NULL) {
+               if (p != s + l - 1)
+                       return NULL;
+               l--; /* the nul is put back below */
+       }
+       if ((r = malloc(l + 1)) == NULL)
+               return NULL;
+       if (l > 0)
+               memcpy(r, s, l);
+       r[l] = '\0';
+       return r;
+}
+

diff --git a/sshbuf.h b/sshbuf.h
index 63495fbb075a8b9e3df747871b6e01d2b554b080..52ff017cc5c97385891fadc6172b97865881b678 100644 (file)
--- a/sshbuf.h
+++ b/sshbuf.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: sshbuf.h,v 1.6 2015/12/10 07:01:35 mmcc Exp $ */
+/*     $OpenBSD: sshbuf.h,v 1.7 2016/05/02 08:49:03 djm Exp $  */
 /*
  * Copyright (c) 2011 Damien Miller
  *
@@ -239,6 +239,13 @@ char       *sshbuf_dtob64(struct sshbuf *buf);
 /* Decode base64 data and append it to the buffer */
 int    sshbuf_b64tod(struct sshbuf *buf, const char *b64);
 
+/*
+ * Duplicate the contents of a buffer to a string (caller to free).
+ * Returns NULL on buffer error, or if the buffer contains a premature
+ * nul character.
+ */
+char *sshbuf_dup_string(struct sshbuf *buf);
+
 /* Macros for decoding/encoding integers */
 #define PEEK_U64(p) \
        (((u_int64_t)(((const u_char *)(p))[0]) << 56) | \

diff --git a/sshconnect2.c b/sshconnect2.c
index f7d0644e8aaae884dc459b180b3526ae1c76339e..1dddf75aac199483b1df30f2e960553a19883615 100644 (file)
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshconnect2.c,v 1.241 2016/04/28 14:30:21 djm Exp $ */
+/* $OpenBSD: sshconnect2.c,v 1.242 2016/05/02 08:49:03 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Damien Miller.  All rights reserved.
@@ -1922,8 +1922,8 @@ authmethods_get(void)
                        buffer_append(&b, method->name, strlen(method->name));
                }
        }
-       buffer_append(&b, "\0", 1);
-       list = xstrdup(buffer_ptr(&b));
+       if ((list = sshbuf_dup_string(&b)) == NULL)
+               fatal("%s: sshbuf_dup_string failed", __func__);
        buffer_free(&b);
        return list;
 }

diff --git a/sshd.c b/sshd.c
index d21aed5156bf298f549d8afefc10646a1943311a..8b8af2494daaefcc95c8a548d6a18cc556fda775 100644 (file)
--- a/sshd.c
+++ b/sshd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.466 2016/03/07 19:02:43 djm Exp $ */
+/* $OpenBSD: sshd.c,v 1.467 2016/05/02 08:49:03 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -845,8 +845,8 @@ list_hostkey_types(void)
                        break;
                }
        }
-       buffer_append(&b, "\0", 1);
-       ret = xstrdup(buffer_ptr(&b));
+       if ((ret = sshbuf_dup_string(&b)) == NULL)
+               fatal("%s: sshbuf_dup_string failed", __func__);
        buffer_free(&b);
        debug("list_hostkey_types: %s", ret);
        return ret;
@@ -1027,12 +1027,13 @@ usage(void)
 }
 
 static void
-send_rexec_state(int fd, Buffer *conf)
+send_rexec_state(int fd, struct sshbuf *conf)
 {
-       Buffer m;
+       struct sshbuf *m;
+       int r;
 
-       debug3("%s: entering fd = %d config len %d", __func__, fd,
-           buffer_len(conf));
+       debug3("%s: entering fd = %d config len %zu", __func__, fd,
+           sshbuf_len(conf));
 
        /*
         * Protocol from reexec master to child:
@@ -1046,31 +1047,41 @@ send_rexec_state(int fd, Buffer *conf)
         *      bignum  q                       "
         *      string rngseed          (only if OpenSSL is not self-seeded)
         */
-       buffer_init(&m);
-       buffer_put_cstring(&m, buffer_ptr(conf));
+       if ((m = sshbuf_new()) == NULL)
+               fatal("%s: sshbuf_new failed", __func__);
+       if ((r = sshbuf_put_stringb(m, conf)) != 0)
+               fatal("%s: buffer error: %s", __func__, ssh_err(r));
 
 #ifdef WITH_SSH1
        if (sensitive_data.server_key != NULL &&
            sensitive_data.server_key->type == KEY_RSA1) {
-               buffer_put_int(&m, 1);
-               buffer_put_bignum(&m, sensitive_data.server_key->rsa->e);
-               buffer_put_bignum(&m, sensitive_data.server_key->rsa->n);
-               buffer_put_bignum(&m, sensitive_data.server_key->rsa->d);
-               buffer_put_bignum(&m, sensitive_data.server_key->rsa->iqmp);
-               buffer_put_bignum(&m, sensitive_data.server_key->rsa->p);
-               buffer_put_bignum(&m, sensitive_data.server_key->rsa->q);
+               if ((r = sshbuf_put_u32(m, 1)) != 0 ||
+                   (r = sshbuf_put_bignum1(m,
+                   sensitive_data.server_key->rsa->e)) != 0 ||
+                   (r = sshbuf_put_bignum1(m,
+                   sensitive_data.server_key->rsa->n)) != 0 ||
+                   (r = sshbuf_put_bignum1(m,
+                   sensitive_data.server_key->rsa->d)) != 0 ||
+                   (r = sshbuf_put_bignum1(m,
+                   sensitive_data.server_key->rsa->iqmp)) != 0 ||
+                   (r = sshbuf_put_bignum1(m,
+                   sensitive_data.server_key->rsa->p)) != 0 ||
+                   (r = sshbuf_put_bignum1(m,
+                   sensitive_data.server_key->rsa->q)) != 0)
+                       fatal("%s: buffer error: %s", __func__, ssh_err(r));
        } else
 #endif
-               buffer_put_int(&m, 0);
+               if ((r = sshbuf_put_u32(m, 1)) != 0)
+                       fatal("%s: buffer error: %s", __func__, ssh_err(r));
 
 #if defined(WITH_OPENSSL) && !defined(OPENSSL_PRNG_ONLY)
-       rexec_send_rng_seed(&m);
+       rexec_send_rng_seed(m);
 #endif
 
-       if (ssh_msg_send(fd, 0, &m) == -1)
+       if (ssh_msg_send(fd, 0, m) == -1)
                fatal("%s: ssh_msg_send failed", __func__);
 
-       buffer_free(&m);
+       sshbuf_free(m);
 
        debug3("%s: done", __func__);
 }