- djm@cvs.openbsd.org 2006/03/25 01:13:23

     [buffer.c channels.c deattack.c misc.c scp.c session.c sftp-client.c]
     [sftp-server.c ssh-agent.c ssh-rsa.c xmalloc.c xmalloc.h auth-pam.c]
     [uidswap.c]
     change OpenSSH's xrealloc() function from being xrealloc(p, new_size)
     to xrealloc(p, new_nmemb, new_itemsize).

     realloc is particularly prone to integer overflows because it is
     almost always allocating "n * size" bytes, so this is a far safer
     API; ok deraadt@

diff --git a/ChangeLog b/ChangeLog
index 167be49522d56474495df0b3ec87b1ebd55cba7b..89e7555df77fbdac720a0ae4814042828530cbd4 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -118,6 +118,16 @@
      to die
 
      feedback and ok deraadt@
+   - djm@cvs.openbsd.org 2006/03/25 01:13:23
+     [buffer.c channels.c deattack.c misc.c scp.c session.c sftp-client.c]
+     [sftp-server.c ssh-agent.c ssh-rsa.c xmalloc.c xmalloc.h auth-pam.c]
+     [uidswap.c]
+     change OpenSSH's xrealloc() function from being xrealloc(p, new_size)
+     to xrealloc(p, new_nmemb, new_itemsize).
+
+     realloc is particularly prone to integer overflows because it is
+     almost always allocating "n * size" bytes, so this is a far safer 
+     API; ok deraadt@
 
 20060325
  - OpenBSD CVS Sync

diff --git a/auth-pam.c b/auth-pam.c
index 3d64de76a59e5f36e3071e40f8e3bf24432f37b2..c12f413e7a3726cd3346e25f43eb21921c63fa71 100644 (file)
--- a/auth-pam.c
+++ b/auth-pam.c
@@ -703,7 +703,7 @@ sshpam_query(void *ctx, char **name, char **info,
                case PAM_PROMPT_ECHO_OFF:
                        *num = 1;
                        len = plen + mlen + 1;
-                       **prompts = xrealloc(**prompts, len);
+                       **prompts = xrealloc(**prompts, 1, len);
                        strlcpy(**prompts + plen, msg, len - plen);
                        plen += mlen;
                        **echo_on = (type == PAM_PROMPT_ECHO_ON);
@@ -713,7 +713,7 @@ sshpam_query(void *ctx, char **name, char **info,
                case PAM_TEXT_INFO:
                        /* accumulate messages */
                        len = plen + mlen + 2;
-                       **prompts = xrealloc(**prompts, len);
+                       **prompts = xrealloc(**prompts, 1, len);
                        strlcpy(**prompts + plen, msg, len - plen);
                        plen += mlen;
                        strlcat(**prompts + plen, "\n", len - plen);

diff --git a/buffer.c b/buffer.c
index 08682e0f125bd17b03ce4a940ce8a92c1bf883b4..1666f742e7f7a123b8449d90f68870c41cb7224a 100644 (file)
--- a/buffer.c
+++ b/buffer.c
@@ -109,7 +109,7 @@ restart:
        if (newlen > BUFFER_MAX_LEN)
                fatal("buffer_append_space: alloc %u not supported",
                    newlen);
-       buffer->buf = xrealloc(buffer->buf, newlen);
+       buffer->buf = xrealloc(buffer->buf, 1, newlen);
        buffer->alloc = newlen;
        goto restart;
        /* NOTREACHED */

diff --git a/channels.c b/channels.c
index 0e7d5cf58505bc56dfa194646c9449d7b9f10093..5706833a98923c6f608eb08eed6ceafe11e17c67 100644 (file)
--- a/channels.c
+++ b/channels.c
@@ -266,8 +266,8 @@ channel_new(char *ctype, int type, int rfd, int wfd, int efd,
                if (channels_alloc > 10000)
                        fatal("channel_new: internal error: channels_alloc %d "
                            "too big.", channels_alloc);
-               channels = xrealloc(channels,
-                   (channels_alloc + 10) * sizeof(Channel *));
+               channels = xrealloc(channels, channels_alloc + 10,
+                   sizeof(Channel *));
                channels_alloc += 10;
                debug2("channel: expanding %d", channels_alloc);
                for (i = found; i < channels_alloc; i++)
@@ -1789,15 +1789,20 @@ void
 channel_prepare_select(fd_set **readsetp, fd_set **writesetp, int *maxfdp,
     u_int *nallocp, int rekeying)
 {
-       u_int n, sz;
+       u_int n, sz, nfdset;
 
        n = MAX(*maxfdp, channel_max_fd);
 
-       sz = howmany(n+1, NFDBITS) * sizeof(fd_mask);
+       nfdset = howmany(n+1, NFDBITS);
+       /* Explicitly test here, because xrealloc isn't always called */
+       if (nfdset && SIZE_T_MAX / nfdset < sizeof(fd_mask))
+               fatal("channel_prepare_select: max_fd (%d) is too large", n);
+       sz = nfdset * sizeof(fd_mask);
+
        /* perhaps check sz < nalloc/2 and shrink? */
        if (*readsetp == NULL || sz > *nallocp) {
-               *readsetp = xrealloc(*readsetp, sz);
-               *writesetp = xrealloc(*writesetp, sz);
+               *readsetp = xrealloc(*readsetp, nfdset, sizeof(fd_mask));
+               *writesetp = xrealloc(*writesetp, nfdset, sizeof(fd_mask));
                *nallocp = sz;
        }
        *maxfdp = n;

diff --git a/deattack.c b/deattack.c
index 746ff5d43ad47f8280304b2694a6a5bb0ddedde5..ff9ca4dd542f2ba6e87af210442a8dca6b1fb244 100644 (file)
--- a/deattack.c
+++ b/deattack.c
@@ -97,7 +97,7 @@ detect_attack(u_char *buf, u_int32_t len)
                n = l;
        } else {
                if (l > n) {
-                       h = (u_int16_t *) xrealloc(h, l * HASH_ENTRYSIZE);
+                       h = (u_int16_t *)xrealloc(h, l, HASH_ENTRYSIZE);
                        n = l;
                }
        }

diff --git a/misc.c b/misc.c
index bf7b1ed6603690153d589600e10149854ce791e3..96d90dec9e79970a7ce1471ffbe018b5d6d982f3 100644 (file)
--- a/misc.c
+++ b/misc.c
@@ -425,7 +425,7 @@ addargs(arglist *args, char *fmt, ...)
        } else if (args->num+2 >= nalloc)
                nalloc *= 2;
 
-       args->list = xrealloc(args->list, nalloc * sizeof(char *));
+       args->list = xrealloc(args->list, 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 bf9db97cf49c0feb5189302274d5245a972560ef..3068b8d32ceb571b0c829c778ce949efbd464328 100644 (file)
--- a/scp.c
+++ b/scp.c
@@ -1190,7 +1190,7 @@ allocbuf(BUF *bp, int fd, int blksize)
        if (bp->buf == NULL)
                bp->buf = xmalloc(size);
        else
-               bp->buf = xrealloc(bp->buf, size);
+               bp->buf = xrealloc(bp->buf, 1, size);
        memset(bp->buf, 0, size);
        bp->cnt = size;
        return (bp);

diff --git a/session.c b/session.c
index b00caa54772fdc5f3bf1562e9935870b6c7d35a0..f0a0bdd2fb930d2fe18dbdbc57d7b3955abf0f08 100644 (file)
--- a/session.c
+++ b/session.c
@@ -837,7 +837,7 @@ child_set_env(char ***envp, u_int *envsizep, const char *name,
                        if (envsize >= 1000)
                                fatal("child_set_env: too many env vars");
                        envsize += 50;
-                       env = (*envp) = xrealloc(env, envsize * sizeof(char *));
+                       env = (*envp) = xrealloc(env, envsize, sizeof(char *));
                        *envsizep = envsize;
                }
                /* Need to set the NULL pointer at end of array beyond the new slot. */
@@ -1941,8 +1941,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 = xrealloc(s->env, sizeof(*s->env) *
-                           (s->num_env + 1));
+                       s->env = xrealloc(s->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/sftp-client.c b/sftp-client.c
index c34f919a4097494198eb1db4f036617fa00e429e..8b4d67b58b46c9eca8bfd2b39f412fbee2af10e1 100644 (file)
--- a/sftp-client.c
+++ b/sftp-client.c
@@ -393,8 +393,7 @@ do_lsreaddir(struct sftp_conn *conn, char *path, int printflag,
                                printf("%s\n", longname);
 
                        if (dir) {
-                               *dir = xrealloc(*dir, sizeof(**dir) *
-                                   (ents + 2));
+                               *dir = xrealloc(*dir, ents + 2, sizeof(**dir));
                                (*dir)[ents] = xmalloc(sizeof(***dir));
                                (*dir)[ents]->filename = xstrdup(filename);
                                (*dir)[ents]->longname = xstrdup(longname);

diff --git a/sftp-server.c b/sftp-server.c
index a6add52aa165145d384a4270ceabfab3753bb2fa..52b7323c2b1abc938077bbfb253aace1d0c889b3 100644 (file)
--- a/sftp-server.c
+++ b/sftp-server.c
@@ -716,7 +716,7 @@ process_readdir(void)
                while ((dp = readdir(dirp)) != NULL) {
                        if (count >= nstats) {
                                nstats *= 2;
-                               stats = xrealloc(stats, nstats * sizeof(Stat));
+                               stats = xrealloc(stats, nstats, sizeof(Stat));
                        }
 /* XXX OVERFLOW ? */
                        snprintf(pathname, sizeof pathname, "%s%s%s", path,

diff --git a/ssh-agent.c b/ssh-agent.c
index 67bde556067263bde80e274154b713d25d7f130b..042b18f540afe0742d742161b9798a0a0abae62c 100644 (file)
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -803,7 +803,7 @@ new_socket(sock_type type, int fd)
                }
        old_alloc = sockets_alloc;
        new_alloc = sockets_alloc + 10;
-       sockets = xrealloc(sockets, new_alloc * sizeof(sockets[0]));
+       sockets = xrealloc(sockets, new_alloc, sizeof(sockets[0]));
        for (i = old_alloc; i < new_alloc; i++)
                sockets[i].type = AUTH_UNUSED;
        sockets_alloc = new_alloc;

diff --git a/ssh-rand-helper.c b/ssh-rand-helper.c
index bdf73ec4841826270dfb3b6dec5ec57cd21db114..662f70080c81d21177a957a04e84ebe846379d87 100644 (file)
--- a/ssh-rand-helper.c
+++ b/ssh-rand-helper.c
@@ -768,7 +768,7 @@ prng_read_commands(char *cmdfilename)
                 */
                if (cur_cmd == num_cmds) {
                        num_cmds *= 2;
-                       entcmd = xrealloc(entcmd, num_cmds *
+                       entcmd = xrealloc(entcmd, num_cmds,
                            sizeof(entropy_cmd_t));
                }
        }
@@ -777,7 +777,7 @@ prng_read_commands(char *cmdfilename)
        memset(&entcmd[cur_cmd], '\0', sizeof(entropy_cmd_t));
 
        /* trim to size */
-       entropy_cmds = xrealloc(entcmd, (cur_cmd + 1) *
+       entropy_cmds = xrealloc(entcmd, (cur_cmd + 1),
            sizeof(entropy_cmd_t));
 
        debug("Loaded %d entropy commands from %.100s", cur_cmd,

diff --git a/ssh-rsa.c b/ssh-rsa.c
index ce4195fead9484b545232b254856c9c1491e3994..55fb7ba59900c901247218f82f3475835de0a621 100644 (file)
--- a/ssh-rsa.c
+++ b/ssh-rsa.c
@@ -144,7 +144,7 @@ ssh_rsa_verify(const Key *key, const u_char *signature, u_int signaturelen,
                u_int diff = modlen - len;
                debug("ssh_rsa_verify: add padding: modlen %u > len %u",
                    modlen, len);
-               sigblob = xrealloc(sigblob, modlen);
+               sigblob = xrealloc(sigblob, 1, modlen);
                memmove(sigblob + diff, sigblob, len);
                memset(sigblob, 0, diff);
                len = modlen;

diff --git a/uidswap.c b/uidswap.c
index ca089480605af35056ff26999a5bae99cb31f704..305895a44f0dc19028d193239069370ae60c0dbb 100644 (file)
--- a/uidswap.c
+++ b/uidswap.c
@@ -76,7 +76,7 @@ temporarily_use_uid(struct passwd *pw)
                fatal("getgroups: %.100s", strerror(errno));
        if (saved_egroupslen > 0) {
                saved_egroups = xrealloc(saved_egroups,
-                   saved_egroupslen * sizeof(gid_t));
+                   saved_egroupslen, sizeof(gid_t));
                if (getgroups(saved_egroupslen, saved_egroups) < 0)
                        fatal("getgroups: %.100s", strerror(errno));
        } else { /* saved_egroupslen == 0 */
@@ -95,7 +95,7 @@ temporarily_use_uid(struct passwd *pw)
                        fatal("getgroups: %.100s", strerror(errno));
                if (user_groupslen > 0) {
                        user_groups = xrealloc(user_groups,
-                           user_groupslen * sizeof(gid_t));
+                           user_groupslen, sizeof(gid_t));
                        if (getgroups(user_groupslen, user_groups) < 0)
                                fatal("getgroups: %.100s", strerror(errno));
                } else { /* user_groupslen == 0 */

diff --git a/xmalloc.c b/xmalloc.c
index 6d56781d921925df514608aacad1db30f9824a9b..d5d7b6bc5cd52a73ebcd5f32c7e91aa2089e2eb7 100644 (file)
--- a/xmalloc.c
+++ b/xmalloc.c
@@ -35,7 +35,7 @@ xcalloc(size_t nmemb, size_t size)
 {
        void *ptr;
 
-        if (nmemb && size && SIZE_T_MAX / nmemb < size)
+       if (nmemb && size && SIZE_T_MAX / nmemb < size)
                fatal("xcalloc: nmemb * size > SIZE_T_MAX");
        if (size == 0 || nmemb == 0)
                fatal("xcalloc: zero size");
@@ -47,10 +47,13 @@ xcalloc(size_t nmemb, size_t size)
 }
 
 void *
-xrealloc(void *ptr, size_t new_size)
+xrealloc(void *ptr, size_t nmemb, size_t size)
 {
        void *new_ptr;
+       size_t new_size = nmemb * size;
 
+       if (nmemb && size && SIZE_T_MAX / nmemb < size)
+               fatal("xrealloc: nmemb * size > SIZE_T_MAX");
        if (new_size == 0)
                fatal("xrealloc: zero size");
        if (ptr == NULL)
@@ -58,7 +61,8 @@ xrealloc(void *ptr, size_t new_size)
        else
                new_ptr = realloc(ptr, new_size);
        if (new_ptr == NULL)
-               fatal("xrealloc: out of memory (new_size %lu bytes)", (u_long) new_size);
+               fatal("xrealloc: out of memory (new_size %lu bytes)",
+                   (u_long) new_size);
        return new_ptr;
 }
 

diff --git a/xmalloc.h b/xmalloc.h
index b6d521a66db30ed7b8eaf03b85ebb61e804be10a..ef29787bd6129a5081a0929b82ba69280e13b520 100644 (file)
--- a/xmalloc.h
+++ b/xmalloc.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: xmalloc.h,v 1.10 2006/03/25 00:05:41 djm Exp $        */
+/*     $OpenBSD: xmalloc.h,v 1.11 2006/03/25 01:13:23 djm Exp $        */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -21,7 +21,7 @@
 
 void   *xmalloc(size_t);
 void   *xcalloc(size_t, size_t);
-void   *xrealloc(void *, size_t);
+void   *xrealloc(void *, size_t, size_t);
 void     xfree(void *);
 char   *xstrdup(const char *);
 int     xasprintf(char **, const char *, ...)