]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
x509asn1: add parse recursion limit
authorStefan Eissing <stefan@eissing.org>
Fri, 31 Jan 2025 12:13:34 +0000 (13:13 +0100)
committerDaniel Stenberg <daniel@haxx.se>
Mon, 3 Feb 2025 19:10:09 +0000 (20:10 +0100)
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
lib/vtls/x509asn1.h
tests/data/Makefile.am
tests/data/test1657 [new file with mode: 0644]
tests/unit/Makefile.inc
tests/unit/unit1657.c [new file with mode: 0644]

index fe4a38b8f747a24ef5aa44dfc71b2b7e1e1a9d66..0bc0a75a4bff273e5dbe8ad3498d21c699cfcc62 100644 (file)
@@ -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.
  *
index 5b48596c75910a5ef74e60c5e114a3e90762beb0..5de8f18e9ca62b86b45e6664139f3ec17b972f22 100644 (file)
@@ -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
 
index 76ba64e43755eb7cd57a3bfeebfad1dd776f7b98..1fc6c66817b18d167baa72495317a6969ada319e 100644 (file)
@@ -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 \
diff --git a/tests/data/test1657 b/tests/data/test1657
new file mode 100644 (file)
index 0000000..8e2a9c5
--- /dev/null
@@ -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>
index c523189233448483ee8b575c1bd47db99b315b91..4b34d4dd22cea40ec0c69f72e40f099c0384916a 100644 (file)
@@ -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)
diff --git a/tests/unit/unit1657.c b/tests/unit/unit1657.c
new file mode 100644 (file)
index 0000000..bd973fd
--- /dev/null
@@ -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