On Thu, May 04, 2006 at 12:18:57PM +1000, Darren Tucker
wrote:> While wandering in auth-pam.c I noticed that there's a few
Portable-specific
> escapees from the xmalloc(foo * bar) cleanup.
My grep for xmalloc missed a couple of bare mallocs, new patch below.
> There's also a "probably can't happen" integer overflow
in
> ssh-rand-helper.c with the memset:
>
> num_cmds = 64;
> - entcmd = xmalloc(num_cmds * sizeof(entropy_cmd_t));
> + entcmd = xcalloc(num_cmds, sizeof(entropy_cmd_t));
> memset(entcmd, '\0', num_cmds * sizeof(entropy_cmd_t));
>
> Unless sizeof(entropy_cmd_t) is *really* big, it ought to safe :-)
Looking at xcalloc I see it's always safe since it will abort on overflow.
The memset is redundant, though, since calloc zeros and there's a few
similar. Zap 'em?
Index: auth-pam.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/auth-pam.c,v
retrieving revision 1.132
diff -u -p -r1.132 auth-pam.c
--- auth-pam.c 26 Mar 2006 03:22:48 -0000 1.132
+++ auth-pam.c 4 May 2006 02:26:39 -0000
@@ -288,7 +288,7 @@ import_environments(Buffer *b)
/* Import environment from subprocess */
num_env = buffer_get_int(b);
- sshpam_env = xmalloc((num_env + 1) * sizeof(*sshpam_env));
+ sshpam_env = xcalloc(num_env + 1, sizeof(*sshpam_env));
debug3("PAM: num env strings %d", num_env);
for(i = 0; i < num_env; i++)
sshpam_env[i] = buffer_get_string(b, NULL);
@@ -335,7 +335,7 @@ sshpam_thread_conv(int n, sshpam_const s
if (n <= 0 || n > PAM_MAX_NUM_MSG)
return (PAM_CONV_ERR);
- if ((reply = malloc(n * sizeof(*reply))) == NULL)
+ if ((reply = calloc(n, sizeof(*reply))) == NULL)
return (PAM_CONV_ERR);
memset(reply, 0, n * sizeof(*reply));
@@ -533,7 +533,7 @@ sshpam_store_conv(int n, sshpam_const st
if (n <= 0 || n > PAM_MAX_NUM_MSG)
return (PAM_CONV_ERR);
- if ((reply = malloc(n * sizeof(*reply))) == NULL)
+ if ((reply = calloc(n, sizeof(*reply))) == NULL)
return (PAM_CONV_ERR);
memset(reply, 0, n * sizeof(*reply));
@@ -935,7 +935,7 @@ sshpam_tty_conv(int n, sshpam_const stru
if (n <= 0 || n > PAM_MAX_NUM_MSG || !isatty(STDIN_FILENO))
return (PAM_CONV_ERR);
- if ((reply = malloc(n * sizeof(*reply))) == NULL)
+ if ((reply = calloc(n, sizeof(*reply))) == NULL)
return (PAM_CONV_ERR);
memset(reply, 0, n * sizeof(*reply));
Index: groupaccess.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/groupaccess.c,v
retrieving revision 1.10
diff -u -p -r1.10 groupaccess.c
--- groupaccess.c 26 Mar 2006 03:24:49 -0000 1.10
+++ groupaccess.c 4 May 2006 01:56:11 -0000
@@ -52,8 +52,8 @@ ga_init(const char *user, gid_t base)
ngroups = MAX(NGROUPS_MAX, sysconf(_SC_NGROUPS_MAX));
#endif
- groups_bygid = xmalloc(ngroups * sizeof(*groups_bygid));
- groups_byname = xmalloc(ngroups * sizeof(*groups_byname));
+ groups_bygid = xcalloc(ngroups, sizeof(*groups_bygid));
+ groups_byname = xcalloc(ngroups, sizeof(*groups_byname));
if (getgrouplist(user, base, groups_bygid, &ngroups) == -1)
logit("getgrouplist: groups list too small");
Index: monitor.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/monitor.c,v
retrieving revision 1.102
diff -u -p -r1.102 monitor.c
--- monitor.c 31 Mar 2006 12:14:24 -0000 1.102
+++ monitor.c 4 May 2006 01:56:40 -0000
@@ -924,7 +924,7 @@ mm_answer_pam_respond(int sock, Buffer *
sshpam_authok = NULL;
num = buffer_get_int(m);
if (num > 0) {
- resp = xmalloc(num * sizeof(char *));
+ resp = xcalloc(num, sizeof(char *));
for (i = 0; i < num; ++i)
resp[i] = buffer_get_string(m, NULL);
ret = (sshpam_device.respond)(sshpam_ctxt, num, resp);
Index: monitor_wrap.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/monitor_wrap.c,v
retrieving revision 1.59
diff -u -p -r1.59 monitor_wrap.c
--- monitor_wrap.c 31 Mar 2006 12:13:02 -0000 1.59
+++ monitor_wrap.c 4 May 2006 01:57:46 -0000
@@ -776,8 +776,8 @@ mm_sshpam_query(void *ctx, char **name,
*name = buffer_get_string(&m, NULL);
*info = buffer_get_string(&m, NULL);
*num = buffer_get_int(&m);
- *prompts = xmalloc((*num + 1) * sizeof(char *));
- *echo_on = xmalloc((*num + 1) * sizeof(u_int));
+ *prompts = xcalloc((*num + 1), sizeof(char *));
+ *echo_on = xcalloc((*num + 1), sizeof(u_int));
for (i = 0; i < *num; ++i) {
(*prompts)[i] = buffer_get_string(&m, NULL);
(*echo_on)[i] = buffer_get_int(&m);
Index: scard-opensc.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/scard-opensc.c,v
retrieving revision 1.15
diff -u -p -r1.15 scard-opensc.c
--- scard-opensc.c 13 May 2004 07:29:35 -0000 1.15
+++ scard-opensc.c 4 May 2006 01:58:41 -0000
@@ -455,7 +455,7 @@ sc_get_keys(const char *id, const char *
}
key_count = r;
}
- keys = xmalloc(sizeof(Key *) * (key_count*2+1));
+ keys = xcalloc(key_count * 2 + 1, sizeof(Key *));
for (i = 0; i < key_count; i++) {
sc_pkcs15_object_t *tmp_obj = NULL;
cert_id = ((sc_pkcs15_cert_info_t *)(certs[i]->data))->id;
Index: session.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/session.c,v
retrieving revision 1.327
diff -u -p -r1.327 session.c
--- session.c 23 Apr 2006 02:10:49 -0000 1.327
+++ session.c 4 May 2006 01:59:13 -0000
@@ -984,7 +984,7 @@ do_setup_env(Session *s, const char *she
/* Initialize the environment. */
envsize = 100;
- env = xmalloc(envsize * sizeof(char *));
+ env = xcalloc(envsize, sizeof(char *));
env[0] = NULL;
#ifdef HAVE_CYGWIN
Index: ssh-rand-helper.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/ssh-rand-helper.c,v
retrieving revision 1.29
diff -u -p -r1.29 ssh-rand-helper.c
--- ssh-rand-helper.c 26 Mar 2006 03:22:48 -0000 1.29
+++ ssh-rand-helper.c 4 May 2006 02:01:13 -0000
@@ -674,7 +674,7 @@ prng_read_commands(char *cmdfilename)
}
num_cmds = 64;
- entcmd = xmalloc(num_cmds * sizeof(entropy_cmd_t));
+ entcmd = xcalloc(num_cmds, sizeof(entropy_cmd_t));
memset(entcmd, '\0', num_cmds * sizeof(entropy_cmd_t));
/* Read in file */
Index: sshd.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/sshd.c,v
retrieving revision 1.333
diff -u -p -r1.333 sshd.c
--- sshd.c 26 Mar 2006 03:24:50 -0000 1.333
+++ sshd.c 4 May 2006 02:01:50 -0000
@@ -921,7 +921,7 @@ main(int ac, char **av)
/* Save argv. Duplicate so setproctitle emulation doesn't clobber it */
saved_argc = ac;
rexec_argc = ac;
- saved_argv = xmalloc(sizeof(*saved_argv) * (ac + 1));
+ saved_argv = xcalloc(ac + 1, sizeof(*saved_argv));
for (i = 0; i < ac; i++)
saved_argv[i] = xstrdup(av[i]);
saved_argv[i] = NULL;
Index: openbsd-compat/bsd-cygwin_util.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/openbsd-compat/bsd-cygwin_util.c,v
retrieving revision 1.16
diff -u -p -r1.16 bsd-cygwin_util.c
--- openbsd-compat/bsd-cygwin_util.c 25 Mar 2006 13:03:24 -0000 1.16
+++ openbsd-compat/bsd-cygwin_util.c 4 May 2006 02:02:57 -0000
@@ -268,7 +268,7 @@ fetch_windows_environment(void)
char **e, **p;
unsigned int i, idx = 0;
- p = xmalloc((WENV_SIZ + 1) * sizeof(char *));
+ p = xcalloc(WENV_SIZ + 1, sizeof(char *));
for (e = environ; *e != NULL; ++e) {
for (i = 0; i < WENV_SIZ; ++i) {
if (!strncmp(*e, wenv_arr[i].name, wenv_arr[i].namelen))
Index: openbsd-compat/fake-rfc2553.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/openbsd-compat/fake-rfc2553.c,v
retrieving revision 1.6
diff -u -p -r1.6 fake-rfc2553.c
--- openbsd-compat/fake-rfc2553.c 25 Mar 2006 13:03:24 -0000 1.6
+++ openbsd-compat/fake-rfc2553.c 4 May 2006 02:28:30 -0000
@@ -115,7 +115,7 @@ addrinfo *malloc_ai(int port, u_long add
{
struct addrinfo *ai;
- ai = malloc(sizeof(*ai) + sizeof(struct sockaddr_in));
+ ai = calloc(sizeof(*ai), sizeof(struct sockaddr_in));
if (ai == NULL)
return (NULL);
Index: openbsd-compat/setproctitle.c
==================================================================RCS file:
/usr/local/src/security/openssh/cvs/openssh_cvs/openbsd-compat/setproctitle.c,v
retrieving revision 1.9
diff -u -p -r1.9 setproctitle.c
--- openbsd-compat/setproctitle.c 20 Feb 2004 09:38:17 -0000 1.9
+++ openbsd-compat/setproctitle.c 4 May 2006 02:29:03 -0000
@@ -80,7 +80,7 @@ compat_init_setproctitle(int argc, char
/* Fail if we can't allocate room for the new environment */
for (i = 0; envp[i] != NULL; i++)
;
- if ((environ = malloc(sizeof(*environ) * (i + 1))) == NULL) {
+ if ((environ = calloc(i + 1, sizeof(*environ))) == NULL) {
environ = envp; /* put it back */
return;
}
--
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.