Bash 2.05 printf buffer overrun; also, length modifiers dump core

[Paul Eggert]

Configuration Information [Automatically generated, do not change]:
Machine: sparc
OS: solaris2.7
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='sparc' 
-DCONF_OSTYPE='solaris2.7' -DCONF_MACHTYPE='sparc-sun-solaris2.7' 
-DCONF_VENDOR='sun' -DSHELL  -DHAVE_CONFIG_H   -I.  -I. -I./include -I./lib 
-I/opt/reb/include -g -O2
uname output: SunOS shade.twinsun.com 5.8 Generic_108528-07 sun4u sparc 
SUNW,Ultra-1
Machine Type: sparc-sun-solaris2.7

Bash Version: 2.05
Patch Level: 0
Release Status: release

Description:
        Bash internally uses a 64-byte buffer for printf formats without
        checking for buffer overrun.  If the buffer is overrun, Bash
        can dump core.

        While fixing this bug, I also noticed that Bash mishandles
        length modifiers in some cases.  For example, the command
        'printf "%.Lg" 234234' dumps core.

Repeat-By:
        $ format='%'`printf '%0100000d' 0`'d\n'
        $ printf $format 0
        Segmentation Fault(coredump)

        The output should be '0'.

        $ printf '%Lg\n' 234234
        Segmentation Fault(coredump)

        The output should be '234234'.

Fix:

2001-04-27  Paul Eggert  <eggert@twinsun.com>

        * builtins/printf.def (PRETURN):
        Free converted format before returning.
        (SKIP2): Change from "list of characters that can precede '.'" to
        list of length modifiers.
        (printf_builtin): Allocate sufficient room for a modified conversion
        specifier.  Use it instead of relying on 'mklong'.
        Ignore length modifiers when constructing the modified conversion
        specifier; always print integers using long formats.
        (mklong): Remove; it suffered from buffer overrun problems, leading to
        core dumps (or worse).

===================================================================
RCS file: builtins/printf.def,v
retrieving revision 2.5.0.5
retrieving revision 2.5.0.6
diff -pu -r2.5.0.5 -r2.5.0.6
--- builtins/printf.def 2001/04/27 18:01:33     2.5.0.5
+++ builtins/printf.def 2001/04/27 18:56:41     2.5.0.6
@@ -74,15 +74,14 @@ extern int errno;
   } while (0)
 
 #define PRETURN(value) \
-  do { /* free (format); */ fflush (stdout); return (value); } while (0)
+  do { free (conv); fflush (stdout); return (value); } while (0)
 
 #define  SKIP1 "#'-+ 0"
-#define  SKIP2 "*0123456789"
+#define  SKIP2 "hlL"
 
 static void printstr __P((char *, char *, int, int, int));
 static int tescape __P((char *, int, char *, int *));
 static char *bexpand __P((char *, int, int *, int *));
-static char *mklong __P((char *, int));
 static int getchr __P((void));
 static char *getstr __P((void));
 static int  getint __P((void));
@@ -100,9 +99,9 @@ int
 printf_builtin (list)
      WORD_LIST *list;
 {
-  int ch, end, fieldwidth, precision, foundmod, fmtlen;
+  int ch, end, fieldwidth, precision;
   int have_fieldwidth, have_precision;
-  char convch, nextch, *format, *fmt, *start;
+  char convch, nextch, *format, *fmt, *start, *conv, *cv, *priconv;
 
   retval = EXECUTION_SUCCESS;
   reset_internal_getopt ();
@@ -134,6 +133,8 @@ printf_builtin (list)
   /* If the format string is empty after preprocessing, return immediately. */
   if (format == 0 || *format == 0)
     return (EXECUTION_SUCCESS);
+
+  conv = xmalloc (strlen (format) + sizeof ("ld") - 1);
          
   /* Basic algorithm is to scan the format string for conversion
      specifications -- once one is found, find out if the field
@@ -147,7 +148,7 @@ printf_builtin (list)
       /* find next format specification */
       for (fmt = format; *fmt; fmt++)
        {
-         precision = fieldwidth = foundmod = 0;
+         precision = fieldwidth = 0;
          have_fieldwidth = have_precision = 0;
 
          if (*fmt == '\\')
@@ -168,7 +169,9 @@ printf_builtin (list)
            }
 
          /* ASSERT(*fmt == '%') */
-         start = fmt++;
+         start = fmt;
+         cv = conv;
+         *cv++ = *fmt++;
 
          if (*fmt == '%')              /* %% prints a % */
            {
@@ -177,37 +180,37 @@ printf_builtin (list)
            }
 
          /* found format specification, skip to field width */
-         for (; *fmt && strchr(SKIP1, *fmt); ++fmt)
-           ;
+         while (*fmt && strchr (SKIP1, *fmt))
+           *cv++ = *fmt++;
+
+         /* skip to possible '.', get following precision */
          if (*fmt == '*')
            {
+             *cv++ = *fmt++;
              fieldwidth = getint ();
              have_fieldwidth = 1;
            }
+         else
+           while (isdigit (*fmt))
+             *cv++ = *fmt++;
 
-         /* skip to possible '.', get following precision */
-         for (; *fmt && strchr(SKIP2, *fmt); ++fmt)
-           ;
          if (*fmt == '.')
            {
-             ++fmt;
+             *cv++ = *fmt++;
              if (*fmt == '*')
                {
+                 *cv++ = *fmt++;
                  precision = getint ();
                  have_precision = 1;
                }
+             else
+               while (isdigit (*fmt))
+                 *cv++ = *fmt++;
            }
 
-         /* skip to conversion char */
-         for (; *fmt && strchr(SKIP2, *fmt); ++fmt)
-           ;
-
          /* skip possible format modifiers */
-         if (*fmt == 'l' || *fmt == 'L' || *fmt == 'h')
-           {
-             fmt++;
-             foundmod = 1;
-           }
+         while (*fmt && strchr (SKIP2, *fmt))
+           fmt++;
            
          if (*fmt == 0)
            {
@@ -216,8 +219,8 @@ printf_builtin (list)
            }
 
          convch = *fmt;
-         nextch = fmt[1];
-         fmt[1] = '\0';
+         *cv++ = convch;
+         *cv = '\0';
          switch(convch)
            {
            case 'c':
@@ -225,7 +228,7 @@ printf_builtin (list)
                char p;
 
                p = getchr ();
-               PF(start, p);
+               PF (conv, p);
                break;
              }
 
@@ -234,7 +237,7 @@ printf_builtin (list)
                char *p;
 
                p = getstr ();
-               PF(start, p);
+               PF (conv, p);
                break;
              }
 
@@ -251,7 +254,7 @@ printf_builtin (list)
                  {
                    /* Have to use printstr because of possible NUL bytes
                       in XP -- printf does not handle that well. */
-                   printstr (start, xp, rlen, fieldwidth, precision);
+                   printstr (conv, xp, rlen, fieldwidth, precision);
                    free (xp);
                  }
 
@@ -269,7 +272,7 @@ printf_builtin (list)
                if (xp)
                  {
                    /* Use printstr to get fieldwidth and precision right. */
-                   printstr (start, xp, strlen (xp), fieldwidth, precision);
+                   printstr (conv, xp, strlen (xp), fieldwidth, precision);
                    free (xp);
                  }
                break;
@@ -279,33 +282,35 @@ printf_builtin (list)
            case 'i':
              {
                long p;
-               char *f;
 
-               if (foundmod == 0 && ((f = mklong (start, convch)) == NULL))
-                 PRETURN (EXECUTION_FAILURE);
-               else
-                 f = start;
                if (getlong (&p))
                  PRETURN (EXECUTION_FAILURE);
-               PF(f, p);
+               strcpy (cv - 1, "ld");
+               PF (conv, p);
                break;
              }
 
            case 'o':
+             priconv = "lo";
+             goto unsigned_argument;
            case 'u':
+             priconv = "lu";
+             goto unsigned_argument;
            case 'x':
+             priconv = "lx";
+             goto unsigned_argument;
            case 'X':
+             priconv = "lX";
+             goto unsigned_argument;
+
+           unsigned_argument:
              {
                unsigned long p;
-               char *f;
 
-               if (foundmod == 0 && ((f = mklong (start, convch)) == NULL))
-                 PRETURN (EXECUTION_FAILURE);
-               else
-                 f = start;
                if (getulong (&p))
                  PRETURN (EXECUTION_FAILURE);
-               PF (f, p);
+               strcpy (cv - 1, priconv);
+               PF (conv, p);
                break;
              }
 
@@ -319,7 +324,7 @@ printf_builtin (list)
 
                if (getdouble (&p))
                  PRETURN (EXECUTION_FAILURE);
-               PF(start, p);
+               PF (conv, p);
                break;
              }
 
@@ -329,8 +334,6 @@ printf_builtin (list)
              builtin_error ("`%c': invalid format character", convch);
              PRETURN (EXECUTION_FAILURE);
            }
-
-         fmt[1] = nextch;
        }
     }
   while (garglist && garglist != list->next);
@@ -604,22 +607,6 @@ bexpand (string, len, sawc, lenp)
   return ret;
 }
 
-static char *
-mklong (str, ch)
-     char *str;
-     int ch;
-{
-  static char copy[64];
-  int len;
-
-  len = strlen (str) + 2;
-  FASTCOPY (str, copy, len - 3);
-  copy[len - 3] = 'l';
-  copy[len - 2] = ch;
-  copy[len - 1] = '\0';
-  return (copy);
-}
-
 static int
 getchr ()
 {