Sources Bugzilla – Bug 13506
tzfile.c heap overrun/corruption
Last modified: 2011-12-21 23:58:14 UTC
Created attachment 6113 [details]
Jeff Law work-in-progress patch
Jeff Law writes:
-----BEGIN PGP SIGNED MESSAGE-----
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.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
-----END PGP SIGNATURE-----
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.
I added a patch.
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).
Also looks like s390 won't build because SIZE_MAX is not defined. Guessing stdint.h needs to be included in tzfile.c
(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.