DER parser: ensure errcode is set for every return path

diff --git a/src/util-decode-der.c b/src/util-decode-der.c
index acc79f3e7797a9dde6fbc4e3d5ce4e4d08b3b4f2..2d57d416adfcc4c808a88d8925c28dc5ab6ed572 100644 (file)
--- a/src/util-decode-der.c
+++ b/src/util-decode-der.c
@@ -158,8 +158,10 @@ static Asn1Generic * DecodeAsn1DerGeneric(const unsigned char *buffer,
 
     el_type = el.tag;
 
-    if (el.tag == 0x1f)
+    if (el.tag == 0x1f) {
+        *errcode = ERR_DER_INVALID_TAG;
         return NULL;
+    }
 
     switch (el.cls) {
         case ASN1_CLASS_CONTEXTSPEC:
@@ -279,6 +281,7 @@ static Asn1Generic * DecodeAsn1DerGeneric(const unsigned char *buffer,
             child->length += (d_ptr - save_d_ptr);
 
             if (child->length > max_size - (d_ptr - buffer)) {
+                *errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
                 SCFree(child);
                 return NULL;
             }
@@ -286,11 +289,14 @@ static Asn1Generic * DecodeAsn1DerGeneric(const unsigned char *buffer,
             break;
     };
 
-    if (child == NULL)
+    if (child == NULL) {
+        *errcode = ERR_DER_INVALID_OBJECT;
         return NULL;
+    }
 
     /* child length should never be zero */
     if (child->length == 0) {
+        *errcode = ERR_DER_INVALID_OBJECT;
         SCFree(child);
         return NULL;
     }
@@ -333,8 +339,10 @@ static Asn1Generic * DecodeAsn1DerInteger(const unsigned char *buffer,
     }
 
     a = Asn1GenericNew();
-    if (a == NULL)
+    if (a == NULL) {
+        *errcode = ERR_DER_GENERIC;
         return NULL;
+    }
 
     a->type = ASN1_INTEGER;
     a->length = (d_ptr - buffer) + numbytes;
@@ -342,6 +350,7 @@ static Asn1Generic * DecodeAsn1DerInteger(const unsigned char *buffer,
 
     a->str = SCMalloc(2*numbytes + 1);
     if (a->str == NULL) {
+        *errcode = ERR_DER_GENERIC;
         SCFree(a);
         return NULL;
     }
@@ -389,6 +398,7 @@ static Asn1Generic * DecodeAsn1DerBoolean(const unsigned char *buffer,
 
     numbytes = d_ptr[1];
     if ((uint32_t)(numbytes + 2) > size) {
+        *errcode = ERR_DER_INVALID_SIZE;
         return NULL;
     }
     d_ptr += 2;
@@ -398,8 +408,10 @@ static Asn1Generic * DecodeAsn1DerBoolean(const unsigned char *buffer,
     }
 
     a = Asn1GenericNew();
-    if (a == NULL)
+    if (a == NULL) {
+        *errcode = ERR_DER_GENERIC;
         return NULL;
+    }
 
     a->type = ASN1_BOOLEAN;
     a->length = (d_ptr - buffer);
@@ -419,6 +431,7 @@ static Asn1Generic * DecodeAsn1DerNull(const unsigned char *buffer,
 
     numbytes = d_ptr[1];
     if ((uint32_t)(numbytes + 2) > size) {
+        *errcode = ERR_DER_INVALID_SIZE;
         return NULL;
     }
     d_ptr += 2;
@@ -428,8 +441,10 @@ static Asn1Generic * DecodeAsn1DerNull(const unsigned char *buffer,
     }
 
     a = Asn1GenericNew();
-    if (a == NULL)
+    if (a == NULL) {
+        *errcode = ERR_DER_GENERIC;
         return NULL;
+    }
 
     a->type = ASN1_NULL;
     a->length = (d_ptr - buffer);
@@ -459,6 +474,7 @@ static Asn1Generic * DecodeAsn1DerBitstring(const unsigned char *buffer,
     } else {
         numbytes = c & 0x7f;
         if ((uint32_t)(numbytes + 2) > max_size) {
+            *errcode = ERR_DER_INVALID_SIZE;
             return NULL;
         }
         d_ptr++;
@@ -467,18 +483,23 @@ static Asn1Generic * DecodeAsn1DerBitstring(const unsigned char *buffer,
         }
     }
 
-    if ((d_ptr-buffer) + length > max_size)
+    if ((d_ptr-buffer) + length > max_size) {
+        *errcode = ERR_DER_INVALID_SIZE;
         return NULL;
+    }
 
     a = Asn1GenericNew();
-    if (a == NULL)
+    if (a == NULL) {
+        *errcode = ERR_DER_GENERIC;
         return NULL;
+    }
 
     a->type = ASN1_BITSTRING;
     a->strlen = length;
 
     a->str = SCMalloc(length);
     if (a->str == NULL) {
+        *errcode = ERR_DER_GENERIC;
         SCFree(a);
         return NULL;
     }
@@ -512,6 +533,7 @@ static Asn1Generic * DecodeAsn1DerOid(const unsigned char *buffer,
     } else {
         numbytes = c & 0x7f;
         if ((uint32_t)(numbytes + 2) > max_size) {
+            *errcode = ERR_DER_INVALID_SIZE;
             return NULL;
         }
         d_ptr++;
@@ -520,17 +542,22 @@ static Asn1Generic * DecodeAsn1DerOid(const unsigned char *buffer,
         }
     }
 
-    if (oid_length == 0 || (d_ptr-buffer) + oid_length > max_size)
+    if (oid_length == 0 || (d_ptr-buffer) + oid_length > max_size) {
+        *errcode = ERR_DER_INVALID_SIZE;
         return NULL;
+    }
 
     a = Asn1GenericNew();
-    if (a == NULL)
+    if (a == NULL) {
+        *errcode = ERR_DER_GENERIC;
         return NULL;
+    }
 
     a->type = ASN1_OID;
 
     a->str = SCMalloc(MAX_OID_LENGTH);
     if (a->str == NULL) {
+        *errcode = ERR_DER_GENERIC;
         SCFree(a);
         return NULL;
     }
@@ -540,6 +567,7 @@ static Asn1Generic * DecodeAsn1DerOid(const unsigned char *buffer,
     d_ptr++;
 
     if (oid_length + (d_ptr-buffer) > max_size) {
+        *errcode = ERR_DER_INVALID_SIZE;
         SCFree(a->str);
         SCFree(a);
         return NULL;
@@ -590,6 +618,7 @@ static Asn1Generic * DecodeAsn1DerIA5String(const unsigned char *buffer,
     } else {
         numbytes = c & 0x7f;
         if (max_size < 1 + numbytes) {
+            *errcode = ERR_DER_INVALID_SIZE;
             return NULL;
         }
         max_size -= 1 + numbytes;
@@ -605,14 +634,17 @@ static Asn1Generic * DecodeAsn1DerIA5String(const unsigned char *buffer,
     }
 
     a = Asn1GenericNew();
-    if (a == NULL)
+    if (a == NULL) {
+        *errcode = ERR_DER_GENERIC;
         return NULL;
+    }
 
     a->type = ASN1_IA5STRING;
     a->strlen = length;
 
     a->str = SCMalloc(length+1);
     if (a->str == NULL) {
+        *errcode = ERR_DER_GENERIC;
         SCFree(a);
         return NULL;
     }
@@ -648,6 +680,7 @@ static Asn1Generic * DecodeAsn1DerOctetString(const unsigned char *buffer,
     } else {
         numbytes = c & 0x7f;
         if (max_size < 1 + numbytes) {
+            *errcode = ERR_DER_INVALID_SIZE;
             return NULL;
         }
         max_size -= 1 + numbytes;
@@ -662,8 +695,10 @@ static Asn1Generic * DecodeAsn1DerOctetString(const unsigned char *buffer,
         return NULL;
     }
     a = Asn1GenericNew();
-    if (a == NULL)
+    if (a == NULL) {
+        *errcode = ERR_DER_GENERIC;
         return NULL;
+    }
 
     a->type = ASN1_OCTETSTRING;
     a->strlen = length;
@@ -672,6 +707,7 @@ static Asn1Generic * DecodeAsn1DerOctetString(const unsigned char *buffer,
        use the string in printf */
     a->str = SCMalloc(length + 1);
     if (a->str == NULL) {
+        *errcode = ERR_DER_GENERIC;
         SCFree(a);
         return NULL;
     }
@@ -718,6 +754,7 @@ static Asn1Generic * DecodeAsn1DerPrintableString(const unsigned char *buffer,
         numbytes = c & 0x7f;
         d_ptr++;
         if (2 + numbytes > max_size) {
+            *errcode = ERR_DER_INVALID_SIZE;
             return NULL;
         }
         if (DecodeAsn1BuildValue(&d_ptr, &length, numbytes, errcode) == -1) {
@@ -731,14 +768,17 @@ static Asn1Generic * DecodeAsn1DerPrintableString(const unsigned char *buffer,
     }
 
     a = Asn1GenericNew();
-    if (a == NULL)
+    if (a == NULL) {
+        *errcode = ERR_DER_GENERIC;
         return NULL;
+    }
 
     a->type = ASN1_PRINTSTRING;
     a->strlen = length;
 
     a->str = SCMalloc(length+1);
     if (a->str == NULL) {
+        *errcode = ERR_DER_GENERIC;
         SCFree(a);
         return NULL;
     }
@@ -764,8 +804,10 @@ static Asn1Generic * DecodeAsn1DerSequence(const unsigned char *buffer,
     d_ptr++;
 
     node = Asn1GenericNew();
-    if (node == NULL)
+    if (node == NULL) {
+        *errcode = ERR_DER_GENERIC;
         return NULL;
+    }
 
     node->type = ASN1_SEQUENCE;
 
@@ -780,6 +822,7 @@ static Asn1Generic * DecodeAsn1DerSequence(const unsigned char *buffer,
         numbytes = c & 0x7f;
         d_ptr++;
         if ( 2 + numbytes > max_size ) {
+            *errcode = ERR_DER_INVALID_SIZE;
             SCFree(node);
             return NULL;
         }
@@ -840,8 +883,10 @@ static Asn1Generic * DecodeAsn1DerSet(const unsigned char *buffer,
     d_ptr++;
 
     node = Asn1GenericNew();
-    if (node == NULL)
+    if (node == NULL) {
+        *errcode = ERR_DER_GENERIC;
         return NULL;
+    }
     node->type = ASN1_SET;
     node->data = NULL;
 
@@ -855,6 +900,7 @@ static Asn1Generic * DecodeAsn1DerSet(const unsigned char *buffer,
     } else {
         numbytes = c & 0x7f;
         if (2 + numbytes > max_size) {
+            *errcode = ERR_DER_INVALID_SIZE;
             SCFree(node);
             return NULL;
         }
@@ -940,31 +986,40 @@ Asn1Generic * DecodeDer(const unsigned char *buffer, uint32_t size,
     uint8_t c;
 
     DEBUG_VALIDATE_BUG_ON(errcode == NULL);
+    *errcode = 0;
 
-    if (size < 2)
+    if (size < 2) {
+        *errcode = ERR_DER_INVALID_SIZE;
         return NULL;
+    }
 
     /* check that buffer is an ASN.1 structure (basic checks) */
-    if (d_ptr[0] != 0x30 && d_ptr[1] != 0x82) /* Sequence */
+    if (d_ptr[0] != 0x30 && d_ptr[1] != 0x82) { /* Sequence */
+        *errcode = ERR_DER_UNKNOWN_ELEMENT;
         return NULL;
+    }
 
     c = d_ptr[1];
-    if ((c & (1<<7))>>7 != 1)
+    if ((c & (1<<7))>>7 != 1) {
+        *errcode = ERR_DER_INVALID_SIZE;
         return NULL;
+    }
 
     numbytes = c & 0x7f;
     d_ptr += 2;
     if (size < numbytes + 2) {
+        *errcode = ERR_DER_INVALID_SIZE;
         return NULL;
     }
     if (DecodeAsn1BuildValue(&d_ptr, &d_length, numbytes, errcode) == -1) {
         return NULL;
     }
 
-    if (d_length+(d_ptr-buffer) != size)
+    if (d_length+(d_ptr-buffer) != size) {
+        *errcode = ERR_DER_INVALID_SIZE;
         return NULL;
+    }
 
-    *errcode = 0;
     cert = DecodeAsn1DerGeneric(buffer, size, 0 /* depth */, 0, errcode);
 
     return cert;

diff --git a/src/util-decode-der.h b/src/util-decode-der.h
index 9710f68c988e541646f3ecddd5071072d58d29ae..5110a4e0f18e9cbef1b70639c66e805b5e6aa8e7 100644 (file)
--- a/src/util-decode-der.h
+++ b/src/util-decode-der.h
@@ -78,13 +78,13 @@ typedef struct Asn1Generic_ {
        struct Asn1Generic_ *next; /* only if type is sequence */
 } Asn1Generic;
 
-/* Generic error */
+/* Generic error (failed allocation, etc.) */
 #define ERR_DER_GENERIC               0x01
 /* Unknown ASN.1 element type */
 #define ERR_DER_UNKNOWN_ELEMENT       0x02
 /* One element requires to read more bytes than available */
 #define ERR_DER_ELEMENT_SIZE_TOO_BIG  0x03
-/* One element size is invalid (more than 4 bytes long) */
+/* One element size is invalid (e.g more than 4 bytes long) */
 #define ERR_DER_INVALID_SIZE          0x04
 /* Unsupported string type */
 #define ERR_DER_UNSUPPORTED_STRING    0x05
@@ -92,6 +92,10 @@ typedef struct Asn1Generic_ {
 #define ERR_DER_MISSING_ELEMENT       0x06
 /* Generic error */
 #define ERR_DER_RECURSION_LIMIT       0x07
+/* Unexpected or unknown tag */
+#define ERR_DER_INVALID_TAG           0x08
+/* Invalid element: empty object, etc. */
+#define ERR_DER_INVALID_OBJECT        0x09
 
 Asn1Generic * DecodeDer(const unsigned char *buffer, uint32_t size, uint32_t *errcode) __attribute__((nonnull));
 void DerFree(Asn1Generic *a);