- 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 20d034a..9d129a1 100644
--- 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
@@ -4375,4 +4385,4 @@
- (djm) Trim deprecated options from INSTALL. Mention UsePAM
- (djm) Fix quote handling in sftp; Patch from admorten AT umich.edu
-$Id: ChangeLog,v 1.4273 2006/03/26 03:19:21 djm Exp $
+$Id: ChangeLog,v 1.4274 2006/03/26 03:22:47 djm Exp $
diff --git a/auth-pam.c b/auth-pam.c
index 3d64de7..c12f413 100644
--- a/auth-pam.c
+++ b/auth-pam.c
@@ -703,7 +703,7 @@
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 @@
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 08682e0..1666f74 100644
--- a/buffer.c
+++ b/buffer.c
@@ -109,7 +109,7 @@
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 0e7d5cf..5706833 100644
--- a/channels.c
+++ b/channels.c
@@ -266,8 +266,8 @@
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 @@
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 746ff5d..ff9ca4d 100644
--- a/deattack.c
+++ b/deattack.c
@@ -97,7 +97,7 @@
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 bf7b1ed..96d90de 100644
--- a/misc.c
+++ b/misc.c
@@ -425,7 +425,7 @@
} 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 bf9db97..3068b8d 100644
--- a/scp.c
+++ b/scp.c
@@ -1190,7 +1190,7 @@
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 b00caa5..f0a0bdd 100644
--- a/session.c
+++ b/session.c
@@ -837,7 +837,7 @@
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 @@
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 c34f919..8b4d67b 100644
--- a/sftp-client.c
+++ b/sftp-client.c
@@ -393,8 +393,7 @@
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 a6add52..52b7323 100644
--- a/sftp-server.c
+++ b/sftp-server.c
@@ -716,7 +716,7 @@
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 67bde55..042b18f 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -803,7 +803,7 @@
}
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 bdf73ec..662f700 100644
--- a/ssh-rand-helper.c
+++ b/ssh-rand-helper.c
@@ -768,7 +768,7 @@
*/
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 @@
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 ce4195f..55fb7ba 100644
--- a/ssh-rsa.c
+++ b/ssh-rsa.c
@@ -144,7 +144,7 @@
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 ca08948..305895a 100644
--- a/uidswap.c
+++ b/uidswap.c
@@ -76,7 +76,7 @@
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 @@
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 6d56781..d5d7b6b 100644
--- a/xmalloc.c
+++ b/xmalloc.c
@@ -35,7 +35,7 @@
{
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 @@
}
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 @@
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 b6d521a..ef29787 100644
--- 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 *, ...)