gzip memory corruption

Eygene Ryabinkin rea-fbsd at codelabs.ru
Fri Jul 31 06:52:10 UTC 2009


Xin,

Thu, Jul 30, 2009 at 10:43:07PM -0700, Xin LI wrote:
> After talking with Matthew Green (the author of NetBSD) it seems that it
> would be more reasonable to fix the bug itself than breaking upon
> receipt.  Here is the patch.

You'll probably want to check that (outsize - suffixes[0].ziplen - 1)
is greater than zero.  Like this:
-----
  		if ((size_t)snprintf(outfile, outsize, "%s%s",
  					file, suffixes[0].zipped) >= outsize) {
  			size_t sfx_start = outsize - suffixes[0] - 1;
  			if (sfx_start > 0) {
				memcpy(outfile + sfx_start,
				  suffixes[0].zipped, suffixes[0].ziplen + 1);
			} else {
				errx(1, "Can't insert suffix: name buffer is too short");
			}
		}
-----
Just now we can garantee that 'outsize' will fit any suffix because of
the suffix length check, but when Someone (TM) will modify the code,
this could no longer be true and a bug will arise again.  So it is
better to check this locally and fail loudly if we can't make it happen.

I should say that transforming the "/long-path/foo.gz" (that is
expected) into "/long-path/f.gz" isn't quite obvious for the end-user.
But if the absence of such a transformation will break anything that
relies on this behaviour (I can't think about any usages of this
behaviour, but who knows), then the code should keep it.  What were
Mattew's arguments for keeping the old behaviour?
-- 
Eygene
 _                ___       _.--.   #
 \`.|\..----...-'`   `-._.-'_.-'`   #  Remember that it is hard
 /  ' `         ,       __.--'      #  to read the on-line manual
 )/' _/     \   `-_,   /            #  while single-stepping the kernel.
 `-'" `"\_  ,_.-;_.-\_ ',  fsc/as   #
     _.-'_./   {_.'   ; /           #    -- FreeBSD Developers handbook
    {_.-``-'         {_/            #


More information about the freebsd-security mailing list