Date: Thu, 9 Jul 2009 00:25:15 +0400 From: Eygene Ryabinkin <rea-fbsd@codelabs.ru> To: rrl <endian.sign@gmail.com> Cc: freebsd-security@freebsd.org Subject: Re: gzip memory corruption Message-ID: <qbNi6WaraP%2BYYd65ZtihTj0ewks@BpFm1zkZmHABxHH1eUOcQSRoWTc> In-Reply-To: <20090708193339.GA4836@minerva.freedsl.mg> References: <20090708193339.GA4836@minerva.freedsl.mg>
next in thread | previous in thread | raw e-mail | index | archive | help
--4ZLFUWh1odzi/v6L
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Wed, Jul 08, 2009 at 10:33:39PM +0300, rrl wrote:
> I run Freebsd 7.2 and gzip doesn't handle correctly long suffix name
> with the -S option.
> > gzip -S `perl -e 'print "A"x1200'` dummy_file
> Memory fault (core dumped)
>
> 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.
--
Eygene
_ ___ _.--. #
\`.|\..----...-'` `-._.-'_.-'` # Remember that it is hard
/ ' ` , __.--' # to read the on-line manual
)/' _/ \ `-_, / # while single-stepping the kernel.
`-'" `"\_ ,_.-;_.-\_ ', fsc/as #
_.-'_./ {_.' ; / # -- FreeBSD Developers handbook
{_.-``-' {_/ #
--4ZLFUWh1odzi/v6L
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="gzip.c-fix-buffer-underflow.diff"
--- usr.bin/gzip/gzip.c.orig 2009-07-09 00:03:03.000000000 +0400
+++ usr.bin/gzip/gzip.c 2009-07-09 00:21:40.000000000 +0400
@@ -1235,9 +1235,12 @@
/* 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);
+ file, suffixes[0].zipped) >= outsize) {
+ warnx("Output file name '%s%s' is too long, exiting",
+ file, suffixes[0].zipped);
+ close(in);
+ return -1;
+ }
#ifndef SMALL
if (check_outfile(outfile) == 0) {
--4ZLFUWh1odzi/v6L--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?qbNi6WaraP%2BYYd65ZtihTj0ewks>