Re: [open-source] Re: Wish for 2002

[Paul Eggert <eggert at twinsun dot com>]

> From: Russ Allbery <rra@stanford.edu>
> Date: Mon, 07 Jan 2002 16:30:49 -0800
> 
> Ben Laurie <ben@algroup.co.uk> writes:
> 
> > What's the first mail I read this morning? A post to Bugtraq about
> > a buffer overflow in gzip (which, guess what, is a GNU app) that
> > was incorrectly fixed using strncpy.
> 
> What is the *real* problem in the source code?

There is no *real* problem in the source code.

Here is a recent post to Bugtraq about the problem:
http://www.securityfocus.com/archive/1/247785

This message is talking about this proposed patch to gzip 1.2.4:

	--- gzip.c	Thu Aug 19 06:39:43 1993
	+++ gzip.c-new	Thu Jan 10 14:29:53 2002
	@@ -1683,7 +1683,7 @@ local void treat_dir(dir)
		}
		len = strlen(dir);
		if (len + NLENGTH(dp) + 1 < MAX_PATH_LEN - 1) {
	-	    strcpy(nbuf,dir);
	+	    strncpy(nbuf, dir, sizeof(nbuf) - 1);
		    if (len != 0 /* dir = "" means current dir on Amiga */
	 #ifdef PATH_SEP2
			&& dir[len-1] != PATH_SEP2

However, nbuf is of size MAX_PATH_LEN, so the strcpy can't overflow in
practice.  The patch is therefore unnecessary.

This sort of kneejerk reaction ("Horrors!  The code is using strcpy!
It must be a security hole!") is all too common these days.  And it
diverts attention from the real problems in the code.  gzip 1.2.4 has
dozens if not hundreds of security holes; this isn't one of them, so
we're wasting our valuable time talking about it.



> Why was there a fixed-length buffer in there at all?  Why was a fixed
> buffer being used for data that could be of arbitrary length?  Could it be
> of arbitrary length?  What's the correct size of the buffer?  Should it be
> dynamically allocated?

gzip is not a typical GNU application, as it does not follow the GNU
coding standards at all.  Among other things, it has fixed-size
buffers all over the place.  It's pretty bad.

I've removed a few (but not all) of the fixed-size problems
in the latest test version, which you can get at:
ftp://alpha.gnu/org/gnu/gzip/gzip-1.3.2.tar.gz

This version also fixes all the gzip security holes that I know about.

Further fixes are welcome of course.  But I don't have a lot of time
to spend on gzip maintenance, so please don't send no-op patches like
the one quoted above.  Also, I prefer to not use strlcpy and strlcat,
since they tend to mess up the code, so please bear that in mind if
you send patches.

If you've read this message carefully, perhaps you'll understand
better why I think that strlcpy/strlcat are counterproductive in real code.
(It's not an opinion that I've developed by armchair theorizing.  :-)