vfprintf typing problem

[Carlos O'Donell]

On Sat, Mar 31, 2012 at 9:06 PM, David Miller <davem@davemloft.net> wrote:
> [PATCH] Tighten up vfprintf width, precision, and total length overflow handling.
>
>        * stdio-common/printf-parse.h (read_int): Change return type to
>        'int', return -1 on INT_MAX overflow.
>        * stdio-common/vfprintf.c (vfprintf): Validate width and precision
>        against overflow of INT_MAX.  Set errno to EOVERFLOW when 'done'
>        overflows INT_MAX.  Check for overflow of in-format-string precision
>        values properly.
>        * stdio-common/bug22.c: Adjust to test both width/prevision
>        INT_MAX overflow as well as total length INT_MAX overflow.  Check
>        explicitly for proper errno values.
> ---
>  ChangeLog                   |   12 ++++++++++++
>  stdio-common/bug22.c        |   28 ++++++++++++++++++++++++++--
>  stdio-common/printf-parse.h |    6 ++++--
>  stdio-common/vfprintf.c     |   33 +++++++++++++++++++++++++--------
>  4 files changed, 67 insertions(+), 12 deletions(-)
>
> diff --git a/stdio-common/printf-parse.h b/stdio-common/printf-parse.h
> index 72665dc..bcf914b 100644
> --- a/stdio-common/printf-parse.h
> +++ b/stdio-common/printf-parse.h
> @@ -68,7 +68,7 @@ union printf_arg
>  #ifndef DONT_NEED_READ_INT
>  /* Read a simple integer from a string and update the string pointer.
>    It is assumed that the first character is a digit.  */
> -static unsigned int
> +static int
>  read_int (const UCHAR_T * *pstr)
>  {
>   unsigned int retval = **pstr - L_('0');
> @@ -76,10 +76,12 @@ read_int (const UCHAR_T * *pstr)
>   while (ISDIGIT (*++(*pstr)))
>     {
>       retval *= 10;
> +      if (retval > INT_MAX)
> +       return -1;

It seems to me like you just moved the problem around but haven't solved it yet.

Next digit is 8.
retval == 214748364
214748364*10=2147483640 < INT_MAX
214748364+8==2147483648==INT_MAX+1 > INT_MAX
No more digits.

>       retval += **pstr - L_('0');
>     }
>
> -  return retval;
> +  return (int) retval;

This overflows returning -2147483648, and now your "== -1" checks in
the callers are not correct.

Does that make sense?

The check probably has to be:

if (retval > INT_MAX/10-digit)
  return -1;
retval *= 10;
retal += digit;

That way you *know* the next two operations won't overflow, and either
you exit or get another chance at looking at the next operation.

It's close to my bedtime so I might have missed something.

Cheers,
Carlos.