GNU bug report logs - #8668
* editfns.c (Fformat): Fix several integer overflow problems.

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Paul Eggert <eggert <at> cs.ucla.edu>
Subject: bug#8668: closed (fixes committed to trunk)
Date: Fri, 27 May 2011 20:41:02 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#8668: * editfns.c (Fformat): Fix several integer overflow problems.

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 8668 <at> debbugs.gnu.org.

-- 
8668: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8668
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 8722-done <at> debbugs.gnu.org, 8719-done <at> debbugs.gnu.org, 
	8668-done <at> debbugs.gnu.org
Subject: fixes committed to trunk
Date: Fri, 27 May 2011 13:40:10 -0700

I just committed bzr 104390 to the Emacs trunk.
It merges fixes for bugs 8668, 8719, and 8722 as
previously discussed.

For Bug#8719, although the patch should fix the problems
it may not be the best patch.  Kenichi Handa wrote that
he'll check it.  Kenichi, if there are problems with it,
please feel free to replace it with something better
or to send me email with suggestions and I'll work on
making it better.  Thanks.

[Message part 3 (message/rfc822, inline)]

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: * editfns.c (Fformat): Fix several integer overflow problems.
Date: Thu, 12 May 2011 20:01:52 -0700

Here's a patch for several integer overflow problems in (format ...),
including some that cause core dumps.  I plan to install this into
the trunk after some more testing.

* editfns.c (Fformat): Fix several integer overflow problems.
For example, without this change, (format "%2147483648d" 1) dumps
core on x86-64 GNU/Linux.  Use EMACS_INT, not size_t, for sizes,
since we prefer using signed values, and EMACS_INT will be big
enough soon, even on 32-bit hosts.  Also, prefer EMACS_INT to int
for sizes.  Don't assume that pI is either "l" or ""; it might be
"ll" or "I64".  Check for width and precision greater than
INT_MAX, as this can make sprintf go kaflooey.
=== modified file 'src/editfns.c'
--- src/editfns.c	2011-04-14 19:34:42 +0000
+++ src/editfns.c	2011-05-13 02:30:06 +0000
@@ -3583,11 +3583,12 @@
 usage: (format STRING &rest OBJECTS)  */)
   (size_t nargs, register Lisp_Object *args)
 {
-  register size_t n;		/* The number of the next arg to substitute */
-  register size_t total;	/* An estimate of the final length */
+  register EMACS_INT n;		/* The number of the next arg to substitute */
+  register EMACS_INT total;	/* An estimate of the final length */
+  int pIlen = sizeof pI - 1;
   char *buf, *p;
   register char *format, *end, *format_start;
-  int nchars;
+  EMACS_INT nchars;
   /* Nonzero if the output should be a multibyte string,
      which is true if any of the inputs is one.  */
   int multibyte = 0;
@@ -3603,7 +3604,7 @@
      no argument, *will* be assigned to in the case that a `%' and `.'
      occur after the final format specifier.  */
   int *precision = (int *) (alloca ((nargs + 1) * sizeof (int)));
-  int longest_format;
+  EMACS_INT longest_format;
   Lisp_Object val;
   int arg_intervals = 0;
   USE_SAFE_ALLOCA;
@@ -3619,7 +3620,8 @@
      info[0] is unused.  Unused elements have -1 for start.  */
   struct info
   {
-    int start, end, intervals;
+    EMACS_INT start, end;
+    int intervals;
   } *info = 0;

   /* It should not be necessary to GCPRO ARGS, because
@@ -3660,8 +3662,8 @@

   /* Allocate the info and discarded tables.  */
   {
-    size_t nbytes = (nargs+1) * sizeof *info;
-    size_t i;
+    EMACS_INT nbytes = (nargs + 1) * sizeof *info;
+    EMACS_INT i;
     if (!info)
       info = (struct info *) alloca (nbytes);
     memset (info, 0, nbytes);
@@ -3706,25 +3708,33 @@
 		   || * format == ' ' || *format == '+'))
 	  ++format;

+	/* Parse width and precision, limiting them to the range of 'int'
+	   because otherwise the underyling sprintf may go kaflooey.  */
+
 	if (*format >= '0' && *format <= '9')
 	  {
-	    for (field_width = 0; *format >= '0' && *format <= '9'; ++format)
-	      field_width = 10 * field_width + *format - '0';
+	    char *width_end;
+	    unsigned long width = strtoul (format, &width_end, 10);
+	    if (INT_MAX < width)
+	      error ("Format string field width too large");
+	    field_width = width;
+	    format = width_end;
 	  }

 	/* N is not incremented for another few lines below, so refer to
 	   element N+1 (which might be precision[NARGS]). */
 	if (*format == '.')
 	  {
-	    ++format;
-	    for (precision[n+1] = 0; *format >= '0' && *format <= '9'; ++format)
-	      precision[n+1] = 10 * precision[n+1] + *format - '0';
+	    char *prec_end;
+	    unsigned long prec = strtoul (format + 1, &prec_end, 10);
+	    if (INT_MAX < prec)
+	      error ("Format string precision too large");
+	    precision[n + 1] = prec;
+	    format = prec_end;
 	  }

-	/* Extra +1 for 'l' that we may need to insert into the
-	   format.  */
-	if (format - this_format_start + 2 > longest_format)
-	  longest_format = format - this_format_start + 2;
+	if (longest_format < format - this_format_start + pIlen + 1)
+	  longest_format = format - this_format_start + pIlen + 1;

 	if (format == end)
 	  error ("Format string ends in middle of format specifier");
@@ -3975,24 +3985,22 @@
 	    }
 	  else if (INTEGERP (args[n]) || FLOATP (args[n]))
 	    {
-	      int this_nchars;
+	      EMACS_INT this_nchars;
+	      EMACS_INT this_format_len = format - this_format_start;

-	      memcpy (this_format, this_format_start,
-		      format - this_format_start);
-	      this_format[format - this_format_start] = 0;
+	      memcpy (this_format, this_format_start, this_format_len);
+	      this_format[this_format_len] = 0;

 	      if (format[-1] == 'e' || format[-1] == 'f' || format[-1] == 'g')
 		sprintf (p, this_format, XFLOAT_DATA (args[n]));
 	      else
 		{
-		  if (sizeof (EMACS_INT) > sizeof (int)
-		      && format[-1] != 'c')
+		  if (pIlen && format[-1] != 'c')
 		    {
-		      /* Insert 'l' before format spec.  */
-		      this_format[format - this_format_start]
-			= this_format[format - this_format_start - 1];
-		      this_format[format - this_format_start - 1] = 'l';
-		      this_format[format - this_format_start + 1] = 0;
+		      /* Insert pI before format spec.  */
+		      memcpy (&this_format[this_format_len - 1], pI, pIlen);
+		      this_format[this_format_len + pIlen - 1] = format[-1];
+		      this_format[this_format_len + pIlen] = 0;
 		    }

 		  if (INTEGERP (args[n]))
@@ -4089,7 +4097,7 @@
       if (CONSP (props))
 	{
 	  EMACS_INT bytepos = 0, position = 0, translated = 0;
-	  int argn = 1;
+	  EMACS_INT argn = 1;
 	  Lisp_Object list;

 	  /* Adjust the bounds of each text property

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.