Subject: sprintf unsafe? [was Re: vixie-crontab vunerable?]
To: None <current-users@NetBSD.ORG>
From: der Mouse <mouse@Holo.Rodents.Montreal.QC.CA>
List: current-users
Date: 12/17/1996 11:27:10
> sprintf is a bit in between. sprintf("%s", p) seems pretty unsafe,
> but is there anything dangerous about sprintf("%d", &d)?
Aside from picking nits - sprintf takes a buffer argument, and you
don't want to print a pointer with %d - these really depend on where
the data come from. Using sprintf with %s can be safe; for example
{ int l;
char *t;
l = strlen(dir) + 1 + strlen(file) + 1;
t = malloc(l);
/* probably test for t != 0 here */
sprintf(t,"%s/%s",dir,file);
}
And, similarly, sprintf(...,"%d",d); can be unsafe if you haven't been
careful to size the buffer based on the possible values d can take on.
If the range is restricted, this can be fine - but if d is
unrestricted, well, I've seen too much code that does something like
{ char buf[16];
sprintf(buf,"%d",some_int_expression);
}
which is fine yesterday, when ints were 32 bits wide, and mostly today,
but with 64-bit ints this risks overflowing buf[]. The buffer in such
cases needs to be declared with something like
char buf[((sizeof(int)*CHAR_BIT)+8)/3];
(which allocates enough space to represent an int in octal, including
sign and terminating NUL, and decimal always uses no more digits than
octal). For that matter, even gets() _can_ be safe - if, for example,
it's reading from a forked copy of the same program via a pipe. (Note
I didn't say anything about exec()ing; this is not accidental.)
However, the set of safe uses of gets() is so tiny that I support the
warning for it; this is not true of sprintf, strcpy, etc.
der Mouse
mouse@rodents.montreal.qc.ca
7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B