x509asn1: add parse recursion limit

For ASN.1 tags with indefinite length, curl's own parser for TLS
backends that do not support certificate inspection calls itself
recursively. A malicious server certificate can then lead to high
recursion level exhausting the stack space.

This PR limits the recursion level to 16 which should be safe on all
architectures.

Added unit test 1657 to verify behaviour.

Fixes #16135
Reported-by: z2_
Closes #16137

lib/vtls/x509asn1.c

@@ -178,8 +178,11 @@ static const char *getASN1Element(struct Curl_asn1Element *elem,
                                   const char *beg, const char *end)
   WARN_UNUSED_RESULT;
 
-static const char *getASN1Element(struct Curl_asn1Element *elem,
-                                  const char *beg, const char *end)
+#define CURL_ASN1_MAX_RECURSIONS    16
+
+static const char *getASN1Element_(struct Curl_asn1Element *elem,
+                                   const char *beg, const char *end,
+                                   size_t lvl)
 {
   unsigned char b;
   size_t len;
@@ -190,7 +193,8 @@ static const char *getASN1Element(struct Curl_asn1Element *elem,
      Returns a pointer in source string after the parsed element, or NULL
      if an error occurs. */
   if(!beg || !end || beg >= end || !*beg ||
-     (size_t)(end - beg) > CURL_ASN1_MAX)
+     ((size_t)(end - beg) > CURL_ASN1_MAX) ||
+     lvl >=  CURL_ASN1_MAX_RECURSIONS)
     return NULL;
 
   /* Process header byte. */
@@ -216,7 +220,7 @@ static const char *getASN1Element(struct Curl_asn1Element *elem,
       return NULL;
     elem->beg = beg;
     while(beg < end && *beg) {
-      beg = getASN1Element(&lelem, beg, end);
+      beg = getASN1Element_(&lelem, beg, end, lvl + 1);
       if(!beg)
         return NULL;
     }
@@ -243,6 +247,12 @@ static const char *getASN1Element(struct Curl_asn1Element *elem,
   return elem->end;
 }
 
+static const char *getASN1Element(struct Curl_asn1Element *elem,
+                                  const char *beg, const char *end)
+{
+  return getASN1Element_(elem, beg, end, 0);
+}
+
 #ifdef WANT_EXTRACT_CERTINFO
 
 /*
@@ -259,6 +269,17 @@ static const struct Curl_OID *searchOID(const char *oid)
   return NULL;
 }
 
+#ifdef UNITTESTS
+/* used by unit1657.c */
+CURLcode Curl_x509_getASN1Element(struct Curl_asn1Element *elem,
+                                  const char *beg, const char *end)
+{
+  if(getASN1Element(elem, beg, end))
+    return CURLE_OK;
+  return CURLE_BAD_FUNCTION_ARGUMENT;
+}
+#endif
+
 /*
  * Convert an ASN.1 Boolean value into its string representation.
  *

lib/vtls/x509asn1.h

@@ -85,6 +85,9 @@ CURLcode Curl_verifyhost(struct Curl_cfilter *cf, struct Curl_easy *data,
 /* used by unit1656.c */
 CURLcode Curl_x509_GTime2str(struct dynbuf *store,
                              const char *beg, const char *end);
+/* used by unit1657.c */
+CURLcode Curl_x509_getASN1Element(struct Curl_asn1Element *elem,
+                                  const char *beg, const char *end);
 #endif
 #endif
 

tests/data/Makefile.am

@@ -218,7 +218,7 @@ test1620 test1621 \
 \
 test1630 test1631 test1632 test1633 test1634 test1635 \
 \
-test1650 test1651 test1652 test1653 test1654 test1655 test1656 \
+test1650 test1651 test1652 test1653 test1654 test1655 test1656 test1657 \
 test1660 test1661 test1662 test1663 test1664 \
 \
 test1670 test1671 \

tests/data/test1657

@@ -0,0 +1,22 @@
+<testcase>
+<info>
+<keywords>
+unittest
+Curl_x509_getASN1Element
+</keywords>
+</info>
+
+#
+# Client-side
+<client>
+<server>
+none
+</server>
+<features>
+unittest
+</features>
+<name>
+Curl_x509_getASN1Element unit tests
+</name>
+</client>
+</testcase>

tests/unit/Makefile.inc

@@ -38,7 +38,7 @@ UNITPROGS = unit1300          unit1302 unit1303 unit1304 unit1305 unit1307 \
  unit1600 unit1601 unit1602 unit1603 unit1604 unit1605 unit1606 unit1607 \
  unit1608 unit1609 unit1610 unit1611 unit1612 unit1614 unit1615 unit1616 \
  unit1620 unit1621 \
- unit1650 unit1651 unit1652 unit1653 unit1654 unit1655 unit1656 \
+ unit1650 unit1651 unit1652 unit1653 unit1654 unit1655 unit1656 unit1657 \
  unit1660 unit1661 unit1663 unit1664 \
  unit2600 unit2601 unit2602 unit2603 unit2604 \
  unit3200 \
@@ -126,6 +126,8 @@ unit1655_SOURCES = unit1655.c $(UNITFILES)
 
 unit1656_SOURCES = unit1656.c $(UNITFILES)
 
+unit1657_SOURCES = unit1657.c $(UNITFILES)
+
 unit1660_SOURCES = unit1660.c $(UNITFILES)
 
 unit1661_SOURCES = unit1661.c $(UNITFILES)

tests/unit/unit1657.c

@@ -0,0 +1,134 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+#include "curlcheck.h"
+
+#include "vtls/x509asn1.h"
+
+static CURLcode unit_setup(void)
+{
+  return CURLE_OK;
+}
+
+static void unit_stop(void)
+{
+
+}
+
+#if defined(USE_GNUTLS) || defined(USE_SCHANNEL) || defined(USE_SECTRANSP) || \
+  defined(USE_MBEDTLS)
+
+#ifndef ARRAYSIZE
+#define ARRAYSIZE(A) (sizeof(A)/sizeof((A)[0]))
+#endif
+
+struct test1657_spec {
+  CURLcode (*setbuf)(struct test1657_spec *spec, struct dynbuf *buf);
+  size_t n;
+  CURLcode exp_result;
+};
+
+static CURLcode make1657_nested(struct test1657_spec *spec, struct dynbuf *buf)
+{
+  CURLcode r;
+  size_t i;
+  unsigned char open_undef[] = { 0x32,  0x80 };
+  unsigned char close_undef[] = { 0x00,  0x00 };
+
+  for(i = 0; i < spec->n; ++i) {
+    r = Curl_dyn_addn(buf, open_undef, sizeof(open_undef));
+    if(r)
+      return r;
+  }
+  for(i = 0; i < spec->n; ++i) {
+    r = Curl_dyn_addn(buf, close_undef, sizeof(close_undef));
+    if(r)
+      return r;
+  }
+  return CURLE_OK;
+}
+
+static struct test1657_spec test1657_specs[] = {
+  { make1657_nested, 3, CURLE_OK },
+  { make1657_nested, 16, CURLE_OK },
+  { make1657_nested, 17, CURLE_BAD_FUNCTION_ARGUMENT },
+  { make1657_nested, 1024, CURLE_BAD_FUNCTION_ARGUMENT },
+};
+
+static bool do_test1657(struct test1657_spec *spec, size_t i,
+                        struct dynbuf *buf)
+{
+  CURLcode result;
+  struct Curl_asn1Element elem;
+  const char *in;
+
+  memset(&elem, 0, sizeof(elem));
+  Curl_dyn_reset(buf);
+  result = spec->setbuf(spec, buf);
+  if(result) {
+    fprintf(stderr, "test %zu: error setting buf %d\n", i, result);
+    return FALSE;
+  }
+  in = Curl_dyn_ptr(buf);
+  result = Curl_x509_getASN1Element(&elem, in, in + Curl_dyn_len(buf));
+  if(result != spec->exp_result) {
+    fprintf(stderr, "test %zu: expect result %d, got %d\n",
+            i, spec->exp_result, result);
+    return FALSE;
+  }
+  return TRUE;
+}
+
+UNITTEST_START
+{
+  size_t i;
+  bool all_ok = TRUE;
+  struct dynbuf dbuf;
+
+  Curl_dyn_init(&dbuf, 32*1024);
+
+  if(curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) {
+    fprintf(stderr, "curl_global_init() failed\n");
+    return TEST_ERR_MAJOR_BAD;
+  }
+
+  for(i = 0; i < ARRAYSIZE(test1657_specs); ++i) {
+    if(!do_test1657(&test1657_specs[i], i, &dbuf))
+      all_ok = FALSE;
+  }
+  fail_unless(all_ok, "some tests of Curl_x509_getASN1Element() fails");
+
+  Curl_dyn_free(&dbuf);
+  curl_global_cleanup();
+}
+UNITTEST_STOP
+
+#else
+
+UNITTEST_START
+{
+  puts("not tested since Curl_x509_getASN1Element() is not built-in");
+}
+UNITTEST_STOP
+
+#endif