Re: LZW decompression issues

[Solar Designer <solar () openwall com>]

Hi Tavis,

On Wed, Sep 28, 2011 at 08:42:56PM +0200, Tavis Ormandy wrote:
> I believe I wrote that patch,

I believe you wrote a different patch, or two:

http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/gzip/Attic/gzip-1.3.5-google-owl-bound.diff
http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/gzip/Attic/gzip-1.3.5-gentoo-huft_build-return.diff

(these are in Attic because we've since updated to gzip 1.4).

As far as I can see, the sanity checks in
gzip-1.3.5-google-owl-bound.diff do not overlap with those in FreeBSD's
latest patch.  These are different sets of checks.

> I found a lot of vulnerabilities in gzip a few
> years ago, and added lots of additional sanity checks.

Right.  Thank you!

> FreeBSD went with my patch, which I think was much safer.

Good.  But apparently FreeBSD did not patch even older issues at the
same time - obviously, you wouldn't have spotted an issue that was
already non-existent in upstream gzip at the time, so you didn't report
it to them.

As to who originally added the "maxbits < 12" check, when, and why
exactly (and why this value), I still don't know.  In NetBSD, it is
added with a commit made 6 weeks ago:

http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.bin/gzip/zuncompress.c?only_with_tag=MAIN

The commit message is merely "Do proper input validation without
penalizing performance", and it makes several other changes as well
(FreeBSD in fact reused essentially the same patch).

NetBSD's advisory is here:

http://ftp.netbsd.org/pub/NetBSD/security/advisories/NetBSD-SA2011-007.txt.asc

and it also (correctly) says that NetBSD's gzip was affected.

Joerg - any comments?  For context:
http://www.openwall.com/lists/oss-security/2011/09/28/5

OpenBSD doesn't have gzip since 2003 - "Our compress, linked against
libz, now does everything gzip does." (from Theo's commit message)

Thanks,

Alexander