[Patches] Fix for bug PR#341

M.-A. Lemburg mal@lemburg.com
Fri, 02 Jun 2000 10:53:13 +0200


Trent Mick wrote:
> 
> Discussion:
> 
> This patch fixes the string formatting overflow problem. It tries to do a
> little better than MAL's magic nunumber (50) check. If this looks good then I
> can do the same for unicodeobject.c
> 
> [Tim P on MAL's original patch]
> > but I'll join Fred in objecting to the code
> > it's mimicking:  not only do magic numbers suck, but these particular magic
> > numbers implicitly rely on PyString_Format's tmpbuf vector being declared of
> > another magical size larger than them.  As usual, flaky code gets flakier.
> 
> My patch still uses the magic number for the temporary buffer. This seems to me
> a good practical limit. With the patch this buffer can no longer overflow (as
> well, it is faster than malloc'ing a perfect sized buffer every time).
> 
> [MAL]
> > A redesign would, of course, use a malloced buffer, the n-variants
> > of printf() and add long support ;-) ... maybe for 1.7.
> 
> No long support in this patch :(
> 
> [Guido on MAL's original patch]
> > Having read the patch and the discussion about magic numbers, I agree
> > with Marc-Andre: let's apply the quick fix now, worry about
> > correctness later.
> 
> Maybe this patch is preferable.

Yep. The patch doesn't yet address the general case (use malloc etc.),
but makes things a little more flexbile.

(Even though I find some tests size_t vs. INT_MAX overkill --
 noone will store 2GB strings in memory ;-)

>                         int sign;
>                         int len;
> ! #define TMPBUFLEN (size_t)120
> !                       char tmpbuf[TMPBUFLEN]; /* For format{float,int,char}() */
>                         char *fmt_start = fmt;

You should move this #define outside of the function for
better visibility (and perhaps add a comment).
 
>                         fmt++;
> ***************
> *** 2618,2624 ****
>                                 if (c == 'i')
>                                         c = 'd';
>                                 buf = tmpbuf;
> !                               len = formatint(buf, flags, prec, c, v);
>                                 if (len < 0)
>                                         goto error;
>                                 sign = (c == 'd');
> --- 2653,2659 ----
>                                 if (c == 'i')
>                                         c = 'd';
>                                 buf = tmpbuf;
> !                               len = formatint(buf, TMPBUFLEN, flags, prec, c, v);

All of these direct references to TMPBUFLEN should
be changed to sizeof(buf) for clarity.

-- 
Marc-Andre Lemburg
______________________________________________________________________
Business:                                      http://www.lemburg.com/
Python Pages:                           http://www.lemburg.com/python/