Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 31 Jul 2009 10:52:06 +0400
From:      Eygene Ryabinkin <rea-fbsd@codelabs.ru>
To:        d@delphij.net
Cc:        rrl <endian.sign@gmail.com>, freebsd-security@freebsd.org
Subject:   Re: gzip memory corruption
Message-ID:  <Tu2B09acFZJXI7%2BMoAa3%2B5FRW24@xg9dzetjpj18poIU9mNsJ0TqP1U>
In-Reply-To: <4A72846B.60604@delphij.net>
References:  <20090708193339.GA4836@minerva.freedsl.mg> <qbNi6WaraP%2BYYd65ZtihTj0ewks@BpFm1zkZmHABxHH1eUOcQSRoWTc> <4A553080.5060205@delphij.net> <4A553458.70005@delphij.net> <LxW4OaFbQKVvB5FP5/FFtXkZd3U@%2BE41IXYRRzAjXLJbRTrYDjniL/s> <4A7231A1.2050104@delphij.net> <856ux8zhn21/d1hDLYeNjC7FQ1Y@xg9dzetjpj18poIU9mNsJ0TqP1U> <4A72846B.60604@delphij.net>

next in thread | previous in thread | raw e-mail | index | archive | help
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
    {_.-``-'         {_/            #



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Tu2B09acFZJXI7%2BMoAa3%2B5FRW24>