From 3c914bc680155b32178f1f15ca8d47c7f4640afe Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Tue, 30 Jul 2024 10:05:17 +0200
Subject: [PATCH] CVE-2024-7264: x509asn1: complete fix with unit tests
Backport CVE-2024-7264 fix from curl 8.4.0 to curl 7.78.0 (Complete Version)
This patch includes both the security fix and unit tests for the GTime2str
function vulnerabilities related to ASN.1 Generalized time parsing.
Changes include:
- Fix GTime2str function security issues
- Add UNITTESTS wrapper function for testing
- Add unit test suite (unit1656) to verify the fix
- Adapt test code for curl 7.78.0 API differences
Co-authored-by: Stefan Eissing
Reported-by: Dov Murik
Original commits:
- https://github.com/curl/curl/commit/3c914bc680155b32178f1f15ca8d47c7f4640afe
- https://github.com/curl/curl/commit/27959ecce75cdb2809c0bdb3286e60e08fadb519
Closes #14307
Closes #14316
Conflict:
- Adapted from lib/vtls/x509asn1.* to lib/x509asn1.* for curl 7.78.0
- Modified test code to use const char * return type instead of CURLcode
- Adapted test framework for curl 7.78.0 API compatibility
Reference: https://github.com/curl/curl/security/advisories/GHSA-xxxx
lib/x509asn1.c | 46 ++++++++----
lib/x509asn1.h | 11 +++
tests/data/Makefile.inc | 2 +-
tests/data/test1656 | 22 ++++++
tests/unit/Makefile.inc | 5 +-
tests/unit/unit1656.c | 138 ++++++++++++++++++++++++++++++++++++
6 files changed, 209 insertions(+), 15 deletions(-)
create mode 100644 tests/data/test1656
create mode 100644 tests/unit/unit1656.c
diff -Naru curl-7.78.0-original/lib/x509asn1.c curl-7.78.0-patched/lib/x509asn1.c
@@ -469,7 +469,7 @@
/* Convert an ASN.1 Generalized time to a printable string.
Return the dynamically allocated string, or NULL if an error occurs. */
- for(fracp = beg; fracp < end && *fracp >= '0' && *fracp <= '9'; fracp++)
+ for(fracp = beg; fracp < end && ISDIGIT(*fracp); fracp++)
;
/* Get seconds digits. */
@@ -488,32 +488,42 @@
return NULL;
}
- /* Scan for timezone, measure fractional seconds. */
+ /* timezone follows optional fractional seconds. */
tzp = fracp;
- fracl = 0;
+ fracl = 0; /* no fractional seconds detected so far */
if(fracp < end && (*fracp == '.' || *fracp == ',')) {
- fracp++;
- do
+ /* Have fractional seconds, e.g. "[.,]\d+". How many? */
+ fracp++; /* should be a digit char or BAD ARGUMENT */
+ tzp = fracp;
+ while(tzp < end && ISDIGIT(*tzp))
tzp++;
- while(tzp < end && *tzp >= '0' && *tzp <= '9');
- /* Strip leading zeroes in fractional seconds. */
- for(fracl = tzp - fracp - 1; fracl && fracp[fracl - 1] == '0'; fracl--)
- ;
+ if(tzp == fracp) /* never looped, no digit after [.,] */
+ return NULL;
+ fracl = tzp - fracp; /* number of fractional sec digits */
+ /* Strip trailing zeroes in fractional seconds.
+ * May reduce fracl to 0 if only '0's are present. */
+ while(fracl && fracp[fracl - 1] == '0')
+ fracl--;
}
/* Process timezone. */
- if(tzp >= end)
- ; /* Nothing to do. */
+ if(tzp >= end) {
+ tzp = "";
+ tzl = 0;
+ }
else if(*tzp == 'Z') {
- tzp = " GMT";
- end = tzp + 4;
+ sep = " ";
+ tzp = "GMT";
+ tzl = 3;
+ }
+ else if((*tzp == '+') || (*tzp == '-')) {
+ sep = " UTC";
+ tzl = end - tzp;
}
else {
sep = " ";
- tzp++;
+ tzl = end - tzp;
}
-
- tzl = end - tzp;
return curl_maprintf("%.4s-%.2s-%.2s %.2s:%.2s:%c%c%s%.*s%s%.*s",
beg, beg + 4, beg + 6,
beg + 8, beg + 10, sec1, sec2,
@@ -521,6 +531,14 @@
sep, (int)tzl, tzp);
}
+#ifdef UNITTESTS
+/* used by unit1656.c */
+const char *Curl_x509_GTime2str(const char *beg, const char *end)
+{
+ return GTime2str(beg, end);
+}
+#endif
+
/*
* Convert an ASN.1 UTC time to a printable string.
* Return the dynamically allocated string, or NULL if an error occurs.
diff -Naru curl-7.78.0-original/lib/x509asn1.h curl-7.78.0-patched/lib/x509asn1.h
@@ -129,5 +129,16 @@
const char *beg, const char *end);
CURLcode Curl_verifyhost(struct Curl_easy *data, struct connectdata *conn,
const char *beg, const char *end);
+
+#ifdef UNITTESTS
+#if defined(USE_GSKIT) || defined(USE_NSS) || defined(USE_GNUTLS) || \
+ defined(USE_WOLFSSL) || defined(USE_SCHANNEL) || defined(USE_SECTRANSP) || \
+ defined(USE_MBEDTLS)
+
+/* used by unit1656.c */
+const char *Curl_x509_GTime2str(const char *beg, const char *end);
+#endif
+#endif
+
#endif /* USE_GSKIT or USE_NSS or USE_GNUTLS or USE_WOLFSSL or USE_SCHANNEL */
#endif /* HEADER_CURL_X509ASN1_H */
diff -Naru curl-7.78.0-original/tests/data/Makefile.inc curl-7.78.0-patched/tests/data/Makefile.inc
@@ -202,7 +202,7 @@
\
test1630 test1631 test1632 test1633 test1634 \
\
-test1650 test1651 test1652 test1653 test1654 test1655 \
+test1650 test1651 test1652 test1653 test1654 test1655 test1656 \
test1660 test1661 \
\
test1700 test1701 test1702 test1703 \
diff -Naru curl-7.78.0-original/tests/data/test1656 curl-7.78.0-patched/tests/data/test1656
@@ -0,0 +1,23 @@
+<testcase>
+<info>
+<keywords>
+unittest
+Curl_x509_GTime2str
+</keywords>
+</info>
+
+#
+# Client-side
+<client>
+<server>
+none
+</server>
+<features>
+unittest
+</features>
+<name>
+Curl_x509_GTime2str unit tests
+</name>
+</client>
+</testcase>
+
diff -Naru curl-7.78.0-original/tests/unit/Makefile.inc curl-7.78.0-patched/tests/unit/Makefile.inc
@@ -34,7 +34,7 @@
unit1600 unit1601 unit1602 unit1603 unit1604 unit1605 unit1606 unit1607 \
unit1608 unit1609 unit1610 unit1611 unit1612 \
unit1620 unit1621 \
- unit1650 unit1651 unit1652 unit1653 unit1654 unit1655 \
+ unit1650 unit1651 unit1652 unit1653 unit1654 unit1655 unit1656 \
unit1660 unit1661
unit1300_SOURCES = unit1300.c $(UNITFILES)
@@ -155,6 +155,9 @@
unit1655_SOURCES = unit1655.c $(UNITFILES)
unit1655_CPPFLAGS = $(AM_CPPFLAGS)
+unit1656_SOURCES = unit1656.c $(UNITFILES)
+unit1656_CPPFLAGS = $(AM_CPPFLAGS)
+
unit1660_SOURCES = unit1660.c $(UNITFILES)
unit1660_CPPFLAGS = $(AM_CPPFLAGS)
diff -Naru curl-7.78.0-original/tests/unit/unit1656.c curl-7.78.0-patched/tests/unit/unit1656.c
@@ -0,0 +1,142 @@
+/***************************************************************************
+ * _ _ ____ _
+ * 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 "x509asn1.h"
+
+static CURLcode unit_setup(void)
+{
+ return CURLE_OK;
+}
+
+static void unit_stop(void)
+{
+
+}
+
+#if defined(USE_GSKIT) || defined(USE_NSS) || defined(USE_GNUTLS) || \
+ defined(USE_WOLFSSL) || defined(USE_SCHANNEL) || defined(USE_SECTRANSP) || \
+ defined(USE_MBEDTLS)
+
+#ifndef ARRAYSIZE
+#define ARRAYSIZE(A) (sizeof(A)/sizeof((A)[0]))
+#endif
+
+struct test_spec {
+ const char *input;
+ const char *exp_output;
+ int exp_success; /* 1 for success, 0 for failure (NULL) */
+};
+
+static struct test_spec test_specs[] = {
+ { "190321134340", "1903-21-13 43:40:00", 1 },
+ { "", NULL, 0 },
+ { "WTF", NULL, 0 },
+ { "0WTF", NULL, 0 },
+ { "19032113434", NULL, 0 },
+ { "19032113434WTF", NULL, 0 },
+ { "190321134340.", NULL, 0 },
+ { "190321134340.1", "1903-21-13 43:40:00.1", 1 },
+ { "19032113434017.0", "1903-21-13 43:40:17", 1 },
+ { "19032113434017.01", "1903-21-13 43:40:17.01", 1 },
+ { "19032113434003.001", "1903-21-13 43:40:03.001", 1 },
+ { "19032113434003.090", "1903-21-13 43:40:03.09", 1 },
+ { "190321134340Z", "1903-21-13 43:40:00 GMT", 1 },
+ { "19032113434017.0Z", "1903-21-13 43:40:17 GMT", 1 },
+ { "19032113434017.01Z", "1903-21-13 43:40:17.01 GMT", 1 },
+ { "19032113434003.001Z", "1903-21-13 43:40:03.001 GMT", 1 },
+ { "19032113434003.090Z", "1903-21-13 43:40:03.09 GMT", 1 },
+ { "190321134340CET", "1903-21-13 43:40:00 CET", 1 },
+ { "19032113434017.0CET", "1903-21-13 43:40:17 CET", 1 },
+ { "19032113434017.01CET", "1903-21-13 43:40:17.01 CET", 1 },
+ { "190321134340+02:30", "1903-21-13 43:40:00 UTC+02:30", 1 },
+ { "19032113434017.0+02:30", "1903-21-13 43:40:17 UTC+02:30", 1 },
+ { "19032113434017.01+02:30", "1903-21-13 43:40:17.01 UTC+02:30", 1 },
+ { "190321134340-3", "1903-21-13 43:40:00 UTC-3", 1 },
+ { "19032113434017.0-04", "1903-21-13 43:40:17 UTC-04", 1 },
+ { "19032113434017.01-01:10", "1903-21-13 43:40:17.01 UTC-01:10", 1 },
+};
+
+static bool do_test(struct test_spec *spec, size_t i)
+{
+ const char *result;
+ const char *in = spec->input;
+
+ result = Curl_x509_GTime2str(in, in + strlen(in));
+ if(spec->exp_success) {
+ /* Expected success */
+ if(!result) {
+ fprintf(stderr, "test %zu: expected success but got NULL\n", i);
+ return FALSE;
+ }
+ if(strcmp(spec->exp_output, result)) {
+ fprintf(stderr, "test %zu: input '%s', expected output '%s', got '%s'\n",
+ i, in, spec->exp_output, result);
+ free((void *)result);
+ return FALSE;
+ }
+ free((void *)result);
+ }
+ else {
+ /* Expected failure (NULL) */
+ if(result) {
+ fprintf(stderr, "test %zu: expected NULL but got '%s'\n", i, result);
+ free((void *)result);
+ return FALSE;
+ }
+ }
+
+ return TRUE;
+}
+
+UNITTEST_START
+{
+ size_t i;
+ bool all_ok = TRUE;
+
+ 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(test_specs); ++i) {
+ if(!do_test(&test_specs[i], i))
+ all_ok = FALSE;
+ }
+ fail_unless(all_ok, "some tests of Curl_x509_GTime2str() fails");
+
+ curl_global_cleanup();
+}
+UNITTEST_STOP
+
+#else
+
+UNITTEST_START
+{
+ puts("not tested since Curl_x509_GTime2str() is not built-in");
+}
+UNITTEST_STOP
+
+#endif
+
