1
0
mirror of https://sourceware.org/git/glibc.git synced 2025-07-30 22:43:12 +03:00

Tighten up vfprintf width, precision, and total length overflow handling.

With help from Paul Eggert, Carlos O'Donell, and Roland McGrath.
	* 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.  Use EOVERFLOW rather than ERANGE throughout.  Use
	SIZE_MAX not INT_MAX for integer overflow test.
	* stdio-common/printf-parsemb.c: If read_int signals an overflow,
	skip the construct in the format string but do not record anything.
	* 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.
This commit is contained in:
David S. Miller
2012-04-02 14:31:19 -07:00
parent 302cadd343
commit 135ffda8b8
5 changed files with 147 additions and 39 deletions

View File

@ -1,3 +1,19 @@
2012-04-02 David S. Miller <davem@davemloft.net>
With help from Paul Eggert, Carlos O'Donell, and Roland McGrath.
* 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. Use EOVERFLOW rather than ERANGE throughout. Use
SIZE_MAX not INT_MAX for integer overflow test.
* stdio-common/printf-parsemb.c: If read_int signals an overflow,
skip the construct in the format string but do not record anything.
* 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.
2012-04-02 Thomas Schwinge <thomas@codesourcery.com> 2012-04-02 Thomas Schwinge <thomas@codesourcery.com>
* string/test-memcmp.c [! WIDE]: #include <limits.h> for CHAR_MIN, * string/test-memcmp.c [! WIDE]: #include <limits.h> for CHAR_MIN,

View File

@ -1,12 +1,22 @@
/* BZ #5424 */ /* BZ #5424 */
#include <stdio.h> #include <stdio.h>
#include <errno.h>
/* INT_MAX + 1 */
#define N 2147483648 #define N 2147483648
/* (INT_MAX / 2) + 2 */
#define N2 1073741825
/* INT_MAX - 3 */
#define N3 2147483644
#define STRINGIFY(S) #S #define STRINGIFY(S) #S
#define MAKE_STR(S) STRINGIFY(S) #define MAKE_STR(S) STRINGIFY(S)
#define SN MAKE_STR(N) #define SN MAKE_STR(N)
#define SN2 MAKE_STR(N2)
#define SN3 MAKE_STR(N3)
static int static int
do_test (void) do_test (void)
@ -20,11 +30,25 @@ do_test (void)
return 1; return 1;
} }
ret = fprintf (fp, "%" SN "d%" SN "d", 1, 1); ret = fprintf (fp, "%" SN "d", 1);
printf ("ret = %d\n", ret);
if (ret != -1 || errno != EOVERFLOW)
return 1;
ret = fprintf (fp, "%." SN "d", 1);
printf ("ret = %d\n", ret);
if (ret != -1 || errno != EOVERFLOW)
return 1;
ret = fprintf (fp, "%." SN3 "d", 1);
printf ("ret = %d\n", ret);
if (ret != -1 || errno != EOVERFLOW)
return 1;
ret = fprintf (fp, "%" SN2 "d%" SN2 "d", 1, 1);
printf ("ret = %d\n", ret); printf ("ret = %d\n", ret);
return ret != -1; return ret != -1 || errno != EOVERFLOW;
} }
#define TIMEOUT 30 #define TIMEOUT 30

View File

@ -68,16 +68,27 @@ union printf_arg
#ifndef DONT_NEED_READ_INT #ifndef DONT_NEED_READ_INT
/* Read a simple integer from a string and update the string pointer. /* Read a simple integer from a string and update the string pointer.
It is assumed that the first character is a digit. */ It is assumed that the first character is a digit. */
static unsigned int static int
read_int (const UCHAR_T * *pstr) read_int (const UCHAR_T * *pstr)
{ {
unsigned int retval = **pstr - L_('0'); int retval = **pstr - L_('0');
while (ISDIGIT (*++(*pstr))) while (ISDIGIT (*++(*pstr)))
{ if (retval >= 0)
retval *= 10; {
retval += **pstr - L_('0'); if (INT_MAX / 10 < retval)
} retval = -1;
else
{
int digit = **pstr - L_('0');
retval *= 10;
if (INT_MAX - digit < retval)
retval = -1;
else
retval += digit;
}
}
return retval; return retval;
} }

View File

@ -87,12 +87,15 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
n = read_int (&format); n = read_int (&format);
if (n > 0 && *format == L_('$')) if (n != 0 && *format == L_('$'))
/* Is positional parameter. */ /* Is positional parameter. */
{ {
++format; /* Skip the '$'. */ ++format; /* Skip the '$'. */
spec->data_arg = n - 1; if (n != -1)
*max_ref_arg = MAX (*max_ref_arg, n); {
spec->data_arg = n - 1;
*max_ref_arg = MAX (*max_ref_arg, n);
}
} }
else else
/* Oops; that was actually the width and/or 0 padding flag. /* Oops; that was actually the width and/or 0 padding flag.
@ -160,10 +163,13 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
/* The width argument might be found in a positional parameter. */ /* The width argument might be found in a positional parameter. */
n = read_int (&format); n = read_int (&format);
if (n > 0 && *format == L_('$')) if (n != 0 && *format == L_('$'))
{ {
spec->width_arg = n - 1; if (n != -1)
*max_ref_arg = MAX (*max_ref_arg, n); {
spec->width_arg = n - 1;
*max_ref_arg = MAX (*max_ref_arg, n);
}
++format; /* Skip '$'. */ ++format; /* Skip '$'. */
} }
} }
@ -177,9 +183,13 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
} }
} }
else if (ISDIGIT (*format)) else if (ISDIGIT (*format))
/* Constant width specification. */ {
spec->info.width = read_int (&format); int n = read_int (&format);
/* Constant width specification. */
if (n != -1)
spec->info.width = n;
}
/* Get the precision. */ /* Get the precision. */
spec->prec_arg = -1; spec->prec_arg = -1;
/* -1 means none given; 0 means explicit 0. */ /* -1 means none given; 0 means explicit 0. */
@ -196,10 +206,13 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
{ {
n = read_int (&format); n = read_int (&format);
if (n > 0 && *format == L_('$')) if (n != 0 && *format == L_('$'))
{ {
spec->prec_arg = n - 1; if (n != -1)
*max_ref_arg = MAX (*max_ref_arg, n); {
spec->prec_arg = n - 1;
*max_ref_arg = MAX (*max_ref_arg, n);
}
++format; ++format;
} }
} }
@ -213,7 +226,12 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
} }
} }
else if (ISDIGIT (*format)) else if (ISDIGIT (*format))
spec->info.prec = read_int (&format); {
int n = read_int (&format);
if (n != -1)
spec->info.prec = n;
}
else else
/* "%.?" is treated like "%.0?". */ /* "%.?" is treated like "%.0?". */
spec->info.prec = 0; spec->info.prec = 0;

View File

@ -67,10 +67,10 @@
do { \ do { \
unsigned int _val = val; \ unsigned int _val = val; \
assert ((unsigned int) done < (unsigned int) INT_MAX); \ assert ((unsigned int) done < (unsigned int) INT_MAX); \
if (__builtin_expect ((unsigned int) INT_MAX - (unsigned int) done \ if (__builtin_expect (INT_MAX - done < _val, 0)) \
< _val, 0)) \
{ \ { \
done = -1; \ done = -1; \
__set_errno (EOVERFLOW); \
goto all_done; \ goto all_done; \
} \ } \
done += _val; \ done += _val; \
@ -141,12 +141,17 @@
do \ do \
{ \ { \
assert ((size_t) done <= (size_t) INT_MAX); \ assert ((size_t) done <= (size_t) INT_MAX); \
if ((size_t) PUT (s, (String), (Len)) != (size_t) (Len) \ if ((size_t) PUT (s, (String), (Len)) != (size_t) (Len)) \
|| (size_t) INT_MAX - (size_t) done < (size_t) (Len)) \
{ \ { \
done = -1; \ done = -1; \
goto all_done; \ goto all_done; \
} \ } \
if (__builtin_expect (INT_MAX - done < (Len), 0)) \
{ \
done = -1; \
__set_errno (EOVERFLOW); \
goto all_done; \
} \
done += (Len); \ done += (Len); \
} \ } \
while (0) while (0)
@ -1435,10 +1440,21 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
const UCHAR_T *tmp; /* Temporary value. */ const UCHAR_T *tmp; /* Temporary value. */
tmp = ++f; tmp = ++f;
if (ISDIGIT (*tmp) && read_int (&tmp) && *tmp == L_('$')) if (ISDIGIT (*tmp))
/* The width comes from a positional parameter. */ {
goto do_positional; int pos = read_int (&tmp);
if (pos == -1)
{
__set_errno (EOVERFLOW);
done = -1;
goto all_done;
}
if (pos && *tmp == L_('$'))
/* The width comes from a positional parameter. */
goto do_positional;
}
width = va_arg (ap, int); width = va_arg (ap, int);
/* Negative width means left justified. */ /* Negative width means left justified. */
@ -1449,9 +1465,9 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
left = 1; left = 1;
} }
if (__builtin_expect (width >= (size_t) -1 / sizeof (CHAR_T) - 32, 0)) if (__builtin_expect (width >= INT_MAX / sizeof (CHAR_T) - 32, 0))
{ {
__set_errno (ERANGE); __set_errno (EOVERFLOW);
done = -1; done = -1;
goto all_done; goto all_done;
} }
@ -1481,9 +1497,10 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
LABEL (width): LABEL (width):
width = read_int (&f); width = read_int (&f);
if (__builtin_expect (width >= (size_t) -1 / sizeof (CHAR_T) - 32, 0)) if (__builtin_expect (width == -1
|| width >= INT_MAX / sizeof (CHAR_T) - 32, 0))
{ {
__set_errno (ERANGE); __set_errno (EOVERFLOW);
done = -1; done = -1;
goto all_done; goto all_done;
} }
@ -1518,10 +1535,21 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
const UCHAR_T *tmp; /* Temporary value. */ const UCHAR_T *tmp; /* Temporary value. */
tmp = ++f; tmp = ++f;
if (ISDIGIT (*tmp) && read_int (&tmp) > 0 && *tmp == L_('$')) if (ISDIGIT (*tmp))
/* The precision comes from a positional parameter. */ {
goto do_positional; int pos = read_int (&tmp);
if (pos == -1)
{
__set_errno (EOVERFLOW);
done = -1;
goto all_done;
}
if (pos && *tmp == L_('$'))
/* The precision comes from a positional parameter. */
goto do_positional;
}
prec = va_arg (ap, int); prec = va_arg (ap, int);
/* If the precision is negative the precision is omitted. */ /* If the precision is negative the precision is omitted. */
@ -1529,15 +1557,26 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
prec = -1; prec = -1;
} }
else if (ISDIGIT (*f)) else if (ISDIGIT (*f))
prec = read_int (&f); {
prec = read_int (&f);
/* The precision was specified in this case as an extremely
large positive value. */
if (prec == -1)
{
__set_errno (EOVERFLOW);
done = -1;
goto all_done;
}
}
else else
prec = 0; prec = 0;
if (prec > width if (prec > width
&& prec > sizeof (work_buffer) / sizeof (work_buffer[0]) - 32) && prec > sizeof (work_buffer) / sizeof (work_buffer[0]) - 32)
{ {
if (__builtin_expect (prec >= (size_t) -1 / sizeof (CHAR_T) - 32, 0)) if (__builtin_expect (prec >= INT_MAX / sizeof (CHAR_T) - 32, 0))
{ {
__set_errno (ERANGE); __set_errno (EOVERFLOW);
done = -1; done = -1;
goto all_done; goto all_done;
} }
@ -1710,9 +1749,9 @@ do_positional:
+ sizeof (*args_type)); + sizeof (*args_type));
/* Check for potential integer overflow. */ /* Check for potential integer overflow. */
if (__builtin_expect (nargs > SIZE_MAX / bytes_per_arg, 0)) if (__builtin_expect (nargs > INT_MAX / bytes_per_arg, 0))
{ {
__set_errno (ERANGE); __set_errno (EOVERFLOW);
done = -1; done = -1;
goto all_done; goto all_done;
} }