This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: vfprintf typing problem
- From: "Carlos O'Donell" <carlos at systemhalted dot org>
- To: David Miller <davem at davemloft dot net>
- Cc: eggert at cs dot ucla dot edu, libc-alpha at sourceware dot org
- Date: Sat, 31 Mar 2012 22:10:44 -0400
- Subject: Re: vfprintf typing problem
- References: <CADZpyizqQWqsCqCkb8+yX+VGAQt4Zq8dCpVQDhzd-8-_YGzG7A@mail.gmail.com><20120331.182538.20629350961321931.davem@davemloft.net><CADZpyixPs415NPaWRc0NYqsTnE5b2MQJLg5H88m4GFtMderspg@mail.gmail.com><20120331.210642.1272515098933091269.davem@davemloft.net>
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.