Bug 13506 (CVE-2009-5029)

Summary: tzfile.c heap overrun/corruption (CVE-2009-5029)
Product: glibc Reporter: Paul Eggert <eggert>
Component: libcAssignee: Ulrich Drepper <drepper.fsp>
Severity: normal CC: allan, fweimer, law, polacek, rguenth, toolchain, vapier
Priority: P2 Flags: fweimer: security+
Version: 2.14   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:
Attachments: Jeff Law work-in-progress patch
catch multiplication as well as addition overflows

Description Paul Eggert 2011-12-15 20:44:55 UTC
Created attachment 6113 [details]
Jeff Law work-in-progress patch

In <http://cygwin.com/ml/libc-alpha/2011-12/msg00037.html>
Jeff Law writes:

Hash: SHA1

As y'all may be aware, there's an integer overflow which can be used
to trigger a heap overrun/corruption in time/tzfile.c



I'm not terribly familiar with the code in question, but ISTM we have
to verify the intermediate computations to determine the amount of
memory to malloc don't overflow/wrap.

Here's a WIP.  It catches the cases I've been made aware of
(overflowing total_size to 0 by creating a tzfile with a very large
tzh_charcnt).  But there may be further overflows I've missed.

Obviously it's not commented and it's unclear to me if we also want to
put in some kind of sanity checks on total_size to prevent it from
trying to malloc unreasoanble amounts of memory.

Your feedback would be greatly appreciated.

Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

Comment 1 Paul Eggert 2011-12-15 21:00:49 UTC
Created attachment 6114 [details]
catch multiplication as well as addition overflows

Jeff Law's work-in-progress patch misses some problematic overflows.  This is
because the integer multiplications may overflow too.  Attached is an
untested patch that catches the problematic overflows that I found
by inspection.  This patch does not attempt to catch all overflows, only
those that might corrupt memory.
Comment 2 Ulrich Drepper 2011-12-18 01:19:35 UTC
I added a patch.
Comment 4 Allan McRae 2011-12-19 05:50:57 UTC
Note that there is a typo in that patch. The "tzspec == 0"  should be "tzspec_len == 0".  I sent the trivial patch to the mailing list (awaiting moderation).
Comment 5 law 2011-12-19 07:57:44 UTC
Also looks like s390 won't build because SIZE_MAX is not defined.  Guessing stdint.h needs to be included in tzfile.c
Comment 6 Ulrich Drepper 2011-12-21 23:58:14 UTC
(In reply to comment #5)
> Also looks like s390 won't build because SIZE_MAX is not defined.  Guessing
> stdint.h needs to be included in tzfile.c

The correct change is to make the s390 header look like the x86-64 headers.