Re: Ping Re: Fix strtod integer/buffer overflow (bug 14459)

[Andreas Jaeger <aj at suse dot com>]

On Friday, August 17, 2012 15:28:21 Joseph S. Myers wrote:
> On Fri, 17 Aug 2012, Florian Weimer wrote:
> > On 08/16/2012 05:58 PM, Joseph S. Myers wrote:
> > > Ping.  This patch
> > > <http://sourceware.org/ml/libc-alpha/2012-08/msg00202.html> is
> > > pending review - it seems we have more people interested in
> > > devising procedures for hypothetical external reports of security
> > > bugs, than in actually reviewing patches for security bugs. 
> > > There was one off-list request for assertions in str_to_mpn that
> > > the number doesn't cause overflow there, so this patch version
> > > adds them.  Tested x86 and x86_64.
> > 
> > These asserts do not fire in my testing (I tried the test case and
> > the asserts in str_to_mpn).  Do we disable asserts, or does the
> > compiler optimize it away for some reason?
> 
> This version also removes the definition of NDEBUG from strtod_l.c -

Thanks!

> the code is complicated enough, and has enough history of bugs, that I
> don't think disabling assertions in it is a good idea.  (Some code in
> glibc with assertions builds with them enabled, some with them
> disabled; there doesn't seem to be any general rule about which.  My
> inclination is that they should generally be enabled by default - and
> if anyone wants an assertions-disabled glibc then they could add a
> --disable-assert configure option.)
> 
> Tested x86_64 and x86.
> 
> 2012-08-17  Joseph Myers  <joseph@codesourcery.com>
> 
> 	[BZ #14459]
> 	* stdlib/strtod_l.c: Include <stdint.h>.
> 	(NDEBUG): Do not define.
> 	(round_and_return): Change exponent parameter to type intmax_t.

AFAIK the custom is to use capitals for variables like EXPONENT. Please 
adjust the changelog entry for that before you commit.

> 	Rearrange calculations to avoid internal overflow possibilities.
> 	(str_to_mpn): Change exponent parameter to type intmax_t *.
> 	Rearrange calculations to avoid internal overflow possibilities.
> 	Assert that number fits inside MPNSIZE limbs.
> 	(____STRTOF_INTERNAL): Change exponent variable to type intmax_t.
> 	Change dig_no, int_no and lead_zero to type size_t.  Rearrange
> 	calculations and add assertions to avoid internal overflow
> 	possibilities.  Add casts to avoid signed/unsigned operations.
> 	* stdlib/tst-strtod-overflow.c: New file.
> 	* stdlib/Makefile (tests): Add tst-strtod-overflow.

Thanks, this is fine, 

Andreas
-- 
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
    GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126