Switch to recallocarray() for a few operations. Both growth and shrinkage

are handled safely, and there also is no need for preallocation dances.
Future changes in this area will be less error prone.
Review and one bug found by markus

diff --git a/usr.bin/ssh/auth2-pubkey.c b/usr.bin/ssh/auth2-pubkey.c
index f1f7f89..a0def36 100644 (file)
--- a/usr.bin/ssh/auth2-pubkey.c
+++ b/usr.bin/ssh/auth2-pubkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2-pubkey.c,v 1.65 2017/05/30 14:29:59 markus Exp $ */
+/* $OpenBSD: auth2-pubkey.c,v 1.66 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -1153,9 +1153,10 @@ auth2_record_userkey(Authctxt *authctxt, struct sshkey *key)
        struct sshkey **tmp;
 
        if (authctxt->nprev_userkeys >= INT_MAX ||
-           (tmp = reallocarray(authctxt->prev_userkeys,
-           authctxt->nprev_userkeys + 1, sizeof(*tmp))) == NULL)
-               fatal("%s: reallocarray failed", __func__);
+           (tmp = recallocarray(authctxt->prev_userkeys,
+           authctxt->nprev_userkeys, authctxt->nprev_userkeys + 1,
+           sizeof(*tmp))) == NULL)
+               fatal("%s: recallocarray failed", __func__);
        authctxt->prev_userkeys = tmp;
        authctxt->prev_userkeys[authctxt->nprev_userkeys] = key;
        authctxt->nprev_userkeys++;

diff --git a/usr.bin/ssh/authfile.c b/usr.bin/ssh/authfile.c
index 080eb80..33f55d9 100644 (file)
--- a/usr.bin/ssh/authfile.c
+++ b/usr.bin/ssh/authfile.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: authfile.c,v 1.125 2017/05/30 08:49:32 markus Exp $ */
+/* $OpenBSD: authfile.c,v 1.126 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * Copyright (c) 2000, 2013 Markus Friedl.  All rights reserved.
  *
@@ -98,25 +98,13 @@ sshkey_load_file(int fd, struct sshbuf *blob)
        u_char buf[1024];
        size_t len;
        struct stat st;
-       int r, dontmax = 0;
+       int r;
 
        if (fstat(fd, &st) < 0)
                return SSH_ERR_SYSTEM_ERROR;
        if ((st.st_mode & (S_IFSOCK|S_IFCHR|S_IFIFO)) == 0 &&
            st.st_size > MAX_KEY_FILE_SIZE)
                return SSH_ERR_INVALID_FORMAT;
-       /*
-        * Pre-allocate the buffer used for the key contents and clamp its
-        * maximum size. This ensures that key contents are never leaked via
-        * implicit realloc() in the sshbuf code.
-        */
-       if ((st.st_mode & S_IFREG) == 0 || st.st_size <= 0) {
-               st.st_size = 64*1024; /* 64k ought to be enough for anybody. :) */
-               dontmax = 1;
-       }
-       if ((r = sshbuf_allocate(blob, st.st_size)) != 0 ||
-           (dontmax && (r = sshbuf_set_max_size(blob, st.st_size)) != 0))
-               return r;
        for (;;) {
                if ((len = atomicio(read, fd, buf, sizeof(buf))) == 0) {
                        if (errno == EPIPE)

diff --git a/usr.bin/ssh/bitmap.c b/usr.bin/ssh/bitmap.c
index 776a01e..01e8f20 100644 (file)
--- a/usr.bin/ssh/bitmap.c
+++ b/usr.bin/ssh/bitmap.c
@@ -85,7 +85,7 @@ reserve(struct bitmap *b, u_int n)
                return -1; /* invalid */
        nlen = (n / BITMAP_BITS) + 1;
        if (b->len < nlen) {
-               if ((tmp = reallocarray(b->d, nlen, BITMAP_BYTES)) == NULL)
+               if ((tmp = recallocarray(b->d, b->len, nlen, BITMAP_BYTES)) == NULL)
                        return -1;
                b->d = tmp;
                memset(b->d + b->len, 0, (nlen - b->len) * BITMAP_BYTES);

diff --git a/usr.bin/ssh/clientloop.c b/usr.bin/ssh/clientloop.c
index c0c1b35..b1ed83f 100644 (file)
--- a/usr.bin/ssh/clientloop.c
+++ b/usr.bin/ssh/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.298 2017/05/31 07:00:13 markus Exp $ */
+/* $OpenBSD: clientloop.c,v 1.299 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1797,9 +1797,9 @@ hostkeys_find(struct hostkey_foreach_line *l, void *_ctx)
        /* This line contained a key that not offered by the server */
        debug3("%s: deprecated %s key at %s:%ld", __func__,
            sshkey_ssh_name(l->key), l->path, l->linenum);
-       if ((tmp = reallocarray(ctx->old_keys, ctx->nold + 1,
+       if ((tmp = recallocarray(ctx->old_keys, ctx->nold, ctx->nold + 1,
            sizeof(*ctx->old_keys))) == NULL)
-               fatal("%s: reallocarray failed nold = %zu",
+               fatal("%s: recallocarray failed nold = %zu",
                    __func__, ctx->nold);
        ctx->old_keys = tmp;
        ctx->old_keys[ctx->nold++] = l->key;
@@ -2031,9 +2031,9 @@ client_input_hostkeys(void)
                        }
                }
                /* Key is good, record it */
-               if ((tmp = reallocarray(ctx->keys, ctx->nkeys + 1,
+               if ((tmp = recallocarray(ctx->keys, ctx->nkeys, ctx->nkeys + 1,
                    sizeof(*ctx->keys))) == NULL)
-                       fatal("%s: reallocarray failed nkeys = %zu",
+                       fatal("%s: recallocarray failed nkeys = %zu",
                            __func__, ctx->nkeys);
                ctx->keys = tmp;
                ctx->keys[ctx->nkeys++] = key;

diff --git a/usr.bin/ssh/hostfile.c b/usr.bin/ssh/hostfile.c
index e8c1e31..18fa396 100644 (file)
--- a/usr.bin/ssh/hostfile.c
+++ b/usr.bin/ssh/hostfile.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: hostfile.c,v 1.70 2017/04/30 23:18:44 djm Exp $ */
+/* $OpenBSD: hostfile.c,v 1.71 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -248,7 +248,7 @@ record_hostkey(struct hostkey_foreach_line *l, void *_ctx)
            l->marker == MRK_NONE ? "" :
            (l->marker == MRK_CA ? "ca " : "revoked "),
            sshkey_type(l->key), l->path, l->linenum);
-       if ((tmp = reallocarray(hostkeys->entries,
+       if ((tmp = recallocarray(hostkeys->entries, hostkeys->num_entries,
            hostkeys->num_entries + 1, sizeof(*hostkeys->entries))) == NULL)
                return SSH_ERR_ALLOC_FAIL;
        hostkeys->entries = tmp;

diff --git a/usr.bin/ssh/krl.c b/usr.bin/ssh/krl.c
index 736858d..cb7f728 100644 (file)
--- a/usr.bin/ssh/krl.c
+++ b/usr.bin/ssh/krl.c
@@ -14,7 +14,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $OpenBSD: krl.c,v 1.39 2017/03/10 07:18:32 dtucker Exp $ */
+/* $OpenBSD: krl.c,v 1.40 2017/05/31 09:15:42 deraadt Exp $ */
 
 #include <sys/types.h>
 #include <sys/tree.h>
@@ -1024,7 +1024,7 @@ ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp,
                        }
                }
                /* Record keys used to sign the KRL */
-               tmp_ca_used = reallocarray(ca_used, nca_used + 1,
+               tmp_ca_used = recallocarray(ca_used, nca_used, nca_used + 1,
                    sizeof(*ca_used));
                if (tmp_ca_used == NULL) {
                        r = SSH_ERR_ALLOC_FAIL;

diff --git a/usr.bin/ssh/misc.c b/usr.bin/ssh/misc.c
index 17fdcc6..ce6cb34 100644 (file)
--- a/usr.bin/ssh/misc.c
+++ b/usr.bin/ssh/misc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: misc.c,v 1.109 2017/03/14 00:55:37 dtucker Exp $ */
+/* $OpenBSD: misc.c,v 1.110 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2005,2006 Damien Miller.  All rights reserved.
@@ -522,7 +522,7 @@ addargs(arglist *args, char *fmt, ...)
        } else if (args->num+2 >= nalloc)
                nalloc *= 2;
 
-       args->list = xreallocarray(args->list, nalloc, sizeof(char *));
+       args->list = xrecallocarray(args->list, args->nalloc, nalloc, sizeof(char *));
        args->nalloc = nalloc;
        args->list[args->num++] = cp;
        args->list[args->num] = NULL;

diff --git a/usr.bin/ssh/scp.c b/usr.bin/ssh/scp.c
index 811fdb5..31cb02e 100644 (file)
--- a/usr.bin/ssh/scp.c
+++ b/usr.bin/ssh/scp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: scp.c,v 1.191 2017/05/02 08:06:33 jmc Exp $ */
+/* $OpenBSD: scp.c,v 1.192 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * scp - secure remote copy.  This is basically patched BSD rcp which
  * uses ssh to do the data transfer (instead of using rcmd).
@@ -1333,11 +1333,7 @@ allocbuf(BUF *bp, int fd, int blksize)
                size = blksize;
        if (bp->cnt >= size)
                return (bp);
-       if (bp->buf == NULL)
-               bp->buf = xmalloc(size);
-       else
-               bp->buf = xreallocarray(bp->buf, 1, size);
-       memset(bp->buf, 0, size);
+       bp->buf = xrecallocarray(bp->buf, bp->cnt, size, 1);
        bp->cnt = size;
        return (bp);
 }

diff --git a/usr.bin/ssh/session.c b/usr.bin/ssh/session.c
index 175edb3..b924901 100644 (file)
--- a/usr.bin/ssh/session.c
+++ b/usr.bin/ssh/session.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: session.c,v 1.287 2017/05/31 08:09:45 markus Exp $ */
+/* $OpenBSD: session.c,v 1.288 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -1348,8 +1348,8 @@ session_new(void)
                        return NULL;
                debug2("%s: allocate (allocated %d max %d)",
                    __func__, sessions_nalloc, options.max_sessions);
-               tmp = xreallocarray(sessions, sessions_nalloc + 1,
-                   sizeof(*sessions));
+               tmp = xrecallocarray(sessions, sessions_nalloc,
+                   sessions_nalloc + 1, sizeof(*sessions));
                if (tmp == NULL) {
                        error("%s: cannot allocate %d sessions",
                            __func__, sessions_nalloc + 1);
@@ -1673,8 +1673,8 @@ session_env_req(Session *s)
        for (i = 0; i < options.num_accept_env; i++) {
                if (match_pattern(name, options.accept_env[i])) {
                        debug2("Setting env %d: %s=%s", s->num_env, name, val);
-                       s->env = xreallocarray(s->env, s->num_env + 1,
-                           sizeof(*s->env));
+                       s->env = xrecallocarray(s->env, s->num_env,
+                           s->num_env + 1, sizeof(*s->env));
                        s->env[s->num_env].name = name;
                        s->env[s->num_env].val = val;
                        s->num_env++;

diff --git a/usr.bin/ssh/ssh-pkcs11.c b/usr.bin/ssh/ssh-pkcs11.c
index d4b2be4..763997e 100644 (file)
--- a/usr.bin/ssh/ssh-pkcs11.c
+++ b/usr.bin/ssh/ssh-pkcs11.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-pkcs11.c,v 1.24 2017/05/30 14:15:17 markus Exp $ */
+/* $OpenBSD: ssh-pkcs11.c,v 1.25 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * Copyright (c) 2010 Markus Friedl.  All rights reserved.
  *
@@ -535,8 +535,8 @@ pkcs11_fetch_keys_filter(struct pkcs11_provider *p, CK_ULONG slotidx,
                                sshkey_free(key);
                        } else {
                                /* expand key array and add key */
-                               *keysp = xreallocarray(*keysp, *nkeys + 1,
-                                   sizeof(struct sshkey *));
+                               *keysp = xrecallocarray(*keysp, *nkeys,
+                                   *nkeys + 1, sizeof(struct sshkey *));
                                (*keysp)[*nkeys] = key;
                                *nkeys = *nkeys + 1;
                                debug("have %d keys", *nkeys);

diff --git a/usr.bin/ssh/sshbuf.c b/usr.bin/ssh/sshbuf.c
index e4a4f1e..c978080 100644 (file)
--- a/usr.bin/ssh/sshbuf.c
+++ b/usr.bin/ssh/sshbuf.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: sshbuf.c,v 1.9 2017/05/26 20:34:49 markus Exp $       */
+/*     $OpenBSD: sshbuf.c,v 1.10 2017/05/31 09:15:42 deraadt Exp $     */
 /*
  * Copyright (c) 2011 Damien Miller
  *
@@ -191,15 +191,16 @@ sshbuf_reset(struct sshbuf *buf)
                buf->off = buf->size;
                return;
        }
-       if (sshbuf_check_sanity(buf) == 0)
-               explicit_bzero(buf->d, buf->alloc);
+       (void) sshbuf_check_sanity(buf);
        buf->off = buf->size = 0;
        if (buf->alloc != SSHBUF_SIZE_INIT) {
-               if ((d = realloc(buf->d, SSHBUF_SIZE_INIT)) != NULL) {
+               if ((d = recallocarray(buf->d, buf->alloc, SSHBUF_SIZE_INIT,
+                   1)) != NULL) {
                        buf->cd = buf->d = d;
                        buf->alloc = SSHBUF_SIZE_INIT;
                }
-       }
+       } else
+               explicit_bzero(buf->d, SSHBUF_SIZE_INIT);
 }
 
 size_t
@@ -251,9 +252,8 @@ sshbuf_set_max_size(struct sshbuf *buf, size_t max_size)
                        rlen = ROUNDUP(buf->size, SSHBUF_SIZE_INC);
                if (rlen > max_size)
                        rlen = max_size;
-               explicit_bzero(buf->d + buf->size, buf->alloc - buf->size);
                SSHBUF_DBG(("new alloc = %zu", rlen));
-               if ((dp = realloc(buf->d, rlen)) == NULL)
+               if ((dp = recallocarray(buf->d, buf->alloc, rlen, 1)) == NULL)
                        return SSH_ERR_ALLOC_FAIL;
                buf->cd = buf->d = dp;
                buf->alloc = rlen;
@@ -342,7 +342,7 @@ sshbuf_allocate(struct sshbuf *buf, size_t len)
        if (rlen > buf->max_size)
                rlen = buf->alloc + need;
        SSHBUF_DBG(("adjusted rlen %zu", rlen));
-       if ((dp = realloc(buf->d, rlen)) == NULL) {
+       if ((dp = recallocarray(buf->d, buf->alloc, rlen, 1)) == NULL) {
                SSHBUF_DBG(("realloc fail"));
                return SSH_ERR_ALLOC_FAIL;
        }

diff --git a/usr.bin/ssh/sshkey.c b/usr.bin/ssh/sshkey.c
index db73b30..8587661 100644 (file)
--- a/usr.bin/ssh/sshkey.c
+++ b/usr.bin/ssh/sshkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.c,v 1.50 2017/05/08 06:11:06 djm Exp $ */
+/* $OpenBSD: sshkey.c,v 1.51 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Alexander von Gernler.  All rights reserved.
@@ -1728,8 +1728,9 @@ cert_parse(struct sshbuf *b, struct sshkey *key, struct sshbuf *certbuf)
                        goto out;
                }
                oprincipals = key->cert->principals;
-               key->cert->principals = reallocarray(key->cert->principals,
-                   key->cert->nprincipals + 1, sizeof(*key->cert->principals));
+               key->cert->principals = recallocarray(key->cert->principals,
+                   key->cert->nprincipals, key->cert->nprincipals + 1,
+                   sizeof(*key->cert->principals));
                if (key->cert->principals == NULL) {
                        free(principal);
                        key->cert->principals = oprincipals;

diff --git a/usr.bin/ssh/utf8.c b/usr.bin/ssh/utf8.c
index e14fbd5..312b181 100644 (file)
--- a/usr.bin/ssh/utf8.c
+++ b/usr.bin/ssh/utf8.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: utf8.c,v 1.6 2017/04/17 14:31:23 schwarze Exp $ */
+/* $OpenBSD: utf8.c,v 1.7 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * Copyright (c) 2016 Ingo Schwarze <schwarze@openbsd.org>
  *
@@ -67,7 +67,7 @@ grow_dst(char **dst, size_t *sz, size_t maxsz, char **dp, size_t need)
        tsz = *sz + 128;
        if (tsz > maxsz)
                tsz = maxsz;
-       if ((tp = realloc(*dst, tsz)) == NULL)
+       if ((tp = recallocarray(*dst, *sz, tsz, 1)) == NULL)
                return -1;
        *dp = tp + (*dp - *dst);
        *dst = tp;

diff --git a/usr.bin/ssh/xmalloc.c b/usr.bin/ssh/xmalloc.c
index 039cae2..83e30c6 100644 (file)
--- a/usr.bin/ssh/xmalloc.c
+++ b/usr.bin/ssh/xmalloc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: xmalloc.c,v 1.33 2016/02/15 09:47:49 dtucker Exp $ */
+/* $OpenBSD: xmalloc.c,v 1.34 2017/05/31 09:15:42 deraadt Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -71,6 +71,18 @@ xreallocarray(void *ptr, size_t nmemb, size_t size)
        return new_ptr;
 }
 
+void *
+xrecallocarray(void *ptr, size_t onmemb, size_t nmemb, size_t size)
+{
+       void *new_ptr;
+
+       new_ptr = recallocarray(ptr, onmemb, nmemb, size);
+       if (new_ptr == NULL)
+               fatal("xrecallocarray: out of memory (%zu elements of %zu bytes)",
+                   nmemb, size);
+       return new_ptr;
+}
+
 char *
 xstrdup(const char *str)
 {

diff --git a/usr.bin/ssh/xmalloc.h b/usr.bin/ssh/xmalloc.h
index e499289..cf38ddf 100644 (file)
--- a/usr.bin/ssh/xmalloc.h
+++ b/usr.bin/ssh/xmalloc.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: xmalloc.h,v 1.16 2016/02/15 09:47:49 dtucker Exp $ */
+/* $OpenBSD: xmalloc.h,v 1.17 2017/05/31 09:15:42 deraadt Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -20,6 +20,7 @@ void   ssh_malloc_init(void);
 void   *xmalloc(size_t);
 void   *xcalloc(size_t, size_t);
 void   *xreallocarray(void *, size_t, size_t);
+void   *xrecallocarray(void *, size_t, size_t, size_t);
 char   *xstrdup(const char *);
 int     xasprintf(char **, const char *, ...)
                 __attribute__((__format__ (printf, 2, 3)))