make 3.80 sprintf potential buffer overruns

[Paul Eggert]

I audited GNU make 3.80 for potential buffer overruns due to sprintf
calls, and came up with the following patch.  Most of the buffer
overruns are fairly theoretical: they can occur only on hosts where
integers are fairly wide.  Some of them are more practical, though:
they can occur on hosts that have 5-digit file descriptor numbers.

2003-10-06  Paul Eggert  <address@hidden>

        * arscan.c (ar_member_touch): Don't overrun buffer if a time stamp
        is so large that it doesn't fit into ar_hdr.ar_date.
        Report overflow instead.

        * function.c (func_words):
        Don't assume int can print in less than 20 bytes.
        (func_shell): Don't assume that line number can print in 11
        bytes or less.
        (func_call): Don't assume that int can print in 10 bytes or less.

        * main.c (main): Don't assume that file descriptors can be
        printed as integers in 4 bytes or less.  Don't assume that
        make level can be printed in less than about 30 bytes.
        (define_makeflags): Don't assume that unsigned can be printed
        in less than 30 bytes.  Don't assume that double %g prints in
        less than 100 bytes.

        * make.h (EOVERFLOW): Define if errno.h doesn't.
        (INT_STRLEN_BOUND, INT_BUFSIZE_BOUND): New macros, taken from gnulib.

        * misc.c (strerror): Don't refer to errno; that is bogus.
        Don't translate "Unknown error %d", as the translation can
        overflow the static buffer.  Don't assume that ints can be
        printed in 20 bytes or less.

        * signame.c (strsignal):
        Don't assume that ints can be printed in 20 bytes or less.

        * variable.c (define_automatic_variables): Don't assume that
        version number + "Customs" fits in 200 bytes.  Don't assume
        that int can be printed in less than 200 bytes.
        (target_environment): Don't assume that double %g prints in less than
        100 bytes.

===================================================================
RCS file: arscan.c,v
retrieving revision 3.80
retrieving revision 3.80.0.1
diff -pu -r3.80 -r3.80.0.1
--- arscan.c    2001/06/01 03:56:50     3.80
+++ arscan.c    2003/10/06 06:53:34     3.80.0.1
@@ -777,6 +777,7 @@ ar_member_touch (arname, memname)
   struct ar_hdr ar_hdr;
   register int i;
   struct stat statbuf;
+  char mtime_buf[INT_BUFSIZE_BOUND (statbuf.st_mtime)];
 
   if (pos < 0)
     return (int) pos;
@@ -803,7 +804,16 @@ ar_member_touch (arname, memname)
   /* Advance member's time to that time */
   for (i = 0; i < sizeof ar_hdr.ar_date; i++)
     ar_hdr.ar_date[i] = ' ';
-  sprintf (ar_hdr.ar_date, "%ld", (long int) statbuf.st_mtime);
+  if (statbuf.st_mtime < 0)
+    sprintf (mtime_buf, "%ld", (long int) statbuf.st_mtime);
+  else
+    sprintf (mtime_buf, "%lu", (unsigned long int) statbuf.st_mtime);
+  if (sizeof ar_hdr.ar_date <= strlen (mtime_buf))
+    {
+      errno = EOVERFLOW;
+      goto lose;
+    }
+  strcpy (ar_hdr.ar_date, mtime_buf);
 #ifdef AIAMAG
   ar_hdr.ar_date[strlen(ar_hdr.ar_date)] = ' ';
 #endif
===================================================================
RCS file: function.c,v
retrieving revision 3.80
retrieving revision 3.80.0.1
diff -pu -r3.80 -r3.80.0.1
--- function.c  2002/10/04 02:13:42     3.80
+++ function.c  2003/10/06 06:53:34     3.80.0.1
@@ -704,7 +704,7 @@ func_words (o, argv, funcname)
 {
   int i = 0;
   char *word_iterator = argv[0];
-  char buf[20];
+  char buf[INT_BUFSIZE_BOUND (i)];
 
   while (find_next_token (&word_iterator, (unsigned int *) 0) != 0)
     ++i;
@@ -1531,7 +1531,9 @@ func_shell (o, argv, funcname)
   /* For error messages.  */
   if (reading_file != 0)
     {
-      error_prefix = (char *) alloca (strlen (reading_file->filenm)+11+4);
+      error_prefix = (char *) alloca (strlen (reading_file->filenm) + 1
+                                     + INT_STRLEN_BOUND (reading_file->lineno)
+                                     + 3);
       sprintf (error_prefix,
               "%s:%lu: ", reading_file->filenm, reading_file->lineno);
     }
@@ -2045,7 +2047,7 @@ func_call (o, argv, funcname)
 
   for (i=0; *argv; ++i, ++argv)
     {
-      char num[11];
+      char num[INT_BUFSIZE_BOUND (i)];
 
       sprintf (num, "%d", i);
       define_variable (num, strlen (num), *argv, o_automatic, 0);
===================================================================
RCS file: main.c,v
retrieving revision 3.80.0.1
retrieving revision 3.80.0.2
diff -pu -r3.80.0.1 -r3.80.0.2
--- main.c      2003/10/06 05:46:41     3.80.0.1
+++ main.c      2003/10/06 07:02:01     3.80.0.2
@@ -1573,7 +1573,8 @@ int main (int argc, char ** argv)
       jobserver_fds = (struct stringlist *)
                         xmalloc (sizeof (struct stringlist));
       jobserver_fds->list = (char **) xmalloc (sizeof (char *));
-      jobserver_fds->list[0] = xmalloc ((sizeof ("1024")*2)+1);
+      jobserver_fds->list[0] = xmalloc (INT_STRLEN_BOUND (job_fds[0]) + 1
+                                       + INT_STRLEN_BOUND (job_fds[1]) + 1);
 
       sprintf (jobserver_fds->list[0], "%d,%d", job_fds[0], job_fds[1]);
       jobserver_fds->idx = 1;
@@ -1852,14 +1853,16 @@ int main (int argc, char ** argv)
                   the concept of storing the result of a function
                   in something other than a local variable.  */
                char *sgi_loses;
-               sgi_loses = (char *) alloca (40);
+               sgi_loses = (char *) alloca (MAKELEVEL_LENGTH + 1
+                                            + INT_STRLEN_BOUND (makelevel)
+                                            + 1);
                *p = sgi_loses;
                sprintf (*p, "%s=%u", MAKELEVEL_NAME, makelevel);
                break;
              }
 #else /* AMIGA */
          {
-           char buffer[256];
+           char buffer[INT_BUFSIZE_BOUND (makelevel)];
            int len;
 
            len = GetVar (MAKELEVEL_NAME, buffer, sizeof (buffer), 
GVF_GLOBAL_ONLY);
@@ -2476,7 +2479,7 @@ define_makeflags (all, makefile)
                ADD_FLAG ("1", 1);
              else
                {
-                 char *buf = (char *) alloca (30);
+                 char *buf = (char *) alloca (INT_BUFSIZE_BOUND (unsigned));
                  sprintf (buf, "%u", *(unsigned int *) cs->value_ptr);
                  ADD_FLAG (buf, strlen (buf));
                }
@@ -2497,7 +2500,7 @@ define_makeflags (all, makefile)
                ADD_FLAG ("", 0); /* Optional value omitted; see below.  */
              else
                {
-                 char *buf = (char *) alloca (100);
+                 char *buf = (char *) alloca (100 + INT_STRLEN_BOUND (int));
                  sprintf (buf, "%g", *(double *) cs->value_ptr);
                  ADD_FLAG (buf, strlen (buf));
                }
===================================================================
RCS file: make.h,v
retrieving revision 3.80
retrieving revision 3.80.0.1
diff -pu -r3.80 -r3.80.0.1
--- make.h      2002/09/11 16:55:44     3.80
+++ make.h      2003/10/06 06:53:34     3.80.0.1
@@ -96,6 +96,10 @@ char *alloca ();
 extern int errno;
 #endif
 
+#ifndef EOVERFLOW
+# define EOVERFLOW EINVAL
+#endif
+
 #ifndef isblank
 # define isblank(c)     ((c) == ' ' || (c) == '\t')
 #endif
@@ -173,6 +177,13 @@ extern unsigned int get_path_max PARAMS 
 # define CHAR_MAX INTEGER_TYPE_MAXIMUM (char)
 #endif
 
+/* Upper bound on the string length of an integer converted to string.
+   302 / 1000 is ceil (log10 (2.0)).  Subtract 1 for the sign bit;
+   add 1 for integer division truncation; add 1 more for a minus sign.  */
+#define INT_STRLEN_BOUND(t) ((sizeof (t) * CHAR_BIT - 1) * 302 / 1000 + 2)
+
+#define INT_BUFSIZE_BOUND(t) (INT_STRLEN_BOUND (t) + 1)
+
 #ifdef STAT_MACROS_BROKEN
 # ifdef S_ISREG
 #  undef S_ISREG
===================================================================
RCS file: misc.c,v
retrieving revision 3.80
retrieving revision 3.80.0.1
diff -pu -r3.80 -r3.80.0.1
--- misc.c      2002/09/12 22:15:58     3.80
+++ misc.c      2003/10/06 06:53:34     3.80.0.1
@@ -319,16 +319,16 @@ char *
 strerror (errnum)
      int errnum;
 {
-  extern int errno, sys_nerr;
+  extern int sys_nerr;
 #ifndef __DECC
   extern char *sys_errlist[];
 #endif
-  static char buf[] = "Unknown error 12345678901234567890";
+  static char buf[sizeof "Unknown error " + INT_STRLEN_BOUND (errnum) + 1];
 
-  if (errno < sys_nerr)
+  if (0 < errnum && errnum < sys_nerr)
     return sys_errlist[errnum];
 
-  sprintf (buf, _("Unknown error %d"), errnum);
+  sprintf (buf, "Unknown error %d", errnum);
   return buf;
 }
 #endif
===================================================================
RCS file: signame.c,v
retrieving revision 3.80
retrieving revision 3.80.0.1
diff -pu -r3.80 -r3.80.0.1
--- signame.c   2002/09/18 04:35:52     3.80
+++ signame.c   2003/10/06 06:53:34     3.80.0.1
@@ -236,7 +236,7 @@ char *
 strsignal (signal)
      int signal;
 {
-  static char buf[] = "Signal 12345678901234567890";
+  static char buf[sizeof "Signal " + INT_STRLEN_BOUND (signal) + 1];
 
 #if !defined(SYS_SIGLIST_DECLARED)
   static char sig_initted = 0;
===================================================================
RCS file: variable.c,v
retrieving revision 3.80
retrieving revision 3.80.0.1
diff -pu -r3.80 -r3.80.0.1
--- variable.c  2002/10/04 02:13:42     3.80
+++ variable.c  2003/10/06 06:53:34     3.80.0.1
@@ -566,7 +566,8 @@ define_automatic_variables ()
   extern char default_shell[];
 #endif
   register struct variable *v;
-  char buf[200];
+  char buf[MAX (INT_BUFSIZE_BOUND (makelevel),
+               sizeof VERSION + sizeof "Customs")];
 
   sprintf (buf, "%u", makelevel);
   (void) define_variable (MAKELEVEL_NAME, MAKELEVEL_LENGTH, buf, o_env, 0);
@@ -781,7 +782,8 @@ target_environment (file)
          }
       }
 
-  *result = (char *) xmalloc (100);
+  *result = (char *) xmalloc (MAKELEVEL_LENGTH + 1
+                             + INT_STRLEN_BOUND (makelevel) + 1);
   (void) sprintf (*result, "%s=%u", MAKELEVEL_NAME, makelevel + 1);
   *++result = 0;