head: Re: branch-1_4 allow cross-compiles; expose buffer overrun

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 6/29/2006 9:36 PM:
> $ M4_cv_have_efgcvt=no configure
> $ make
> ...
> $ echo 'format(%300d,1)'|src/m4
> Segmentation fault (core dumped)
>
> 2006-06-29  Eric Blake  <address@hidden>
> 
>       Fix buffer overrun bug.
>       * m4/gnulib-cache.m4: Augment with gnulib-tool --import
>       xvasprintf.
...

forward ported to head (where the buffer overflow was at 4096, instead of
256, characters; revision 1.9 of modules/format.c, in 2001).  It looks
like m4-1.4o (now 6 years old, but the last released snapshot of head) was
still using ecvt when possible, so the buffer overflow wasn't on all
platforms until revision 1.6 of modules/format.c (Aug 2001).

There is still a lot of work I would like to do on format.c, such as
issuing decent warnings on bad input, or recognizing newer modifiers like
%Lg or %j, but my first course of business is forward-porting all the
fixes from the branch.

2006-07-17  Eric Blake  <address@hidden>

        * ltdl/m4/gnulib-cache.m4: Augment with `gnulib-tool --import
        xvasprintf'.
        * modules/format.c (includes): Use xvasprintf.h.
        (format): Make static.  Avoid buffer overflow, that can lead to
        arbitrary code execution exploit.  Only pass unsigned char to
        is*().  Support F, g, and G specifiers.
        * doc/m4.texinfo (Format): Expose buffer overrun bug.  Document
        new specifiers.

- --
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

iD8DBQFEu4/G84KuGfSFAYARAkVHAJ9pLs5V+T/EMv/K5lnFMuBSg/HrLgCgsQiK
4OiiJ/TCGVsgSRy1jZel0JU=
=xJGh
-----END PGP SIGNATURE-----

? ltdl/m4/eoverflow.m4
? ltdl/m4/intmax_t.m4
? ltdl/m4/stdarg.m4
? ltdl/m4/vasnprintf.m4
? ltdl/m4/vasprintf.m4
? ltdl/m4/xvasprintf.m4
Index: doc/m4.texinfo
===================================================================
RCS file: /sources/m4/m4/doc/m4.texinfo,v
retrieving revision 1.26
diff -u -p -r1.26 m4.texinfo
--- doc/m4.texinfo      15 Jul 2006 21:35:37 -0000      1.26
+++ doc/m4.texinfo      17 Jul 2006 13:18:46 -0000
@@ -3464,6 +3464,10 @@ define(`foo', `The brown fox jumped over
 @result{}
 format(`The string "%s" uses %d characters', foo, len(foo))
 @result{}The string "The brown fox jumped over the lazy dog" uses 38 characters
+format(`%.0f', `56789.9876')
address@hidden
+len(format(`%-*X', `5000', `1'))
address@hidden
 @end example
 
 Using the @code{forloop} macro defined in @xref{Loops}, this
@@ -3488,11 +3492,14 @@ forloop(`i', 1, 10, `format(`%6d squared
 The builtin @code{format} is modeled after the ANSI C @samp{printf}
 function, and supports the normal @samp{%} specifiers: @samp{c},
 @samp{s}, @samp{d}, @samp{o}, @samp{x}, @samp{X}, @samp{u}, @samp{e},
address@hidden and @samp{f}; it supports field widths and precisions, and the
address@hidden, @samp{f}, @samp{F}, @samp{g}, and @samp{G}; it supports
+field widths and precisions, and the
 modifiers @samp{+}, @samp{-}, @address@hidden }}, @samp{0}, @samp{#}, @samp{h} 
and
 @samp{l}.  For more details on the functioning of @code{printf}, see the
 C Library Manual.
 
address@hidden FIXME - format still needs some improvements.
+
 @node Arithmetic
 @chapter Macros for doing arithmetic
 
Index: ltdl/m4/gnulib-cache.m4
===================================================================
RCS file: /sources/m4/m4/ltdl/m4/gnulib-cache.m4,v
retrieving revision 1.4
diff -u -p -r1.4 gnulib-cache.m4
--- ltdl/m4/gnulib-cache.m4     15 Jul 2006 21:35:37 -0000      1.4
+++ ltdl/m4/gnulib-cache.m4     17 Jul 2006 13:18:46 -0000
@@ -15,10 +15,10 @@
 
 
 # Specification in the form of a command-line invocation:
-#   gnulib-tool --import --dir=. --lib=libgnu --source-base=gnu 
--m4-base=ltdl/m4 --doc-base=doc --aux-dir=ltdl/config --libtool 
--macro-prefix=M4 assert error exit fdl free gendocs gettext mkstemp obstack 
progname regex stdbool strtol xalloc xalloc-die xstrndup
+#   gnulib-tool --import --dir=. --lib=libgnu --source-base=gnu 
--m4-base=ltdl/m4 --doc-base=doc --aux-dir=ltdl/config --libtool 
--macro-prefix=M4 assert error exit fdl free gendocs gettext mkstemp obstack 
progname regex stdbool strtol xalloc xalloc-die xstrndup xvasprintf
 
 # Specification in the form of a few gnulib-tool.m4 macro invocations:
-gl_MODULES([assert error exit fdl free gendocs gettext mkstemp obstack 
progname regex stdbool strtol xalloc xalloc-die xstrndup])
+gl_MODULES([assert error exit fdl free gendocs gettext mkstemp obstack 
progname regex stdbool strtol xalloc xalloc-die xstrndup xvasprintf])
 gl_AVOID([])
 gl_SOURCE_BASE([gnu])
 gl_M4_BASE([ltdl/m4])
Index: modules/format.c
===================================================================
RCS file: /sources/m4/m4/modules/format.c,v
retrieving revision 1.17
diff -u -p -r1.17 format.c
--- modules/format.c    1 May 2005 11:10:05 -0000       1.17
+++ modules/format.c    17 Jul 2006 13:18:46 -0000
@@ -1,5 +1,5 @@
 /* GNU m4 -- A simple macro processor
-   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 2001
+   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 2001, 2006
    Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
@@ -20,7 +20,13 @@
 
 /* printf like formatting for m4.  */
 
-/* Simple varargs substitute.  */
+#include "xvasprintf.h"
+
+/* Simple varargs substitute.
+   TODO - warn if we use these because too many % specifiers were used in
+   relation to number of arguments passed.
+   TODO - use xstrtoimax, not atoi, to catch overflow, non-numeric
+   arguments, etc.  */
 
 #define ARG_INT(argc, argv) \
        ((argc == 0) ? 0 : \
@@ -50,9 +56,8 @@
 /* The main formatting function.  Output is placed on the obstack OBS, the
    first argument in ARGV is the formatting string, and the rest is
    arguments for the string.  */
-void format (m4_obstack *obs, int argc, m4_symbol_value **argv);
 
-void
+static void
 format (m4_obstack *obs, int argc, m4_symbol_value **argv)
 {
   char *fmt;                   /* format control string */
@@ -69,7 +74,7 @@ format (m4_obstack *obs, int argc, m4_sy
   char hflag;                  /* short flag */
 
   /* Buffer and stuff.  */
-  char str[4096];              /* buffer for formatted text */
+  char *str;                   /* malloc'd buffer for formatted text */
   enum {INT, UINT, LONG, ULONG, DOUBLE, STR} datatype;
 
   fmt = ARG_STR (argc, argv);
@@ -77,7 +82,7 @@ format (m4_obstack *obs, int argc, m4_sy
     {
       while ((c = *fmt++) != '%')
        {
-         if (c == 0)
+         if (c == '\0')
            return;
          obstack_1grow (obs, c);
        }
@@ -92,6 +97,8 @@ format (m4_obstack *obs, int argc, m4_sy
        }
 
       /* Parse flags.  */
+      /* TODO - borrow ideas from coreutils printf(1) for argument validation,
+        and for supporting specifiers for larger types.  */
       flags = 1;
       do
        {
@@ -118,13 +125,13 @@ format (m4_obstack *obs, int argc, m4_sy
          width = ARG_INT (argc, argv);
          fmt++;
        }
-      else if (isdigit ((int) *fmt))
+      else if (isdigit ((unsigned char) *fmt))
        {
          do
            {
              fmt++;
            }
-         while (isdigit ((int) *fmt));
+         while (isdigit ((unsigned char) *fmt));
        }
 
       /* Maximum precision.  */
@@ -136,13 +143,13 @@ format (m4_obstack *obs, int argc, m4_sy
              prec = ARG_INT (argc, argv);
              ++fmt;
            }
-         else if (isdigit ((int) *fmt))
+         else if (isdigit ((unsigned char) *fmt))
            {
              do
                {
                  fmt++;
                }
-             while (isdigit ((int) *fmt));
+             while (isdigit ((unsigned char) *fmt));
            }
        }
 
@@ -156,6 +163,7 @@ format (m4_obstack *obs, int argc, m4_sy
        {
 
        case '\0':
+         /* TODO - warn about incomplete % specifier.  */
          return;
 
        case 'c':
@@ -195,6 +203,9 @@ format (m4_obstack *obs, int argc, m4_sy
        case 'e':
        case 'E':
        case 'f':
+       case 'F':
+       case 'g':
+       case 'G':
          datatype = DOUBLE;
          break;
 
@@ -209,73 +220,79 @@ format (m4_obstack *obs, int argc, m4_sy
        {
        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);
     }
 }