Added overflow detection that does not depend on specific compiler, and asn1_get_der_length() verifies the length of the input data in small numbers as well.

3 files changed, 90 insertions, 27 deletions

diff --git a/NEWS b/NEWS
index 48c2528..3c50cb4 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,8 @@
 GNU Libtasn1 NEWS                                     -*- outline -*-
 
+* Noteworthy changes in release 2.13 (unreleased) [stable]
+- Updated fix for DER decoding issue to not depend on specific compilers.
+
 * Noteworthy changes in release 2.12 (2012-03-19) [stable]
 - Cleanup license headers.
 - build: Update gnulib files.
diff --git a/lib/decoding.c b/lib/decoding.c
index eaca0d2..8f33930 100644
--- a/lib/decoding.c
+++ b/lib/decoding.c
@@ -30,6 +30,7 @@
 #include <gstr.h>
 #include "structure.h"
 #include "element.h"
+#include <limits.h>
 
 static asn1_retCode
 _asn1_get_indefinite_length_string (const unsigned char *der, int *len);
@@ -45,6 +46,22 @@ _asn1_error_description_tag_error (ASN1_TYPE node, char *ErrorDescription)
 
 }
 
+inline static int safe_mul(int a, int b)
+{
+  if (INT_MAX/a < b)
+    return INT_MAX;
+  else
+    return a*b;
+}
+
+inline static int safe_add(int a, int b)
+{
+  if (INT_MAX-a < b)
+    return INT_MAX;
+  else
+    return a+b;
+}
+
 /**
  * asn1_get_length_der:
  * @der: DER data to decode.
@@ -60,7 +77,7 @@ _asn1_error_description_tag_error (ASN1_TYPE node, char *ErrorDescription)
 signed long
 asn1_get_length_der (const unsigned char *der, int der_len, int *len)
 {
-  int ans;
+  int ans, sum;
   int k, punt;
 
   *len = 0;
@@ -71,7 +88,12 @@ asn1_get_length_der (const unsigned char *der, int der_len, int *len)
     {
       /* short form */
       *len = 1;
-      return der[0];
+      ans = der[0];
+      
+      if (ans > der_len)
+	return -4;
+      
+      return ans;
     }
   else
     {
@@ -83,10 +105,11 @@ asn1_get_length_der (const unsigned char *der, int der_len, int *len)
 	  ans = 0;
 	  while (punt <= k && punt < der_len)
 	    {
-	      int last = ans;
-
-	      ans = ans * 256 + der[punt++];
-	      if (ans < last)
+	      ans = safe_mul(256, ans);
+	      if (ans == INT_MAX) return -2;
+	      
+	      ans = safe_add(der[punt++], ans);
+	      if (ans == INT_MAX)
 		/* we wrapped around, no bignum support... */
 		return -2;
 	    }
@@ -98,8 +121,10 @@ asn1_get_length_der (const unsigned char *der, int der_len, int *len)
 	}
 
       *len = punt;
-      if (ans + *len < ans || ans + *len > der_len)
+      sum = safe_add(ans, *len);
+      if (sum == INT_MAX || sum > der_len)
 	return -4;
+
       return ans;
     }
 }
@@ -139,18 +164,24 @@ asn1_get_tag_der (const unsigned char *der, int der_len,
       ris = 0;
       while (punt <= der_len && der[punt] & 128)
 	{
-	  int last = ris;
-	  ris = ris * 128 + (der[punt++] & 0x7F);
-	  if (ris < last)
+	  ris = safe_mul(128, ris);
+	  if (ris == INT_MAX)
+	    return ASN1_DER_ERROR;
+
+	  ris = safe_add(der[punt++] & 0x7F, ris);
+	  if (ris == INT_MAX)
 	    /* wrapper around, and no bignums... */
 	    return ASN1_DER_ERROR;
 	}
       if (punt >= der_len)
 	return ASN1_DER_ERROR;
       {
-	int last = ris;
-	ris = ris * 128 + (der[punt++] & 0x7F);
-	if (ris < last)
+        ris = safe_mul(128, ris);
+        if (ris == INT_MAX)
+          return ASN1_DER_ERROR;
+        
+        ris = safe_add(der[punt++] & 0x7F, ris);
+	if (ris == INT_MAX)
 	  /* wrapper around, and no bignums... */
 	  return ASN1_DER_ERROR;
       }
diff --git a/tests/Test_overflow.c b/tests/Test_overflow.c
index 747507f..bd6ede7 100644
--- a/tests/Test_overflow.c
+++ b/tests/Test_overflow.c
@@ -74,22 +74,51 @@ main (void)
 
   /* Test that values larger than would fit in the input string are
      rejected.  This problem was fixed in libtasn1 2.12. */
-  {
-    unsigned char der[] = "\x84\x7F\xFF\xFF\xFF";
-    long l;
-    int len;
+    {
+      unsigned long num = 64;
+      unsigned char der[20];
+      int der_len;
+      long l;
+      int len;
 
-    l = asn1_get_length_der (der, sizeof der, &len);
+      asn1_length_der (num, der, &der_len);
 
-    if (l == -4L)
-      puts ("OK: asn1_get_length_der overflow");
-    else
-      {
-	printf ("ERROR: asn1_get_length_der overflow (l %ld len %d)\n", l,
-		len);
-	return 1;
-      }
-  }
+      der_len = sizeof(der);
+      l = asn1_get_length_der (der, der_len, &len);
+
+      if (l == -4L)
+	puts ("OK: asn1_get_length_der overflow-small");
+      else
+	{
+	  printf ("ERROR: asn1_get_length_der overflow-small (l %ld len %d)\n", l,
+		  len);
+	  return 1;
+	}
+    }
+
+  /* Test that values larger than would fit in the input string are
+     rejected.  This problem was fixed in libtasn1 2.12. */
+    {
+      unsigned long num = 2147483647;
+      unsigned char der[20];
+      int der_len;
+      long l;
+      int len;
+
+      asn1_length_der (num, der, &der_len);
+
+      der_len = sizeof(der);
+      l = asn1_get_length_der (der, der_len, &len);
+
+      if (l == -2L)
+	puts ("OK: asn1_get_length_der overflow-large");
+      else
+	{
+	  printf ("ERROR: asn1_get_length_der overflow-large (l %ld len %d)\n", l,
+		  len);
+	  return 1;
+	}
+    }
 
   return 0;
 }