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 <[EMAIL PROTECTED]>
* 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;
_______________________________________________
Bug-make mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-make
