From 13c488e2781cc6acb195b6dbb70c023df1b90c8f Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Thu, 27 Mar 2025 20:35:20 -0400 Subject: [PATCH 1/7] Validate asn date based on string length --- wolfcrypt/src/asn.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 30afd46e55..fbf9862012 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -15984,6 +15984,26 @@ static WC_INLINE int GetTime_Long(long* value, const byte* date, int* idx) int ExtractDate(const unsigned char* date, unsigned char format, struct tm* certTime, int* idx) { + int i = *idx; + + /* Validate date string length based on format */ + if (format == ASN_UTC_TIME) { + /* UTCTime format requires YYMMDDHHMMSSZ. + * subtract 1 to exclude null terminator. */ + if (XSTRLEN((const char*)date + i) < (ASN_UTC_TIME_SIZE - 1)) { + return ASN_PARSE_E; + } + } + else if (format == ASN_GENERALIZED_TIME) { + /* GeneralizedTime format requires YYYYMMDDHHMMSSZ. + * subtract 1 to exclude null terminator. */ + if (XSTRLEN((const char*)date + i) < (ASN_GENERALIZED_TIME_SIZE - 1)) { + return ASN_PARSE_E; + } + } else { + return ASN_PARSE_E; + } + XMEMSET(certTime, 0, sizeof(struct tm)); /* Get the first two bytes of the year (century) */ From ef82fd4b42636af9df8abc71b727856675532c86 Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Fri, 28 Mar 2025 17:52:46 -0400 Subject: [PATCH 2/7] Stop assuming dates are NULL terminated --- wolfcrypt/src/asn.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index fbf9862012..b8ad4f7020 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -15986,22 +15986,22 @@ int ExtractDate(const unsigned char* date, unsigned char format, { int i = *idx; - /* Validate date string length based on format */ + /* Validate date string length based on format Can not assume null + * terminated strings. Must check for the 'Z'. Subtract 2; one for zero + * indexing and one to exclude null terminator built into macro values */ if (format == ASN_UTC_TIME) { - /* UTCTime format requires YYMMDDHHMMSSZ. - * subtract 1 to exclude null terminator. */ - if (XSTRLEN((const char*)date + i) < (ASN_UTC_TIME_SIZE - 1)) { - return ASN_PARSE_E; + /* UTCTime format requires YYMMDDHHMMSSZ. */ + if (date[i + ASN_UTC_TIME_SIZE - 2] != 'Z') { + return 0; } } else if (format == ASN_GENERALIZED_TIME) { - /* GeneralizedTime format requires YYYYMMDDHHMMSSZ. - * subtract 1 to exclude null terminator. */ - if (XSTRLEN((const char*)date + i) < (ASN_GENERALIZED_TIME_SIZE - 1)) { - return ASN_PARSE_E; + /* GeneralizedTime format requires YYYYMMDDHHMMSSZ. */ + if (date[ i + ASN_GENERALIZED_TIME_SIZE - 2] != 'Z') { + return 0; } } else { - return ASN_PARSE_E; + return 0; } XMEMSET(certTime, 0, sizeof(struct tm)); From 67f586e047221a3b9a95696c5d656911dc1f0f69 Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Fri, 4 Apr 2025 10:47:14 -0400 Subject: [PATCH 3/7] Comment fixup --- wolfcrypt/src/asn.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index b8ad4f7020..ee439b9574 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -15986,9 +15986,10 @@ int ExtractDate(const unsigned char* date, unsigned char format, { int i = *idx; - /* Validate date string length based on format Can not assume null - * terminated strings. Must check for the 'Z'. Subtract 2; one for zero - * indexing and one to exclude null terminator built into macro values */ + /* Validate date string length based on format. Can not assume null + * terminated strings. Must check for the 'Z'. + * Subtract 2; one for zero indexing and one to exclude null terminator + * built into macro values. */ if (format == ASN_UTC_TIME) { /* UTCTime format requires YYMMDDHHMMSSZ. */ if (date[i + ASN_UTC_TIME_SIZE - 2] != 'Z') { From d6f9918f336eebc91a52e07110c0d874dd1bc64d Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Fri, 4 Apr 2025 10:49:09 -0400 Subject: [PATCH 4/7] Drop else --- wolfcrypt/src/asn.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index ee439b9574..be89ec618c 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -16001,7 +16001,8 @@ int ExtractDate(const unsigned char* date, unsigned char format, if (date[ i + ASN_GENERALIZED_TIME_SIZE - 2] != 'Z') { return 0; } - } else { + } + else { return 0; } From b98bf85146037c928934e9da2ec620a85f6d6725 Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Tue, 22 Apr 2025 15:02:08 -0400 Subject: [PATCH 5/7] add a test --- certs/crl/bad_time_fmt.pem | 13 +++++++++++++ tests/api.c | 10 +++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 certs/crl/bad_time_fmt.pem diff --git a/certs/crl/bad_time_fmt.pem b/certs/crl/bad_time_fmt.pem new file mode 100644 index 0000000000..589771d8f5 --- /dev/null +++ b/certs/crl/bad_time_fmt.pem @@ -0,0 +1,13 @@ +-----BEGIN X509 CRL----- +MIIB7DCB1QIBATANBgkqhkiG9w0BAQsFADBOMQswCQYDVQQGEwJVUzELMAkGA1UE +CAwCVVMxCzAJBgNVBAcMAlVTMQswCQYDVQQKDAJVUzELMAkGA1UEAwwCVVMxCzAJ +BgNVBAsMAlVTGA0yNDAxMjMwMDAwMDBaGA0zNDAxMjAwMDAwMDBaMDUwMwIUHIAC +LvgfJAXulqYS3LYf4KxwHl4XDTI1MDMxMzAyNDQ0MFowDDAKBgNVHRUEAwoBBqAc +MBowGAYDVR0UBBECDxnP/97adO3y9qRGDM7hQDANBgkqhkiG9w0BAQsFAAOCAQEA +aDY9jBdAJiAujUkaLYLVtzNWF/0SxD5CB4dYIcZMqtPKLn5ykcxkXvnRbVihJ+Kn +AAv9Fkn5iwj77EGwxNjyZktQ4gAmcMhCTBEcAHbmi92tHttot9Sr44+CN+0NaaQD +OflIeVw7Zir90TWufjScy8/e7FkVm+aD5CicrbJWqoe21pB1Q1jS49iNrZzqZ2vw +HLiqNAzpecxwUih/YPe5+CBk5Nq4vICeieGVC/JO9r5SkdDwWQTl0I3kSK6n4Jh7 +53FmIen80F2ZZuZu4/fhJ7C4rlr6W9i6FrK06s5mk1PeYFHKhCkwI8wp8cIudJQD +lLsK2u4CTcuTKdbDLsszYA== +-----END X509 CRL----- diff --git a/tests/api.c b/tests/api.c index e67f38f7d5..500d74cd18 100644 --- a/tests/api.c +++ b/tests/api.c @@ -46392,9 +46392,17 @@ static int test_sk_X509_CRL(void) ExpectIntEQ(BIO_get_mem_data(bio, NULL), 1324); #endif BIO_free(bio); - + bio = NULL; wolfSSL_X509_CRL_free(crl); crl = NULL; + +#ifndef NO_ASN_TIME + /* Test CRL with invalid GeneralizedTime */ + ExpectNotNull(bio = BIO_new_file("./certs/crl/bad_time_fmt.pem", "rb")); + ExpectNull(crl = PEM_read_bio_X509_CRL(bio, NULL, NULL, NULL)); + BIO_free(bio); + bio = NULL; + #endif /* NO_ASN_TIME */ #endif #if !defined(NO_FILESYSTEM) && !defined(NO_STDIO_FILESYSTEM) From c7a2d01e567bd4d1fa3157a9d3ac33d63d5f833e Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Mon, 12 May 2025 15:50:37 -0400 Subject: [PATCH 6/7] squash me --- src/x509.c | 18 ++++++------------ tests/api.c | 13 ++++++++++--- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/x509.c b/src/x509.c index eda280e735..67e090ddb8 100644 --- a/src/x509.c +++ b/src/x509.c @@ -9046,13 +9046,10 @@ static int X509CRLPrintDates(WOLFSSL_BIO* bio, WOLFSSL_X509_CRL* crl, } if (crl->crlList->lastDate[0] != 0) { - if (GetTimeString(crl->crlList->lastDate, ASN_UTC_TIME, + if (GetTimeString(crl->crlList->lastDate, crl->crlList->lastDateFormat, tmp, MAX_WIDTH) != WOLFSSL_SUCCESS) { - if (GetTimeString(crl->crlList->lastDate, ASN_GENERALIZED_TIME, - tmp, MAX_WIDTH) != WOLFSSL_SUCCESS) { - WOLFSSL_MSG("Error getting last update date"); - return WOLFSSL_FAILURE; - } + WOLFSSL_MSG("Error getting last update date"); + return WOLFSSL_FAILURE; } } else { @@ -9077,13 +9074,10 @@ static int X509CRLPrintDates(WOLFSSL_BIO* bio, WOLFSSL_X509_CRL* crl, } if (crl->crlList->nextDate[0] != 0) { - if (GetTimeString(crl->crlList->nextDate, ASN_UTC_TIME, + if (GetTimeString(crl->crlList->nextDate, crl->crlList->nextDateFormat, tmp, MAX_WIDTH) != WOLFSSL_SUCCESS) { - if (GetTimeString(crl->crlList->nextDate, ASN_GENERALIZED_TIME, - tmp, MAX_WIDTH) != WOLFSSL_SUCCESS) { - WOLFSSL_MSG("Error getting next update date"); - return WOLFSSL_FAILURE; - } + WOLFSSL_MSG("Error getting next update date"); + return WOLFSSL_FAILURE; } } else { diff --git a/tests/api.c b/tests/api.c index 500d74cd18..ce43607eb0 100644 --- a/tests/api.c +++ b/tests/api.c @@ -46399,11 +46399,18 @@ static int test_sk_X509_CRL(void) #ifndef NO_ASN_TIME /* Test CRL with invalid GeneralizedTime */ ExpectNotNull(bio = BIO_new_file("./certs/crl/bad_time_fmt.pem", "rb")); - ExpectNull(crl = PEM_read_bio_X509_CRL(bio, NULL, NULL, NULL)); + ExpectNotNull(crl = PEM_read_bio_X509_CRL(bio, NULL, NULL, NULL)); BIO_free(bio); bio = NULL; - #endif /* NO_ASN_TIME */ -#endif + ExpectNotNull(bio = BIO_new(BIO_s_mem())); + ExpectIntEQ(wolfSSL_X509_CRL_print(bio, crl), WOLFSSL_FAILURE); + + BIO_free(bio); + bio = NULL; + wolfSSL_X509_CRL_free(crl); + crl = NULL; +#endif /* !NO_ASN_TIME */ +#endif /* !NO_BIO */ #if !defined(NO_FILESYSTEM) && !defined(NO_STDIO_FILESYSTEM) ExpectTrue((fp = XFOPEN("./certs/crl/crl.der", "rb")) != XBADFILE); From 607272ee1c89753ff980581100201a7ba7019cd5 Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Mon, 12 May 2025 16:23:09 -0400 Subject: [PATCH 7/7] Added a new file; added to include.am --- certs/crl/include.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/certs/crl/include.am b/certs/crl/include.am index d3194933a4..6f7f6f26bf 100644 --- a/certs/crl/include.am +++ b/certs/crl/include.am @@ -16,7 +16,8 @@ EXTRA_DIST += \ certs/crl/wolfssl.cnf \ certs/crl/crl.der \ certs/crl/crl2.der \ - certs/crl/crl_rsapss.pem + certs/crl/crl_rsapss.pem \ + certs/crl/bad_time_fmt.pem EXTRA_DIST += \ certs/crl/crl.revoked \