Bug 1064670 - (CVE-2014-1569) ASN.1 DER decoding of lengths is too permissive, allowing undetected smuggling of arbitrary data (r=wtc)
authorJ.C. Jones <jjones@mozilla.com>
Mon, 10 Nov 2014 09:28:29 -1000
changeset 11305 e9a7991380db1973c84643c842066b3b5ec18306
parent 11298 10f6178ceb617b4bc0cc7ee246ee0c8276f7f55c
child 11306 902bc119dcdb6ae24bf82c466b22000577639e08
push id514
push usermartin.thomson@gmail.com
push dateMon, 10 Nov 2014 19:28:47 +0000
reviewerswtc
bugs1064670
Bug 1064670 - (CVE-2014-1569) ASN.1 DER decoding of lengths is too permissive, allowing undetected smuggling of arbitrary data (r=wtc)
lib/util/quickder.c
--- a/lib/util/quickder.c
+++ b/lib/util/quickder.c
@@ -11,65 +11,120 @@
 #include "secasn1.h" /* for SEC_ASN1GetSubtemplate */
 #include "secitem.h"
 
 /*
  * simple definite-length ASN.1 decoder
  */
 
 static unsigned char* definite_length_decoder(const unsigned char *buf,
-                                              const unsigned int length,
-                                              unsigned int *data_length,
+                                              const unsigned int buf_length,
+                                              unsigned int *out_data_length,
                                               PRBool includeTag)
 {
     unsigned char tag;
-    unsigned int used_length= 0;
-    unsigned int data_len;
+    unsigned int used_length = 0;
+    unsigned int data_length = 0;
+    unsigned char length_field_len = 0;
+    unsigned char byte;
+    unsigned int i;
 
-    if (used_length >= length)
+    if (used_length >= buf_length)
     {
+        /* Tag field was not found! */
         return NULL;
     }
     tag = buf[used_length++];
 
-    /* blow out when we come to the end */
     if (tag == 0)
     {
+        /* End-of-contents octects should not be present in DER because
+           DER doesn't use the indefinite length form. */
+        return NULL;
+    }
+
+    if ((tag & 0x1F) == 0x1F)
+    {
+        /* High tag number (a tag number > 30) is not supported */
         return NULL;
     }
 
-    if (used_length >= length)
+    if (used_length >= buf_length)
     {
+        /* Length field was not found! */
         return NULL;
     }
-    data_len = buf[used_length++];
+    byte = buf[used_length++];
 
-    if (data_len&0x80)
+    if (!(byte & 0x80))
+    {
+        /* Short form: The high bit is not set. */
+        data_length = byte; /* clarity; we're returning a 32-bit int. */
+    }
+    else
     {
-        int  len_count = data_len & 0x7f;
+        /* Long form. Extract the field length */
+        length_field_len = byte & 0x7F;
+        if (length_field_len == 0)
+        {
+            /* DER doesn't use the indefinite length form. */
+            return NULL;
+        }
 
-        data_len = 0;
+        if (length_field_len > sizeof(data_length))
+        {
+            /* We don't support an extended length field  longer than
+               4 bytes (2^32) */
+            return NULL;
+        }
 
-        while (len_count-- > 0)
+        if (length_field_len > (buf_length - used_length))
         {
-            if (used_length >= length)
+            /* Extended length field was not found */
+            return NULL;
+        }
+
+        /* Iterate across the extended length field */
+        for (i = 0; i < length_field_len; i++)
+        {
+            byte = buf[used_length++];
+            data_length = (data_length << 8) | byte;
+
+            if (i == 0)
             {
-                return NULL;
+                PRBool too_long = PR_FALSE;
+                if (length_field_len == 1)
+                {
+                    too_long = ((byte & 0x80) == 0); /* Short form suffices */
+                }
+                else
+                {
+                    too_long = (byte == 0); /* This zero byte can be omitted */
+                }
+                if (too_long)
+                {
+                    /* The length is longer than needed. */
+                    return NULL;
+                }
             }
-            data_len = (data_len << 8) | buf[used_length++];
         }
     }
 
-    if (data_len > (length-used_length) )
+    if (data_length > (buf_length - used_length))
     {
+        /* The decoded length exceeds the available buffer */
         return NULL;
     }
-    if (includeTag) data_len += used_length;
 
-    *data_length = data_len;
+    if (includeTag)
+    {
+        data_length += used_length;
+    }
+
+    *out_data_length = data_length;
     return ((unsigned char*)buf + (includeTag ? 0 : used_length));
 }
 
 static SECStatus GetItem(SECItem* src, SECItem* dest, PRBool includeTag)
 {
     if ( (!src) || (!dest) || (!src->data && src->len) )
     {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);