--- Begin Message ---
|
Subject: |
* editfns.c (Fformat): Fix several integer overflow problems. |
|
Date: |
Thu, 12 May 2011 20:01:52 -0700 |
|
User-agent: |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110424 Thunderbird/3.1.10 |
Here's a patch for several integer overflow problems in (format ...),
including some that cause core dumps. I plan to install this into
the trunk after some more testing.
* editfns.c (Fformat): Fix several integer overflow problems.
For example, without this change, (format "%2147483648d" 1) dumps
core on x86-64 GNU/Linux. Use EMACS_INT, not size_t, for sizes,
since we prefer using signed values, and EMACS_INT will be big
enough soon, even on 32-bit hosts. Also, prefer EMACS_INT to int
for sizes. Don't assume that pI is either "l" or ""; it might be
"ll" or "I64". Check for width and precision greater than
INT_MAX, as this can make sprintf go kaflooey.
=== modified file 'src/editfns.c'
--- src/editfns.c 2011-04-14 19:34:42 +0000
+++ src/editfns.c 2011-05-13 02:30:06 +0000
@@ -3583,11 +3583,12 @@
usage: (format STRING &rest OBJECTS) */)
(size_t nargs, register Lisp_Object *args)
{
- register size_t n; /* The number of the next arg to substitute */
- register size_t total; /* An estimate of the final length */
+ register EMACS_INT n; /* The number of the next arg to
substitute */
+ register EMACS_INT total; /* An estimate of the final length */
+ int pIlen = sizeof pI - 1;
char *buf, *p;
register char *format, *end, *format_start;
- int nchars;
+ EMACS_INT nchars;
/* Nonzero if the output should be a multibyte string,
which is true if any of the inputs is one. */
int multibyte = 0;
@@ -3603,7 +3604,7 @@
no argument, *will* be assigned to in the case that a `%' and `.'
occur after the final format specifier. */
int *precision = (int *) (alloca ((nargs + 1) * sizeof (int)));
- int longest_format;
+ EMACS_INT longest_format;
Lisp_Object val;
int arg_intervals = 0;
USE_SAFE_ALLOCA;
@@ -3619,7 +3620,8 @@
info[0] is unused. Unused elements have -1 for start. */
struct info
{
- int start, end, intervals;
+ EMACS_INT start, end;
+ int intervals;
} *info = 0;
/* It should not be necessary to GCPRO ARGS, because
@@ -3660,8 +3662,8 @@
/* Allocate the info and discarded tables. */
{
- size_t nbytes = (nargs+1) * sizeof *info;
- size_t i;
+ EMACS_INT nbytes = (nargs + 1) * sizeof *info;
+ EMACS_INT i;
if (!info)
info = (struct info *) alloca (nbytes);
memset (info, 0, nbytes);
@@ -3706,25 +3708,33 @@
|| * format == ' ' || *format == '+'))
++format;
+ /* Parse width and precision, limiting them to the range of 'int'
+ because otherwise the underyling sprintf may go kaflooey. */
+
if (*format >= '0' && *format <= '9')
{
- for (field_width = 0; *format >= '0' && *format <= '9'; ++format)
- field_width = 10 * field_width + *format - '0';
+ char *width_end;
+ unsigned long width = strtoul (format, &width_end, 10);
+ if (INT_MAX < width)
+ error ("Format string field width too large");
+ field_width = width;
+ format = width_end;
}
/* N is not incremented for another few lines below, so refer to
element N+1 (which might be precision[NARGS]). */
if (*format == '.')
{
- ++format;
- for (precision[n+1] = 0; *format >= '0' && *format <= '9'; ++format)
- precision[n+1] = 10 * precision[n+1] + *format - '0';
+ char *prec_end;
+ unsigned long prec = strtoul (format + 1, &prec_end, 10);
+ if (INT_MAX < prec)
+ error ("Format string precision too large");
+ precision[n + 1] = prec;
+ format = prec_end;
}
- /* Extra +1 for 'l' that we may need to insert into the
- format. */
- if (format - this_format_start + 2 > longest_format)
- longest_format = format - this_format_start + 2;
+ if (longest_format < format - this_format_start + pIlen + 1)
+ longest_format = format - this_format_start + pIlen + 1;
if (format == end)
error ("Format string ends in middle of format specifier");
@@ -3975,24 +3985,22 @@
}
else if (INTEGERP (args[n]) || FLOATP (args[n]))
{
- int this_nchars;
+ EMACS_INT this_nchars;
+ EMACS_INT this_format_len = format - this_format_start;
- memcpy (this_format, this_format_start,
- format - this_format_start);
- this_format[format - this_format_start] = 0;
+ memcpy (this_format, this_format_start, this_format_len);
+ this_format[this_format_len] = 0;
if (format[-1] == 'e' || format[-1] == 'f' || format[-1] == 'g')
sprintf (p, this_format, XFLOAT_DATA (args[n]));
else
{
- if (sizeof (EMACS_INT) > sizeof (int)
- && format[-1] != 'c')
+ if (pIlen && format[-1] != 'c')
{
- /* Insert 'l' before format spec. */
- this_format[format - this_format_start]
- = this_format[format - this_format_start - 1];
- this_format[format - this_format_start - 1] = 'l';
- this_format[format - this_format_start + 1] = 0;
+ /* Insert pI before format spec. */
+ memcpy (&this_format[this_format_len - 1], pI, pIlen);
+ this_format[this_format_len + pIlen - 1] = format[-1];
+ this_format[this_format_len + pIlen] = 0;
}
if (INTEGERP (args[n]))
@@ -4089,7 +4097,7 @@
if (CONSP (props))
{
EMACS_INT bytepos = 0, position = 0, translated = 0;
- int argn = 1;
+ EMACS_INT argn = 1;
Lisp_Object list;
/* Adjust the bounds of each text property
--- End Message ---
--- Begin Message ---
|
Subject: |
fixes committed to trunk |
|
Date: |
Fri, 27 May 2011 13:40:10 -0700 |
|
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110428 Fedora/3.1.10-1.fc14 Thunderbird/3.1.10 |
I just committed bzr 104390 to the Emacs trunk.
It merges fixes for bugs 8668, 8719, and 8722 as
previously discussed.
For Bug#8719, although the patch should fix the problems
it may not be the best patch. Kenichi Handa wrote that
he'll check it. Kenichi, if there are problems with it,
please feel free to replace it with something better
or to send me email with suggestions and I'll work on
making it better. Thanks.
--- End Message ---