Bug#205048: ITP: neverball -- A colorful 3D arcade game in which you tilt the floor to roll the ball through the obstacle course before time runs out
On Tue, Aug 12, 2003 at 09:54:22AM +0100, Steve Kemp wrote:
> Please apply the enclosed patch before packaging - this closes a
> buffer overflow attack which is locally exploitable.
[...]
> --- config.c 2003-08-12 09:42:54.000000000 +0100
> +++ config.c-orig 2003-08-12 09:43:54.000000000 +0100
> @@ -66,18 +66,14 @@
>
> if ((dir = getenv("HOME")))
> {
> - strcpy(dst, dir);
> - strcat(dst, "/");
> - strcat(dst, src);
> - return 1;
> + snprintf(dst, sizeof(dst)-1, "%s/%s", dir, src);
> + return( 1 );
> }
>
> if ((vol = getenv("HOMEDRIVE")) && (dir = getenv("HOMEPATH")))
> {
> - strcpy(dst, vol);
> - strcat(dst, dir);
> - strcat(dst, "\\");
> - strcat(dst, src);
> + snprintf(dst, sizeof(dst)-1, "%s%s\\%s",
> + vol, dir, src );
> return 1;
> }
>
It would be a good idea to check the return value of snprintf() too -
and be careful, doing things with snprintf()'s return value portably is
a pain - since otherwise you may produce wrong results instead of
overrunning a buffer and not notice.
I wish something like man-db's strappend() [1] were more widely used.
I've been fixing a lot of buffer overruns by moving to that (he says,
going off to try to stop the damn thing segfaulting).
[1] Append a varargs list of strings to its first argument, allocating
or reallocating memory as necessary. If the first argument is NULL,
concatenate all the strings given into newly allocated memory. The
above would look like this:
char *dst;
if ((dir = getenv("HOME")))
{
dst = strappend(NULL, dir, "/", src, NULL);
return 1;
}
if ((vol = getenv("HOMEDRIVE")) && (dir = getenv("HOMEPATH")))
{
dst = strappend (NULL, vol, dir, "\\", src, NULL);
return 1;
}
--
Colin Watson [cjwatson@flatline.org.uk]
Reply to: