Simplify overflow check.

5 files changed, 98 insertions, 88 deletions

diff --git a/.gitignore b/.gitignore
index f5c283c..9c56412 100644
--- a/.gitignore
+++ b/.gitignore
@@ -182,6 +182,8 @@ tests/Test_errors
 tests/Test_errors.o
 tests/Test_indefinite
 tests/Test_indefinite.o
+tests/Test_overflow
+tests/Test_overflow.o
 tests/Test_parser
 tests/Test_parser.o
 tests/Test_parser_ERROR.asn
diff --git a/lib/coding.c b/lib/coding.c
index ab430fc..8b72eba 100644
--- a/lib/coding.c
+++ b/lib/coding.c
@@ -68,7 +68,7 @@ _asn1_error_description_value_not_found (ASN1_TYPE node,
  * The @ans buffer is pre-allocated and must have room for the output.
  **/
 void
-asn1_length_der (unsigned long len, unsigned char *ans, int *ans_len)
+asn1_length_der (unsigned long int len, unsigned char *ans, int *ans_len)
 {
   int k;
   unsigned char temp[SIZEOF_UNSIGNED_LONG_INT];
diff --git a/lib/decoding.c b/lib/decoding.c
index 9cd5a34..968fa96 100644
--- a/lib/decoding.c
+++ b/lib/decoding.c
@@ -45,8 +45,20 @@ _asn1_error_description_tag_error (ASN1_TYPE node, char *ErrorDescription)
 
 }
 
-static int
-_asn1_get_length_der (const unsigned char *der, int der_len, int *len)
+/**
+ * asn1_get_length_der:
+ * @der: DER data to decode.
+ * @der_len: Length of DER data to decode.
+ * @len: Output variable containing the length of the DER length field.
+ *
+ * Extract a length field from DER data.
+ *
+ * Returns: Return the decoded length value, or -1 on indefinite
+ *   length, or -2 when the value was too big to fit in a int, or -4
+ *   when the decoded length value plus @len would exceed @der_len.
+ **/
+signed long
+asn1_get_length_der (const unsigned char *der, int der_len, int *len)
 {
   int ans;
   int k, punt;
@@ -71,7 +83,7 @@ _asn1_get_length_der (const unsigned char *der, int der_len, int *len)
 	  ans = 0;
 	  while (punt <= k && punt < der_len)
 	    {
-	      unsigned long last = ans;
+	      int last = ans;
 
 	      ans = ans * 256 + der[punt++];
 	      if (ans < last)
@@ -81,61 +93,17 @@ _asn1_get_length_der (const unsigned char *der, int der_len, int *len)
 	}
       else
 	{			/* indefinite length method */
-	  ans = -1;
+	  *len = punt;
+	  return -1;
 	}
 
       *len = punt;
+      if (ans + *len < ans || ans + *len > der_len)
+	return -4;
       return ans;
     }
 }
 
-/*-
- * asn1_get_length_der_checked:
- * @der: DER data to decode.
- * @der_len: Length of DER data to decode.
- * @len: Output variable containing the length of the DER length field.
- *
- * Extract a length field from DER data.
- *
- * Returns: Return the decoded length value, or -1 on indefinite
- *   length, -2 when the value was too big or -3 when the value
- *   and the size of length exceed the @der_len.
- -*/
-static int
-asn1_get_length_der_checked (const unsigned char *der, int der_len, int *len)
-{
-int ret, tot;
-
-  ret = _asn1_get_length_der(der, der_len, len);
-  if (ret < 0)
-    return ret;
-  
-  tot = ret + *len;
-
-  if (tot < ret || tot > der_len)
-    return -3;
-
-  return ret;
-}
-
-/**
- * asn1_get_length_der:
- * @der: DER data to decode.
- * @der_len: Length of DER data to decode.
- * @len: Output variable containing the length of the DER length field.
- *
- * Extract a length field from DER data.
- *
- * Returns: Return the decoded length value, or -1 on indefinite
- *   length, -2 when the value was too big or -3 when the value
- *   and the size of length exceed the @der_len.
- **/
-long
-asn1_get_length_der (const unsigned char *der, int der_len, int *len)
-{
-  return asn1_get_length_der_checked(der, der_len, len);
-}
-
 /**
  * asn1_get_tag_der:
  * @der: DER data to decode.
@@ -193,24 +161,6 @@ asn1_get_tag_der (const unsigned char *der, int der_len,
   return ASN1_SUCCESS;
 }
 
-static int
-_asn1_get_length_ber (const unsigned char *ber, int ber_len, int *len)
-{
-  int ret;
-  long err;
-
-  ret = _asn1_get_length_der (ber, ber_len, len);
-  if (ret == -1)
-    {				/* indefinite length method */
-      ret = ber_len;
-      err = _asn1_get_indefinite_length_string (ber + 1, &ret);
-      if (err != ASN1_SUCCESS)
-	return -3;
-    }
-
-  return ret;
-}
-
 /**
  * asn1_get_length_ber:
  * @ber: BER data to decode.
@@ -226,10 +176,22 @@ _asn1_get_length_ber (const unsigned char *ber, int ber_len, int *len)
  *
  * Since: 2.0
  **/
-long
+signed long
 asn1_get_length_ber (const unsigned char *ber, int ber_len, int *len)
 {
-  return _asn1_get_length_ber(ber, ber_len, len);
+  int ret;
+  long err;
+
+  ret = asn1_get_length_der (ber, ber_len, len);
+  if (ret == -1)
+    {				/* indefinite length method */
+      ret = ber_len;
+      err = _asn1_get_indefinite_length_string (ber + 1, &ret);
+      if (err != ASN1_SUCCESS)
+	return -3;
+    }
+
+  return ret;
 }
 
 /**
@@ -434,7 +396,7 @@ _asn1_extract_tag_der (ASN1_TYPE node, const unsigned char *der, int der_len,
 		  counter += len2;
 
 		  len3 =
-		    _asn1_get_length_ber (der + counter, der_len - counter,
+		    asn1_get_length_ber (der + counter, der_len - counter,
 					 &len2);
 		  if (len3 < 0)
 		    return ASN1_DER_ERROR;
diff --git a/lib/libtasn1.h b/lib/libtasn1.h
index ed8ba87..289fb57 100644
--- a/lib/libtasn1.h
+++ b/lib/libtasn1.h
@@ -261,14 +261,14 @@ extern "C"
 		      int *ret_len, unsigned char *str,
 		      int str_size, int *bit_len);
 
-  extern ASN1_API long
+  extern ASN1_API signed long
     asn1_get_length_der (const unsigned char *der, int der_len, int *len);
 
-  extern ASN1_API long
+  extern ASN1_API signed long
     asn1_get_length_ber (const unsigned char *ber, int ber_len, int *len);
 
   extern ASN1_API void
-    asn1_length_der (unsigned long len, unsigned char *ans, int *ans_len);
+    asn1_length_der (unsigned long int len, unsigned char *ans, int *ans_len);
 
   /* Other utility functions. */
 
diff --git a/tests/Test_overflow.c b/tests/Test_overflow.c
index 383f723..2136899 100644
--- a/tests/Test_overflow.c
+++ b/tests/Test_overflow.c
@@ -23,25 +23,71 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
+#include <limits.h>
 
 #include "libtasn1.h"
 
 int
 main (void)
 {
-  unsigned char der[] = "\x84\x7F\xFF\xFF\xFF";
-  long l;
-  int len;
-
-  l = asn1_get_length_der (der, sizeof der, &len);
-
-  if (l == -3L)
-    puts ("asn1_get_length_der rejected overflow OK");
-  else
-    { 
-      printf ("asn1_get_length_der overflow (l %lX len %X)\n", l, len);
-      return 1;
+  /* Test that values larger than long are rejected.  This has worked
+     fine with all versions of libtasn1. */
+  {
+    unsigned char der[] = "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF";
+    long l;
+    int len;
+
+    l = asn1_get_length_der (der, sizeof der, &len);
+
+    if (l == -2L)
+      puts ("OK: asn1_get_length_der bignum");
+    else
+      {
+	printf ("ERROR: asn1_get_length_der bignum (l %ld len %d)\n", l, len);
+	return 1;
+      }
+  }
+
+  /* Test that values larger than int but smaller than long are
+     rejected.  This limitation was introduced with libtasn1 2.12. */
+  if (LONG_MAX > INT_MAX)
+    {
+      unsigned long num = ((long) UINT_MAX) << 2;
+      unsigned char der[20];
+      int der_len;
+      long l;
+      int len;
+
+      asn1_length_der (num, der, &der_len);
+
+      l = asn1_get_length_der (der, der_len, &len);
+
+      if (l == -2L)
+	puts ("OK: asn1_get_length_der intnum");
+      else
+	{
+	  printf ("ERROR: asn1_get_length_der intnum (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 char der[] = "\x84\x7F\xFF\xFF\xFF";
+    long l;
+    int len;
+
+    l = asn1_get_length_der (der, sizeof 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;
+      }
+  }
+
   return 0;
 }