This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: vfprintf typing problem


From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 01 Apr 2012 00:13:53 -0700

> Instead, I suggest something like this:
 ...

Ok.

> Also, please adjust the other seven calls to read_int
> (in printf-parsemb.c and in vfprintf.c) to adjust
> to read_int's new behavior.  This will catch bad formats
> with outlandishly large values before '$', which is yet another
> bug in this area.

The printf parser interface has no mechanism to signal errors,
therfore I've taken the policy to skip constructs in the
format string which have overflows.

Otherwise this is updated as you've asked.

Comments?

2012-04-02  David S. Miller  <davem@davemloft.net>

	With help from Paul Eggert and Carlos O'Donell.
	* stdio-common/printf-parse.h (read_int): Check for overflows more
	carefully.
	* stdio-common/printf-parsemb.c: If read_int signals an overflow,
	skip the construct in the format string but do not record anything.
	* stdio-common/vfprintf.c (vfprintf): Adjust all read_int() callers
	to handle overflow correctly.

diff --git a/stdio-common/printf-parse.h b/stdio-common/printf-parse.h
index bcf914b..0123717 100644
--- a/stdio-common/printf-parse.h
+++ b/stdio-common/printf-parse.h
@@ -71,17 +71,26 @@ union printf_arg
 static int
 read_int (const UCHAR_T * *pstr)
 {
-  unsigned int retval = **pstr - L_('0');
+  int retval = **pstr - L_('0');
 
   while (ISDIGIT (*++(*pstr)))
-    {
-      retval *= 10;
-      if (retval > INT_MAX)
-	return -1;
-      retval += **pstr - L_('0');
-    }
-
-  return (int) retval;
+    if (0 <= retval)
+      {
+	if (INT_MAX / 10 < retval)
+	  retval = -1;
+	else
+	  {
+	    int digit = **pstr - L_('0');
+
+	    retval *= 10;
+	    if (INT_MAX - digit < retval)
+	      retval = -1;
+	    else
+	      retval += digit;
+	  }
+      }
+
+  return retval;
 }
 #endif
 
diff --git a/stdio-common/printf-parsemb.c b/stdio-common/printf-parsemb.c
index 2bdb5e6..a45ac74 100644
--- a/stdio-common/printf-parsemb.c
+++ b/stdio-common/printf-parsemb.c
@@ -87,12 +87,15 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
 
       n = read_int (&format);
 
-      if (n > 0 && *format == L_('$'))
+      if (n != 0 && *format == L_('$'))
 	/* Is positional parameter.  */
 	{
 	  ++format;		/* Skip the '$'.  */
-	  spec->data_arg = n - 1;
-	  *max_ref_arg = MAX (*max_ref_arg, n);
+	  if (n != -1)
+	    {
+	      spec->data_arg = n - 1;
+	      *max_ref_arg = MAX (*max_ref_arg, n);
+	    }
 	}
       else
 	/* Oops; that was actually the width and/or 0 padding flag.
@@ -160,10 +163,13 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
 	  /* The width argument might be found in a positional parameter.  */
 	  n = read_int (&format);
 
-	  if (n > 0 && *format == L_('$'))
+	  if (n != 0 && *format == L_('$'))
 	    {
-	      spec->width_arg = n - 1;
-	      *max_ref_arg = MAX (*max_ref_arg, n);
+	      if (n != -1)
+		{
+		  spec->width_arg = n - 1;
+		  *max_ref_arg = MAX (*max_ref_arg, n);
+		}
 	      ++format;		/* Skip '$'.  */
 	    }
 	}
@@ -177,9 +183,13 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
 	}
     }
   else if (ISDIGIT (*format))
-    /* Constant width specification.  */
-    spec->info.width = read_int (&format);
+    {
+      int n = read_int (&format);
 
+      /* Constant width specification.  */
+      if (n != -1)
+	spec->info.width = n;
+    }
   /* Get the precision.  */
   spec->prec_arg = -1;
   /* -1 means none given; 0 means explicit 0.  */
@@ -196,10 +206,13 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
 	    {
 	      n = read_int (&format);
 
-	      if (n > 0 && *format == L_('$'))
+	      if (n != 0 && *format == L_('$'))
 		{
-		  spec->prec_arg = n - 1;
-		  *max_ref_arg = MAX (*max_ref_arg, n);
+		  if (n != -1)
+		    {
+		      spec->prec_arg = n - 1;
+		      *max_ref_arg = MAX (*max_ref_arg, n);
+		    }
 		  ++format;
 		}
 	    }
@@ -213,7 +226,12 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
 	    }
 	}
       else if (ISDIGIT (*format))
-	spec->info.prec = read_int (&format);
+	{
+	  int n = read_int (&format);
+
+	  if (n != -1)
+	    spec->info.prec = n;
+	}
       else
 	/* "%.?" is treated like "%.0?".  */
 	spec->info.prec = 0;
diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index 4287291..9d443b0 100644
--- a/stdio-common/vfprintf.c
+++ b/stdio-common/vfprintf.c
@@ -1440,10 +1440,21 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 	const UCHAR_T *tmp;	/* Temporary value.  */
 
 	tmp = ++f;
-	if (ISDIGIT (*tmp) && read_int (&tmp) && *tmp == L_('$'))
-	  /* The width comes from a positional parameter.  */
-	  goto do_positional;
+	if (ISDIGIT (*tmp))
+	  {
+	    int pos = read_int (&tmp);
 
+	    if (pos == -1)
+	      {
+		__set_errno (ERANGE);
+		done = -1;
+		goto all_done;
+	      }
+
+	    if (pos && *tmp == L_('$'))
+	      /* The width comes from a positional parameter.  */
+	      goto do_positional;
+	  }
 	width = va_arg (ap, int);
 
 	/* Negative width means left justified.  */
@@ -1524,10 +1535,21 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 	  const UCHAR_T *tmp;	/* Temporary value.  */
 
 	  tmp = ++f;
-	  if (ISDIGIT (*tmp) && read_int (&tmp) > 0 && *tmp == L_('$'))
-	    /* The precision comes from a positional parameter.  */
-	    goto do_positional;
+	  if (ISDIGIT (*tmp))
+	    {
+	      int pos = read_int (&tmp);
 
+	      if (pos == -1)
+		{
+		  __set_errno (ERANGE);
+		  done = -1;
+		  goto all_done;
+		}
+
+	      if (pos && *tmp == L_('$'))
+		/* The precision comes from a positional parameter.  */
+		goto do_positional;
+	    }
 	  prec = va_arg (ap, int);
 
 	  /* If the precision is negative the precision is omitted.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]