Patchwork avoid undefined behavior in libiberty/cplus-dem.c

login
register
mail settings
Submitter Nickolai Zeldovich
Date Jan. 7, 2013, 2:21 p.m.
Message ID <Pine.LNX.4.64.1301070918470.14717@escalante.csail.mit.edu>
Download mbox | patch
Permalink /patch/209933/
State New
Headers show

Comments

Nickolai Zeldovich - Jan. 7, 2013, 2:21 p.m.
On Mon, 7 Jan 2013, Jakub Jelinek wrote:
> Won't the above preclude parsing 2147483640 up to 2147483647 ?
> Because then in the last iteration count 214748364 > (INT_MAX - 9) / 10.

You're right -- thanks for catching that!  Below is a patch with a more 
precise check.

Nickolai.

Patch

--- libiberty/cplus-dem.c	(revision 194959)
+++ libiberty/cplus-dem.c	(working copy)
@@ -48,7 +48,18 @@ 
  #include <sys/types.h>
  #include <string.h>
  #include <stdio.h>
+#ifdef HAVE_LIMITS_H
+#include <limits.h>
+#endif

+#ifndef UINT_MAX
+#define UINT_MAX	((unsigned int)(~0U))		/* 0xFFFFFFFF */
+#endif
+
+#ifndef INT_MAX
+#define INT_MAX		((int)(UINT_MAX >> 1))		/* 0x7FFFFFFF */
+#endif
+
  #ifdef HAVE_STDLIB_H
  #include <stdlib.h>
  #else
@@ -494,28 +505,25 @@ 

    while (ISDIGIT ((unsigned char)**type))
      {
+      /* Check whether we can add another digit without overflow. */
+      if (count > INT_MAX / 10)
+	goto overflow;
+
        count *= 10;

-      /* Check for overflow.
-	 We assume that count is represented using two's-complement;
-	 no power of two is divisible by ten, so if an overflow occurs
-	 when multiplying by ten, the result will not be a multiple of
-	 ten.  */
-      if ((count % 10) != 0)
-	{
-	  while (ISDIGIT ((unsigned char) **type))
-	    (*type)++;
-	  return -1;
-	}
+      if (count > INT_MAX - (**type - '0'))
+	goto overflow;

        count += **type - '0';
        (*type)++;
      }

-  if (count < 0)
-    count = -1;
+  return count;

-  return (count);
+ overflow:
+  while (ISDIGIT ((unsigned char) **type))
+    (*type)++;
+  return -1;
  }