DER parser: fix undefined behaviors and add missing length tests

Fix several undefined behaviors, caused by possible use or read of
uninitialized memory.

diff --git a/src/util-decode-der.c b/src/util-decode-der.c
index 02c16723d9cf33aa156f5fc1242be67dc8c8b0d6..acc79f3e7797a9dde6fbc4e3d5ce4e4d08b3b4f2 100644 (file)
--- a/src/util-decode-der.c
+++ b/src/util-decode-der.c
@@ -147,6 +147,10 @@ static Asn1Generic * DecodeAsn1DerGeneric(const unsigned char *buffer,
         *errcode = ERR_DER_RECURSION_LIMIT;
         return NULL;
     }
+    if (max_size < 2) {
+        *errcode = ERR_DER_INVALID_SIZE;
+        return NULL;
+    }
 
     el.cls = (d_ptr[0] & 0xc0) >> 6;
     el.pc = (d_ptr[0] & 0x20) >> 5;
@@ -247,6 +251,7 @@ static Asn1Generic * DecodeAsn1DerGeneric(const unsigned char *buffer,
             /* total sequence length */
             const unsigned char * save_d_ptr = d_ptr;
             d_ptr++;
+            el_max_size--;
             c = d_ptr[0];
 
             /* short form 8.1.3.4 */
@@ -256,7 +261,7 @@ static Asn1Generic * DecodeAsn1DerGeneric(const unsigned char *buffer,
             /* long form 8.1.3.5 */
             } else {
                 numbytes = c & 0x7f;
-                if (numbytes > el_max_size) {
+                if (2 + numbytes > el_max_size) {
                     SCFree(child);
                     *errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
                     return NULL;
@@ -306,7 +311,7 @@ static Asn1Generic * DecodeAsn1DerInteger(const unsigned char *buffer,
 
     numbytes = d_ptr[1];
 
-    if (numbytes > size) {
+    if ((uint32_t)(numbytes + 2) > size) {
         *errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
         return NULL;
     }
@@ -383,6 +388,9 @@ static Asn1Generic * DecodeAsn1DerBoolean(const unsigned char *buffer,
     Asn1Generic *a;
 
     numbytes = d_ptr[1];
+    if ((uint32_t)(numbytes + 2) > size) {
+        return NULL;
+    }
     d_ptr += 2;
 
     if (DecodeAsn1BuildValue(&d_ptr, &value, numbytes, errcode) == -1) {
@@ -410,6 +418,9 @@ static Asn1Generic * DecodeAsn1DerNull(const unsigned char *buffer,
     Asn1Generic *a;
 
     numbytes = d_ptr[1];
+    if ((uint32_t)(numbytes + 2) > size) {
+        return NULL;
+    }
     d_ptr += 2;
 
     if (DecodeAsn1BuildValue(&d_ptr, &value, numbytes, errcode) == -1) {
@@ -447,13 +458,16 @@ static Asn1Generic * DecodeAsn1DerBitstring(const unsigned char *buffer,
     /* long form 8.1.3.5 */
     } else {
         numbytes = c & 0x7f;
+        if ((uint32_t)(numbytes + 2) > max_size) {
+            return NULL;
+        }
         d_ptr++;
         if (DecodeAsn1BuildValue(&d_ptr, &length, numbytes, errcode) == -1) {
             return NULL;
         }
     }
 
-    if (length > max_size)
+    if ((d_ptr-buffer) + length > max_size)
         return NULL;
 
     a = Asn1GenericNew();
@@ -497,13 +511,16 @@ static Asn1Generic * DecodeAsn1DerOid(const unsigned char *buffer,
     /* long form 8.1.3.5 */
     } else {
         numbytes = c & 0x7f;
+        if ((uint32_t)(numbytes + 2) > max_size) {
+            return NULL;
+        }
         d_ptr++;
         if (DecodeAsn1BuildValue(&d_ptr, &oid_length, numbytes, errcode) == -1) {
             return NULL;
         }
     }
 
-    if (oid_length > max_size)
+    if (oid_length == 0 || (d_ptr-buffer) + oid_length > max_size)
         return NULL;
 
     a = Asn1GenericNew();
@@ -522,6 +539,12 @@ static Asn1Generic * DecodeAsn1DerOid(const unsigned char *buffer,
     snprintf(a->str, MAX_OID_LENGTH, "%d.%d", (d_ptr[0]/40), (d_ptr[0]%40));
     d_ptr++;
 
+    if (oid_length + (d_ptr-buffer) > max_size) {
+        SCFree(a->str);
+        SCFree(a);
+        return NULL;
+    }
+
     /* sub-identifiers are multi-valued, coded and 7 bits, first bit of
        the 8bits is used to indicate, if a new value is starting */
     for (i=1; i<oid_length; ) {
@@ -554,6 +577,7 @@ static Asn1Generic * DecodeAsn1DerIA5String(const unsigned char *buffer,
     unsigned char c;
 
     d_ptr++;
+    max_size--;
 
     /* total sequence length */
     c = d_ptr[0];
@@ -561,16 +585,21 @@ static Asn1Generic * DecodeAsn1DerIA5String(const unsigned char *buffer,
     if ((c & (1<<7))>>7 == 0) {
         length = c;
         d_ptr++;
+        max_size--;
     /* long form 8.1.3.5 */
     } else {
         numbytes = c & 0x7f;
+        if (max_size < 1 + numbytes) {
+            return NULL;
+        }
+        max_size -= 1 + numbytes;
         d_ptr++;
         if (DecodeAsn1BuildValue(&d_ptr, &length, numbytes, errcode) == -1) {
             return NULL;
         }
     }
 
-    if (length == UINT32_MAX || length > max_size) {
+    if (length == UINT32_MAX || (d_ptr-buffer) + length > max_size) {
         *errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
         return NULL;
     }
@@ -606,6 +635,7 @@ static Asn1Generic * DecodeAsn1DerOctetString(const unsigned char *buffer,
     unsigned char c;
 
     d_ptr++;
+    max_size--;
 
     /* total sequence length */
     c = d_ptr[0];
@@ -613,20 +643,24 @@ static Asn1Generic * DecodeAsn1DerOctetString(const unsigned char *buffer,
     if ((c & (1<<7))>>7 == 0) {
         length = c;
         d_ptr++;
+        max_size--;
     /* long form 8.1.3.5 */
     } else {
         numbytes = c & 0x7f;
+        if (max_size < 1 + numbytes) {
+            return NULL;
+        }
+        max_size -= 1 + numbytes;
         d_ptr++;
         if (DecodeAsn1BuildValue(&d_ptr, &length, numbytes, errcode) == -1) {
             return NULL;
         }
     }
 
-    if (length == UINT32_MAX || length > max_size) {
+    if (length == UINT32_MAX || (d_ptr-buffer) + length > max_size) {
         *errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
         return NULL;
     }
-
     a = Asn1GenericNew();
     if (a == NULL)
         return NULL;
@@ -683,12 +717,15 @@ static Asn1Generic * DecodeAsn1DerPrintableString(const unsigned char *buffer,
     } else {
         numbytes = c & 0x7f;
         d_ptr++;
+        if (2 + numbytes > max_size) {
+            return NULL;
+        }
         if (DecodeAsn1BuildValue(&d_ptr, &length, numbytes, errcode) == -1) {
             return NULL;
         }
     }
 
-    if (length == UINT32_MAX || length > max_size) {
+    if (length == UINT32_MAX || (d_ptr-buffer) + length > max_size) {
         *errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
         return NULL;
     }
@@ -742,6 +779,10 @@ static Asn1Generic * DecodeAsn1DerSequence(const unsigned char *buffer,
     } else {
         numbytes = c & 0x7f;
         d_ptr++;
+        if ( 2 + numbytes > max_size ) {
+            SCFree(node);
+            return NULL;
+        }
         if (DecodeAsn1BuildValue(&d_ptr, &d_length, numbytes, errcode) == -1) {
             SCFree(node);
             return NULL;
@@ -813,6 +854,10 @@ static Asn1Generic * DecodeAsn1DerSet(const unsigned char *buffer,
     /* long form 8.1.3.5 */
     } else {
         numbytes = c & 0x7f;
+        if (2 + numbytes > max_size) {
+            SCFree(node);
+            return NULL;
+        }
         d_ptr++;
         if (DecodeAsn1BuildValue(&d_ptr, &d_length, numbytes, errcode) == -1) {
             SCFree(node);
@@ -909,6 +954,9 @@ Asn1Generic * DecodeDer(const unsigned char *buffer, uint32_t size,
 
     numbytes = c & 0x7f;
     d_ptr += 2;
+    if (size < numbytes + 2) {
+        return NULL;
+    }
     if (DecodeAsn1BuildValue(&d_ptr, &d_length, numbytes, errcode) == -1) {
         return NULL;
     }