upstream commit

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

Upstream-ID: 822d664d6a5a1d10eccb23acdd53578a679d5065

diff --git a/auth2-pubkey.c b/auth2-pubkey.c
index 7a6280f8df2929fe86df89804e3136bee5eea02c..271dbaf65a84d6dcdd34646018c6834f2c1eff71 100644 (file)
--- a/auth2-pubkey.c
+++ b/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.
  *
@@ -1156,9 +1156,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/authfile.c b/authfile.c
index af4190eeb7100457f67bccb4cc7be71017c4f8aa..3481e0b0434b762eec930fcd18b5b4de0f134538 100644 (file)
--- a/authfile.c
+++ b/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.
  *
@@ -100,25 +100,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/bitmap.c b/bitmap.c
index 3d7aa1379598770ae2ea7fbe04971c95f1ba5ac7..71f87ec54ae8f1051f35bb58817dd25ec7d8e762 100644 (file)
--- a/bitmap.c
+++ b/bitmap.c
@@ -87,7 +87,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/clientloop.c b/clientloop.c
index 33d6fa90832ed0fc3a5b3d05c9386d56138ee386..612838376991e445282a7c88582aad86ceba1fae 100644 (file)
--- a/clientloop.c
+++ b/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
@@ -1812,9 +1812,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;
@@ -2046,9 +2046,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/hostfile.c b/hostfile.c
index 1804cff9903b83a808b6116c286d58855052eab1..12f174ff97b1b76a889ae9146eb7c027cdf18f0e 100644 (file)
--- a/hostfile.c
+++ b/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
@@ -251,7 +251,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/krl.c b/krl.c
index 3f28178b7b3c3b79856fce057ca43ace98bccc0d..086fc20e59336ccfe1a7eccf4b550250b2fa2532 100644 (file)
--- a/krl.c
+++ b/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 "includes.h"
 
@@ -1026,7 +1026,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/misc.c b/misc.c
index cfd32729ac7a50de28551186f2e1da03dc8b1dd2..af24fa5c46f5a551a9506405315bb0a16d90132c 100644 (file)
--- a/misc.c
+++ b/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.
@@ -539,7 +539,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/scp.c b/scp.c
index f9f48e0759cffc50cc8e3149daeba45a72ef92e8..a533eb09741224aaef4fc2dce973354f94c900c8 100644 (file)
--- a/scp.c
+++ b/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).
@@ -1368,11 +1368,7 @@ allocbuf(BUF *bp, int fd, int blksize)
 #endif /* HAVE_STRUCT_STAT_ST_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/session.c b/session.c
index cbd27c6896b4a4ede7c1d22575b9959a61ddb086..4ef48ecd6c85f929f456245852e9afed1efdf32c 100644 (file)
--- a/session.c
+++ b/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
@@ -1711,8 +1711,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);
@@ -2036,8 +2036,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/ssh-pkcs11.c b/ssh-pkcs11.c
index ea97508f125e4c99ebce2ea3cff3b43252548b90..b37491c5d68d71dd2c7b7ca1dbcecb2ed3408b60 100644 (file)
--- a/ssh-pkcs11.c
+++ b/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.
  *
@@ -546,8 +546,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/sshbuf.c b/sshbuf.c
index 652c99a21c389e575c2ab312062b21432298ba45..b7a90b5c27ce71fd454f9731b3386b422aba7cc7 100644 (file)
--- a/sshbuf.c
+++ b/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
  *
@@ -193,15 +193,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
@@ -253,9 +254,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;
@@ -344,7 +344,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/sshkey.c b/sshkey.c
index f9518bd770492bd1fe7ba033a75fbd2d5955c323..9a3f0be588a2cbda8bb87d07811e0dd62b073395 100644 (file)
--- a/sshkey.c
+++ b/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.
@@ -1764,8 +1764,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/utf8.c b/utf8.c
index da5778138cdb5648dc61b9bbb8ad1b12d5495956..bc131385f5362d08043b59a0280780e6260dbf43 100644 (file)
--- a/utf8.c
+++ b/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>
  *
@@ -76,7 +76,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/xmalloc.c b/xmalloc.c
index b58323677ee7a82232638fa6b629160a6c06f1f1..5cc0310a4766ccdca8e0ecba5332fdf6ab051e0f 100644 (file)
--- a/xmalloc.c
+++ b/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
@@ -77,6 +77,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/xmalloc.h b/xmalloc.h
index e4992893276d91197f85f04ff256f523e15b19c6..cf38ddfa48c5c4bff6b031ca800b6a2b6f745d6d 100644 (file)
--- a/xmalloc.h
+++ b/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)))