[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: branch-1_4 allow cross-compiles; expose buffer overrun
|
From: |
Eric Blake |
|
Subject: |
Re: branch-1_4 allow cross-compiles; expose buffer overrun |
|
Date: |
Thu, 29 Jun 2006 21:36:27 -0600 |
|
User-agent: |
Thunderbird 1.5.0.4 (Windows/20060516) |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
According to Eric Blake on 6/29/2006 7:36 AM:
>
> $ M4_cv_have_efgcvt=no configure
> $ make
> ...
> $ echo 'format(%300d,1)'|src/m4
> Segmentation fault (core dumped)
>
Applying this patch now. The only thing remaining is for me to resolve
the OS/2 and mingw use of system() (I've been working on that by
cross-compiling a mingw image, but most of the effort has been on the
bug-gnulib list), and then we should get 1.4.5 out the door soon. I doubt
many platforms lack ecvt (even mingw had it), so the buffer overrun was
probably limited in scope, but that is no excuse for not fixing the
security hole now that we know about it. Unfortunately, this patch does
not backport nicely to 1.4.4, since it relies on gnulib (and 1.4.4 was
pre-automake days), so I really don't know what to recommend to distros as
a security fix.
2006-06-29 Eric Blake <address@hidden>
Fix buffer overrun bug.
* m4/gnulib-cache.m4: Augment with gnulib-tool --import
xvasprintf.
* src/format.c [HAVE_EFGCVT]: Delete this code, and use *printf
variant instead, since [efg]cvt are obsolete and our use of them
was buggy (savannah sr #104303).
(format): Fix buffer overflow by using xasprintf.
* doc/m4.texinfo (Format): Expand format test to catch both bugs.
* NEWS: Document this fix.
- --
Life is short - so eat dessert first!
Eric Blake address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFEpJw784KuGfSFAYARAiA5AKCxQRZlXnnYJiwA2DeEaHx26KDLvgCgkyvF
n82ncyatK4tgDx4F0dIXRt8=
=WhgK
-----END PGP SIGNATURE-----
Index: NEWS
===================================================================
RCS file: /sources/m4/m4/NEWS,v
retrieving revision 1.1.1.1.2.26
diff -u -p -r1.1.1.1.2.26 NEWS
--- NEWS 27 Jun 2006 13:31:43 -0000 1.1.1.1.2.26
+++ NEWS 30 Jun 2006 03:34:46 -0000
@@ -12,12 +12,16 @@ Version 1.4.5 - ?? 2006, by ??? (CVS ve
better diagnose version mismatch. Additionally, if the F directive
(builtin function) names an unknown builtin that existed in the m4 that
froze the file but not in the current m4 (for example, changeword), the
- warning is deferred until an attempt is made to actually use the builtin.
- This allows downgrading from beta m4-1.4o to m4-1.4.5 without breaking
- autoconf.
+ warning is deferred until an attempt is made to actually use the
+ builtin. This allows downgrading from beta m4-1.4o to stable m4-1.4.5
+ without breaking autoconf.
* The format and indir macros are now recognized only with arguments.
* The eval macro no longer crashes on x86 architectures when dividing the
minimum integer by -1.
+* On systems with ecvt and fcvt, format no longer truncates trailing
+ zeroes on integers printed with %.0f. On systems without these
+ functions, format is no longer subject to a buffer overflow that
+ permitted arbitrary code execution.
Version 1.4.4b - 17 June 2006, by Eric Blake (CVS version 1.4.4a)
Index: src/format.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/format.c,v
retrieving revision 1.1.1.1.2.2
diff -u -p -r1.1.1.1.2.2 format.c
--- src/format.c 15 Jun 2006 21:51:37 -0000 1.1.1.1.2.2
+++ src/format.c 30 Jun 2006 03:34:46 -0000
@@ -22,65 +22,7 @@
/* printf like formatting for m4. */
#include "m4.h"
-
-#ifdef HAVE_EFGCVT
-
-/* Various constants for floating point formatting. */
-#define MAXFIELD 128 /* size of buffer for formatted text */
-/* The following two are hardware dependant. */
-#define ECVTMAX 18 /* max number of significant digits for
%e */
-#define FCVTMAX (18+38+4) /* max number of significant digits
for %f */
-
-/* Externs used herein. */
-#if HAVE_EFGCVT <= 1
-extern char *ecvt (), *fcvt (), *gcvt ();
-#endif
-
-#ifndef STDC_HEADERS
-extern int atoi ();
-extern long atol ();
-extern double atof ();
-#endif /* STDC_HEADERS */
-
-#define min(a, b) ((a) < (b) ? (a) : (b))
-
-static char const digits[] = "0123456789abcdef";
-static char const Digits[] = "0123456789ABCDEF";
-
-/* STR has dimension MAXFIELD (?). */
-
-static char *
-ulong_to_str (register unsigned long val, char *str, int base,
- const char *digits)
-{
- register char *s = &str[MAXFIELD];
-
- *--s = '\0';
- do
- {
- *--s = digits[val % base];
- val /= base;
- }
- while (val > 0);
-
- return s;
-}
-
-/*-----------------------------------------.
-| Clear trailing zeroes, return argument. |
-`-----------------------------------------*/
-
-static char *
-clr0 (char *s)
-{
- register char *t;
-
- for (t = s + strlen (s); *--t == '0' && t > s;)
- *t = '\0';
- return s;
-}
-
-#endif /* HAVE_EFGCVT */
+#include "xvasprintf.h"
/* Simple varargs substitute. */
@@ -118,407 +60,6 @@ clr0 (char *s)
void
format (struct obstack *obs, int argc, token_data **argv)
{
-#ifdef HAVE_EFGCVT
-
- const char *fmt; /* format control string */
- int c; /* a simple character */
- char fc; /* format code */
-
- /* Flags. */
- char flags; /* 1 iff treating flags */
- char ljust; /* left justification */
- char mandsign; /* mandatory sign */
- char noplus; /* use space if no sign */
- char alternate; /* use alternate form */
- char zeropad; /* do zero padding */
- char plus; /* plus-sign, according to mandatory and noplus
*/
-
- /* Precision specifiers. */
- int width; /* minimum field width */
- int prec; /* precision */
- int maxch; /* maximum no. of chars to print */
- char lflag; /* long flag */
- char hflag; /* short flag */
-
- /* Different parts of each specification. */
- char sign; /* wanted sign, iff any */
- int ppad; /* pre-prefix zero padding */
- const char *prefix; /* value prefix */
- int lpad; /* zero padding on the left */
- register char *s; /* ptr to formatted text */
- int rpad; /* zero padding on the rigth*/
- const char *suffix; /* value suffix */
-
- /* Buffer and stuff. */
- char str[MAXFIELD]; /* buffer for formatted text */
- int length; /* length of str */
- int padding; /* padding at the left or rigth */
- register int i; /* an index */
-
-/* Length of trailing string in str. */
-#define LENGTH(s) (&str[MAXFIELD-1] - (s))
-#define HAS_SIGN (sign != '\0')
-
- fmt = ARG_STR (argc, argv);
- for (;;)
- {
- while ((c = *fmt++) != '%')
- {
- if (c == 0)
- return;
- obstack_1grow (obs, c);
- }
- if (*fmt == '%')
- {
- obstack_1grow (obs, '%');
- fmt++;
- continue;
- }
-
- /* Parse flags. */
- flags = 1;
- ljust = mandsign = noplus = alternate = zeropad = 0;
- do
- {
- switch (*fmt)
- {
- case '-': /* left justification */
- ljust = 1;
- break;
-
- case '+': /* mandatory sign */
- mandsign = 1;
- break;
-
- case ' ': /* space instead of positive sign */
- noplus = 1;
- break;
-
- case '0': /* zero padding */
- zeropad = 1;
- break;
-
- case '#': /* alternate output */
- alternate = 1;
- break;
-
- default:
- flags = 0;
- break;
- }
- }
- while (flags && fmt++);
-
- plus = '\0'; /* what to use as a plus ??? */
- if (mandsign)
- plus = '+';
- else if (noplus)
- plus = ' ';
-
- if (ljust)
- zeropad = 0;
-
- /* Minimum field width. */
- width = -1;
- if (*fmt == '*')
- {
- width = ARG_INT (argc, argv);
- fmt++;
- }
- else if (isdigit (to_uchar (*fmt)))
- {
- width = 0;
- do
- {
- width = width * 10 + *fmt++ - '0';
- }
- while (isdigit (to_uchar (*fmt)));
- }
-
- /* Maximum precision. */
- prec = -1;
- if (*fmt == '.')
- {
- if (*(++fmt) == '*')
- {
- prec = ARG_INT (argc, argv);
- ++fmt;
- }
- else if (isdigit (to_uchar (*fmt)))
- {
- prec = 0;
- do
- {
- prec = prec * 10 + *fmt++ - '0';
- }
- while (isdigit (to_uchar (*fmt)))
- ;
- }
- }
-
- /* Length modifiers. */
- lflag = (*fmt == 'l');
- hflag = (*fmt == 'h');
- if (lflag || hflag)
- fmt++;
-
- sign = '\0';
- ppad = lpad = rpad = 0;
- maxch = -1;
- prefix = suffix = "";
-
- switch (fc = *fmt++)
- {
-
- case '\0':
- return;
-
- case 'c':
- c = ARG_INT (argc, argv);
- str[0] = (unsigned char) c;
- str[1] = '\0';
- s = str;
- break;
-
- case 's':
- s = ARG_STR (argc, argv);
- maxch = prec;
- break;
-
- case 'd':
- case 'i':
- if (lflag)
- {
- long val = ARG_LONG (argc, argv);
- if (val < 0)
- {
- val = -val; /* does not work for MINLONG */
- sign = '-';
- }
- else
- sign = plus;
- s = ulong_to_str ((unsigned long) val, str, 10, digits);
- }
- else
- {
- int val = ARG_INT (argc, argv);
- if (hflag)
- val = (short) val;
- if (val < 0)
- {
- val = -val; /* does not work for MININT */
- sign = '-';
- }
- else
- sign = plus;
- s = ulong_to_str ((unsigned long) val, str, 10, digits);
- }
- if (zeropad)
- lpad = width - LENGTH (s) - HAS_SIGN;
- break;
-
- case 'o':
- if (lflag)
- {
- unsigned long val = ARG_ULONG (argc, argv);
- s = ulong_to_str ((unsigned long) val, str, 8, digits);
- }
- else
- {
- unsigned int val = ARG_UINT (argc, argv);
- if (hflag)
- val = (unsigned short) val;
- s = ulong_to_str ((unsigned long) val, str, 8, digits);
- }
- if (alternate)
- prefix = "0";
- if (zeropad)
- lpad = width - LENGTH (s) - alternate;
- break;
-
- case 'x':
- case 'X':
- if (lflag)
- {
- unsigned long val = ARG_ULONG (argc, argv);
- s = ulong_to_str ((unsigned long) val, str, 16,
- (fc == 'x') ? digits : Digits);
- }
- else
- {
- unsigned int val = ARG_UINT (argc, argv);
- if (hflag)
- val = (unsigned short) val;
- s = ulong_to_str ((unsigned long) val, str, 16,
- (fc == 'x') ? digits : Digits);
- }
- if (alternate)
- prefix = (fc == 'X') ? "0X" : "0x";
- if (zeropad)
- lpad = width - LENGTH (s) - 2*alternate;
- break;
-
- case 'u':
- if (lflag)
- {
- unsigned long val = ARG_ULONG (argc, argv);
- s = ulong_to_str ((unsigned long) val, str, 10, digits);
- }
- else
- {
- unsigned int val = ARG_UINT (argc, argv);
- if (hflag)
- val = (unsigned short) val;
- s = ulong_to_str ((unsigned long) val, str, 10, digits);
- }
- if (zeropad)
- lpad = width - LENGTH (s);
- break;
-
- case 'e':
- case 'E':
- {
- char *t;
- int sgn, decpt, exp, n;
- double val = ARG_DOUBLE (argc, argv);
-
- if (prec < 0)
- prec = 6;
- t = clr0 (ecvt (val, min (prec + 1, ECVTMAX), &decpt, &sgn));
- sign = sgn ? '-' : plus;
-
- n = prec;
- s = str;
- exp = (t[0] == '0' && t[1] == '\0') ? 0 : decpt - 1;
-
- *s++ = *t++;
- if (n > 0 || alternate)
- *s++ = '.';
- while (*t != '\0' && --n >= 0)
- *s++ = *t++;
- *s = '\0';
- rpad = n;
-
- sgn = 0;
- if (exp < 0)
- {
- exp = -exp;
- sgn = 1;
- }
- t = ulong_to_str ((unsigned long) exp, str, 10, digits);
- if (exp < 10)
- *--t = '0'; /* always at least two digits */
- *--t = sgn ? '-' : '+';
- *--t = fc;
-
- if (zeropad)
- {
- lpad = width - HAS_SIGN - (s - str) - LENGTH (t);
- if (rpad > 0)
- lpad -= rpad;
- }
-
- suffix = t;
- s = str;
- }
- break;
-
- case 'f':
- {
- const char *t;
- int sgn, decpt, n;
- double val = ARG_DOUBLE (argc, argv);
-
- if (prec < 0)
- prec = 6;
-
- /* FIXME: For the following line, Dave Anglin reports
- ``warning: passing arg 1 of `clr0' discards `const' from
- pointer target type''. I suspect fcvt might be declared
- as returning const on some systems. Pouah! I should
- revise this whole module, one of these days... */
-
- t = clr0 (fcvt (val, min (prec, FCVTMAX), &decpt, &sgn));
-
- sign = sgn ? '-' : plus;
-
- n = prec;
- s = str;
-
- if (decpt <= 0)
- {
- prefix = (n > 0 || alternate) ? "0." : "0";
- lpad = min (-decpt, prec);
- n -= lpad;
- }
- else
- {
- while (--decpt >= 0)
- *s++ = *t++;
- if (n > 0 || alternate)
- *s++ = '.';
- }
- while (*t && --n >= 0)
- *s++ = *t++;
-
- *s = '\0';
- rpad = n;
-
- if (zeropad)
- ppad = width - HAS_SIGN - (prefix[1] ? 2 : 1) - lpad -
- (s - str) - rpad;
-
- s = str;
- }
- break;
-
- default:
- continue;
- }
-
- if (lpad < 0)
- lpad = 0;
- if (rpad < 0)
- rpad = 0;
- if (width < 0)
- width = 0;
-
- i = strlen (s);
- if (maxch <= 0 || maxch > i)
- maxch = i;
-
- length = (HAS_SIGN + ppad + strlen (prefix) + lpad + maxch
- + rpad + strlen (suffix));
- padding = 0;
- if (width != 0)
- {
- padding = width - length;
- }
-
- if (ljust == 0) /* left padding */
- for (i = padding; --i >= 0;)
- obstack_1grow (obs, ' ');
- if (HAS_SIGN) /* sign */
- obstack_1grow (obs, sign);
- for (i = ppad; --i >= 0;) /* pre-prefix zero padding */
- obstack_1grow (obs, '0');
- for (; *prefix; ++prefix) /* prefix */
- obstack_1grow (obs, *prefix);
- for (i = lpad; --i >= 0;) /* left zero padding */
- obstack_1grow (obs, '0');
- for (i = maxch; --i >= 0; ++s) /* actual text */
- obstack_1grow (obs, *s);
- for (i = rpad; --i >= 0;) /* right zero padding */
- obstack_1grow (obs, '0');
- for (; *suffix; ++suffix) /* suffix */
- obstack_1grow (obs, *suffix);
- if (ljust != 0) /* right padding */
- for (i = padding; --i >= 0;)
- obstack_1grow (obs, ' ');
- }
-
-#else /* not HAVE_EFGCVT */
-
char *fmt; /* format control string */
const char *fstart; /* beginning of current format spec */
int c; /* a simple character */
@@ -533,7 +74,7 @@ format (struct obstack *obs, int argc, t
char hflag; /* short flag */
/* Buffer and stuff. */
- char str[256]; /* buffer for formatted text */
+ char *str; /* malloc'd buffer of formatted text */
enum {INT, UINT, LONG, ULONG, DOUBLE, STR} datatype;
fmt = ARG_STR (argc, argv);
@@ -673,75 +214,79 @@ format (struct obstack *obs, int argc, t
{
case INT:
if (width != -1 && prec != -1)
- sprintf (str, fstart, width, prec, ARG_INT(argc, argv));
+ str = xasprintf (fstart, width, prec, ARG_INT(argc, argv));
else if (width != -1)
- sprintf (str, fstart, width, ARG_INT(argc, argv));
+ str = xasprintf (fstart, width, ARG_INT(argc, argv));
else if (prec != -1)
- sprintf (str, fstart, prec, ARG_INT(argc, argv));
+ str = xasprintf (fstart, prec, ARG_INT(argc, argv));
else
- sprintf (str, fstart, ARG_INT(argc, argv));
+ str = xasprintf (fstart, ARG_INT(argc, argv));
break;
case UINT:
if (width != -1 && prec != -1)
- sprintf (str, fstart, width, prec, ARG_UINT(argc, argv));
+ str = xasprintf (fstart, width, prec, ARG_UINT(argc, argv));
else if (width != -1)
- sprintf (str, fstart, width, ARG_UINT(argc, argv));
+ str = xasprintf (fstart, width, ARG_UINT(argc, argv));
else if (prec != -1)
- sprintf (str, fstart, prec, ARG_UINT(argc, argv));
+ str = xasprintf (fstart, prec, ARG_UINT(argc, argv));
else
- sprintf (str, fstart, ARG_UINT(argc, argv));
+ str = xasprintf (fstart, ARG_UINT(argc, argv));
break;
case LONG:
if (width != -1 && prec != -1)
- sprintf (str, fstart, width, prec, ARG_LONG(argc, argv));
+ str = xasprintf (fstart, width, prec, ARG_LONG(argc, argv));
else if (width != -1)
- sprintf (str, fstart, width, ARG_LONG(argc, argv));
+ str = xasprintf (fstart, width, ARG_LONG(argc, argv));
else if (prec != -1)
- sprintf (str, fstart, prec, ARG_LONG(argc, argv));
+ str = xasprintf (fstart, prec, ARG_LONG(argc, argv));
else
- sprintf (str, fstart, ARG_LONG(argc, argv));
+ str = xasprintf (fstart, ARG_LONG(argc, argv));
break;
case ULONG:
if (width != -1 && prec != -1)
- sprintf (str, fstart, width, prec, ARG_ULONG(argc, argv));
+ str = xasprintf (fstart, width, prec, ARG_ULONG(argc, argv));
else if (width != -1)
- sprintf (str, fstart, width, ARG_ULONG(argc, argv));
+ str = xasprintf (fstart, width, ARG_ULONG(argc, argv));
else if (prec != -1)
- sprintf (str, fstart, prec, ARG_ULONG(argc, argv));
+ str = xasprintf (fstart, prec, ARG_ULONG(argc, argv));
else
- sprintf (str, fstart, ARG_ULONG(argc, argv));
+ str = xasprintf (fstart, ARG_ULONG(argc, argv));
break;
case DOUBLE:
if (width != -1 && prec != -1)
- sprintf (str, fstart, width, prec, ARG_DOUBLE(argc, argv));
+ str = xasprintf (fstart, width, prec, ARG_DOUBLE(argc, argv));
else if (width != -1)
- sprintf (str, fstart, width, ARG_DOUBLE(argc, argv));
+ str = xasprintf (fstart, width, ARG_DOUBLE(argc, argv));
else if (prec != -1)
- sprintf (str, fstart, prec, ARG_DOUBLE(argc, argv));
+ str = xasprintf (fstart, prec, ARG_DOUBLE(argc, argv));
else
- sprintf (str, fstart, ARG_DOUBLE(argc, argv));
+ str = xasprintf (fstart, ARG_DOUBLE(argc, argv));
break;
case STR:
if (width != -1 && prec != -1)
- sprintf (str, fstart, width, prec, ARG_STR(argc, argv));
+ str = xasprintf (fstart, width, prec, ARG_STR(argc, argv));
else if (width != -1)
- sprintf (str, fstart, width, ARG_STR(argc, argv));
+ str = xasprintf (fstart, width, ARG_STR(argc, argv));
else if (prec != -1)
- sprintf (str, fstart, prec, ARG_STR(argc, argv));
+ str = xasprintf (fstart, prec, ARG_STR(argc, argv));
else
- sprintf (str, fstart, ARG_STR(argc, argv));
+ str = xasprintf (fstart, ARG_STR(argc, argv));
break;
}
*fmt = c;
+ /* NULL was returned on failure, such as invalid format string. For
+ now, just silently ignore that bad specifier. */
+ if (str == NULL)
+ continue;
+
obstack_grow (obs, str, strlen (str));
+ free (str);
}
-
-#endif /* not HAVE_EFGCVT */
}
Index: m4/gnulib-cache.m4
===================================================================
RCS file: /sources/m4/m4/m4/Attic/gnulib-cache.m4,v
retrieving revision 1.1.2.2
diff -u -p -r1.1.2.2 gnulib-cache.m4
--- m4/gnulib-cache.m4 22 Jun 2006 14:55:48 -0000 1.1.2.2
+++ m4/gnulib-cache.m4 30 Jun 2006 03:34:46 -0000
@@ -15,10 +15,10 @@
# Specification in the form of a command-line invocation:
-# gnulib-tool --import --dir=. --lib=libm4 --source-base=lib --m4-base=m4
--aux-dir=. --macro-prefix=M4 alloca error getopt mkstemp obstack regex strtol
xalloc
+# gnulib-tool --import --dir=. --lib=libm4 --source-base=lib --m4-base=m4
--aux-dir=. --macro-prefix=M4 alloca error getopt mkstemp obstack regex strtol
xalloc xvasprintf
# Specification in the form of a few gnulib-tool.m4 macro invocations:
-gl_MODULES([alloca error getopt mkstemp obstack regex strtol xalloc])
+gl_MODULES([alloca error getopt mkstemp obstack regex strtol xalloc
xvasprintf])
gl_AVOID([])
gl_SOURCE_BASE([lib])
gl_M4_BASE([m4])