Limit ASN.1 constructed types recursive definition depth

Constructed types with a recursive definition could eventually exceed
the stack given malicious input with excessive recursion. Therefore we
limit the stack depth.

CVE-2018-0739

Credit to OSSFuzz for finding this issue.

(Imported from upstream's 9310d45087ae546e27e61ddf8f6367f29848220d.)

BoringSSL does not contain any such structures, but import this anyway
with a test.

Change-Id: I0e84578ea795134f25dae2ac8b565f3c26ef3204
Reviewed-on: https://boringssl-review.googlesource.com/26844
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/asn1/asn1_test.cc

@@ -12,13 +12,18 @@
  * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
 
+#include <limits.h>
 #include <stdio.h>
 
+#include <vector>
+
 #include <gtest/gtest.h>
-#include <limits.h>
 
 #include <openssl/asn1.h>
+#include <openssl/asn1t.h>
+#include <openssl/bytestring.h>
 #include <openssl/err.h>
+#include <openssl/mem.h>
 
 #include "../test/test_util.h"
 
@@ -88,3 +93,58 @@ TEST(ASN1Test, IntegerSetting) {
     }
   }
 }
+
+typedef struct asn1_linked_list_st {
+  struct asn1_linked_list_st *next;
+} ASN1_LINKED_LIST;
+
+DECLARE_ASN1_ITEM(ASN1_LINKED_LIST)
+DECLARE_ASN1_FUNCTIONS(ASN1_LINKED_LIST)
+
+ASN1_SEQUENCE(ASN1_LINKED_LIST) = {
+  ASN1_OPT(ASN1_LINKED_LIST, next, ASN1_LINKED_LIST),
+} ASN1_SEQUENCE_END(ASN1_LINKED_LIST)
+
+IMPLEMENT_ASN1_FUNCTIONS(ASN1_LINKED_LIST)
+
+static bool MakeLinkedList(bssl::UniquePtr<uint8_t> *out, size_t *out_len,
+                           size_t count) {
+  bssl::ScopedCBB cbb;
+  std::vector<CBB> cbbs(count);
+  if (!CBB_init(cbb.get(), 2 * count) ||
+      !CBB_add_asn1(cbb.get(), &cbbs[0], CBS_ASN1_SEQUENCE)) {
+    return false;
+  }
+  for (size_t i = 1; i < count; i++) {
+    if (!CBB_add_asn1(&cbbs[i - 1], &cbbs[i], CBS_ASN1_SEQUENCE)) {
+      return false;
+    }
+  }
+  uint8_t *ptr;
+  if (!CBB_finish(cbb.get(), &ptr, out_len)) {
+    return false;
+  }
+  out->reset(ptr);
+  return true;
+}
+
+TEST(ASN1Test, Recursive) {
+  bssl::UniquePtr<uint8_t> data;
+  size_t len;
+
+  // Sanity-check that MakeLinkedList can be parsed.
+  ASSERT_TRUE(MakeLinkedList(&data, &len, 5));
+  const uint8_t *ptr = data.get();
+  ASN1_LINKED_LIST *list = d2i_ASN1_LINKED_LIST(nullptr, &ptr, len);
+  EXPECT_TRUE(list);
+  ASN1_LINKED_LIST_free(list);
+
+  // Excessively deep structures are rejected.
+  ASSERT_TRUE(MakeLinkedList(&data, &len, 100));
+  ptr = data.get();
+  list = d2i_ASN1_LINKED_LIST(nullptr, &ptr, len);
+  EXPECT_FALSE(list);
+  // Note checking the error queue here does not work. The error "stack trace"
+  // is too deep, so the |ASN1_R_NESTED_TOO_DEEP| entry drops off the queue.
+  ASN1_LINKED_LIST_free(list);
+}

crypto/asn1/tasn_dec.c

@@ -66,6 +66,14 @@
 
 #include "../internal.h"
 
+/*
+ * Constructed types with a recursive definition (such as can be found in PKCS7)
+ * could eventually exceed the stack given malicious input with excessive
+ * recursion. Therefore we limit the stack depth. This is the maximum number of
+ * recursive invocations of asn1_item_embed_d2i().
+ */
+#define ASN1_MAX_CONSTRUCTED_NEST 30
+
 static int asn1_check_eoc(const unsigned char **in, long len);
 static int asn1_find_end(const unsigned char **in, long len, char inf);
 
@@ -82,11 +90,11 @@ static int asn1_check_tlen(long *olen, int *otag, unsigned char *oclass,
 static int asn1_template_ex_d2i(ASN1_VALUE **pval,
                                 const unsigned char **in, long len,
                                 const ASN1_TEMPLATE *tt, char opt,
-                                ASN1_TLC *ctx);
+                                ASN1_TLC *ctx, int depth);
 static int asn1_template_noexp_d2i(ASN1_VALUE **val,
                                    const unsigned char **in, long len,
                                    const ASN1_TEMPLATE *tt, char opt,
-                                   ASN1_TLC *ctx);
+                                   ASN1_TLC *ctx, int depth);
 static int asn1_d2i_ex_primitive(ASN1_VALUE **pval,
                                  const unsigned char **in, long len,
                                  const ASN1_ITEM *it,
@@ -153,9 +161,9 @@ ASN1_VALUE *ASN1_item_d2i(ASN1_VALUE **pval,
  * tag mismatch return -1 to handle OPTIONAL
  */
 
-int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
-                     const ASN1_ITEM *it,
-                     int tag, int aclass, char opt, ASN1_TLC *ctx)
+static int asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in,
+                            long len, const ASN1_ITEM *it, int tag, int aclass,
+                            char opt, ASN1_TLC *ctx, int depth)
 {
     const ASN1_TEMPLATE *tt, *errtt = NULL;
     const ASN1_COMPAT_FUNCS *cf;
@@ -188,6 +196,11 @@ int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
         len = INT_MAX/2;
     }
 
+    if (++depth > ASN1_MAX_CONSTRUCTED_NEST) {
+        OPENSSL_PUT_ERROR(ASN1, ASN1_R_NESTED_TOO_DEEP);
+        goto err;
+    }
+
     switch (it->itype) {
     case ASN1_ITYPE_PRIMITIVE:
         if (it->templates) {
@@ -203,7 +216,7 @@ int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
                 goto err;
             }
             return asn1_template_ex_d2i(pval, in, len,
-                                        it->templates, opt, ctx);
+                                        it->templates, opt, ctx, depth);
         }
         return asn1_d2i_ex_primitive(pval, in, len, it,
                                      tag, aclass, opt, ctx);
@@ -326,7 +339,7 @@ int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
             /*
              * We mark field as OPTIONAL so its absence can be recognised.
              */
-            ret = asn1_template_ex_d2i(pchptr, &p, len, tt, 1, ctx);
+            ret = asn1_template_ex_d2i(pchptr, &p, len, tt, 1, ctx, depth);
             /* If field not present, try the next one */
             if (ret == -1)
                 continue;
@@ -444,7 +457,8 @@ int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
              * attempt to read in field, allowing each to be OPTIONAL
              */
 
-            ret = asn1_template_ex_d2i(pseqval, &p, len, seqtt, isopt, ctx);
+            ret = asn1_template_ex_d2i(pseqval, &p, len, seqtt, isopt, ctx,
+                                       depth);
             if (!ret) {
                 errtt = seqtt;
                 goto err;
@@ -514,6 +528,13 @@ int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
     return 0;
 }
 
+int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
+                     const ASN1_ITEM *it,
+                     int tag, int aclass, char opt, ASN1_TLC *ctx)
+{
+    return asn1_item_ex_d2i(pval, in, len, it, tag, aclass, opt, ctx, 0);
+}
+
 /*
  * Templates are handled with two separate functions. One handles any
  * EXPLICIT tag and the other handles the rest.
@@ -522,7 +543,7 @@ int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
 static int asn1_template_ex_d2i(ASN1_VALUE **val,
                                 const unsigned char **in, long inlen,
                                 const ASN1_TEMPLATE *tt, char opt,
-                                ASN1_TLC *ctx)
+                                ASN1_TLC *ctx, int depth)
 {
     int flags, aclass;
     int ret;
@@ -556,7 +577,7 @@ static int asn1_template_ex_d2i(ASN1_VALUE **val,
             return 0;
         }
         /* We've found the field so it can't be OPTIONAL now */
-        ret = asn1_template_noexp_d2i(val, &p, len, tt, 0, ctx);
+        ret = asn1_template_noexp_d2i(val, &p, len, tt, 0, ctx, depth);
         if (!ret) {
             OPENSSL_PUT_ERROR(ASN1, ASN1_R_NESTED_ASN1_ERROR);
             return 0;
@@ -579,7 +600,7 @@ static int asn1_template_ex_d2i(ASN1_VALUE **val,
             }
         }
     } else
-        return asn1_template_noexp_d2i(val, in, inlen, tt, opt, ctx);
+        return asn1_template_noexp_d2i(val, in, inlen, tt, opt, ctx, depth);
 
     *in = p;
     return 1;
@@ -592,7 +613,7 @@ static int asn1_template_ex_d2i(ASN1_VALUE **val,
 static int asn1_template_noexp_d2i(ASN1_VALUE **val,
                                    const unsigned char **in, long len,
                                    const ASN1_TEMPLATE *tt, char opt,
-                                   ASN1_TLC *ctx)
+                                   ASN1_TLC *ctx, int depth)
 {
     int flags, aclass;
     int ret;
@@ -661,8 +682,8 @@ static int asn1_template_noexp_d2i(ASN1_VALUE **val,
                 break;
             }
             skfield = NULL;
-            if (!ASN1_item_ex_d2i(&skfield, &p, len,
-                                  ASN1_ITEM_ptr(tt->item), -1, 0, 0, ctx)) {
+             if (!asn1_item_ex_d2i(&skfield, &p, len, ASN1_ITEM_ptr(tt->item),
+                                   -1, 0, 0, ctx, depth)) {
                 OPENSSL_PUT_ERROR(ASN1, ASN1_R_NESTED_ASN1_ERROR);
                 goto err;
             }
@@ -679,9 +700,8 @@ static int asn1_template_noexp_d2i(ASN1_VALUE **val,
         }
     } else if (flags & ASN1_TFLG_IMPTAG) {
         /* IMPLICIT tagging */
-        ret = ASN1_item_ex_d2i(val, &p, len,
-                               ASN1_ITEM_ptr(tt->item), tt->tag, aclass, opt,
-                               ctx);
+        ret = asn1_item_ex_d2i(val, &p, len, ASN1_ITEM_ptr(tt->item), tt->tag,
+                               aclass, opt, ctx, depth);
         if (!ret) {
             OPENSSL_PUT_ERROR(ASN1, ASN1_R_NESTED_ASN1_ERROR);
             goto err;
@@ -689,8 +709,9 @@ static int asn1_template_noexp_d2i(ASN1_VALUE **val,
             return -1;
     } else {
         /* Nothing special */
-        ret = ASN1_item_ex_d2i(val, &p, len, ASN1_ITEM_ptr(tt->item),
-                               -1, tt->flags & ASN1_TFLG_COMBINE, opt, ctx);
+        ret = asn1_item_ex_d2i(val, &p, len, ASN1_ITEM_ptr(tt->item),
+                               -1, tt->flags & ASN1_TFLG_COMBINE, opt, ctx,
+                               depth);
         if (!ret) {
             OPENSSL_PUT_ERROR(ASN1, ASN1_R_NESTED_ASN1_ERROR);
             goto err;

crypto/err/asn1.errordata

@@ -58,6 +58,7 @@ ASN1,156,MSTRING_NOT_UNIVERSAL
 ASN1,157,MSTRING_WRONG_TAG
 ASN1,158,NESTED_ASN1_ERROR
 ASN1,159,NESTED_ASN1_STRING
+ASN1,192,NESTED_TOO_DEEP
 ASN1,160,NON_HEX_CHARACTERS
 ASN1,161,NOT_ASCII_FORMAT
 ASN1,162,NOT_ENOUGH_DATA

include/openssl/asn1.h

@@ -976,5 +976,6 @@ BORINGSSL_MAKE_DELETER(ASN1_TYPE, ASN1_TYPE_free)
 #define ASN1_R_WRONG_PUBLIC_KEY_TYPE 189
 #define ASN1_R_WRONG_TAG 190
 #define ASN1_R_WRONG_TYPE 191
+#define ASN1_R_NESTED_TOO_DEEP 192
 
 #endif