Re: gzip memory corruption



Xin, good day.

Wed, Jul 08, 2009 at 05:05:44PM -0700, Xin LI wrote:
The offending code lays in the function file_compress:
/* Add (usually) .gz to filename */
if ((size_t)snprintf(outfile, outsize, "%s%s",
file, suffixes[0].zipped) >= outsize)
memcpy(outfile - suffixes[0].ziplen - 1,
suffixes[0].zipped, suffixes[0].ziplen + 1);
The memcpy() call looks like a complete madness: it will write before
the beginning of the 'outfile', so it will be buffer underflow in any
case (unless I am terribly mistaken and missing some obvious point).

I'd change the above code to warn and return if snprintf will discard
some trailing characters, the patch is attached.

I have attached another possible fix, which catches the problem when
parsing the command line. The point is that, I think we really want to
catch bad input as early as possible.

Yes, it is good to catch it here too.

Index: gzip.c
===================================================================
--- gzip.c (?????? 195435)
+++ gzip.c (????????????)
@@ -372,6 +372,8 @@
case 'S':
len = strlen(optarg);
if (len != 0) {
+ if (len >= PATH_MAX)
+ errx(1, "incorrect suffix: '%s'", optarg);
suffixes[0].zipped = optarg;
suffixes[0].ziplen = len;
} else {

But the place with the memcpy() should be patched too. Two reasons:

- suffix could not (yet) overflow PATH_MAX, but filename + suffix --
can;

- I am really worried about the usage of memcpy with underflow;
I had tried to study the reasons for it via NetBSD CVS, but
it just appeared one day and the reason for going to
'outfile - suffixes[0].ziplen - 1' (with .gz its outfile - 4)
are unknown; I am still taking this as the programming error.

So, unless you know why we're underflowing the passed pointer,
the memcpy block should be patched too, for future safety and
code correctness.
--
Eygene
_ ___ _.--. #
\`.|\..----...-'` `-._.-'_.-'` # Remember that it is hard
/ ' ` , __.--' # to read the on-line manual
)/' _/ \ `-_, / # while single-stepping the kernel.
`-'" `"\_ ,_.-;_.-\_ ', fsc/as #
_.-'_./ {_.' ; / # -- FreeBSD Developers handbook
{_.-``-' {_/ #
_______________________________________________
freebsd-security@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-security
To unsubscribe, send any mail to "freebsd-security-unsubscribe@xxxxxxxxxxx"