From c399fba4ceeb3c4744e8e9b7842cc818c2386e88 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 7 Jun 2023 13:23:43 -0600 Subject: [PATCH 1/7] set ext pointer to null after free'ing it --- src/x509.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/x509.c b/src/x509.c index c29503d76..dc8bc4adc 100644 --- a/src/x509.c +++ b/src/x509.c @@ -1218,6 +1218,7 @@ WOLFSSL_X509_EXTENSION* wolfSSL_X509_set_ext(WOLFSSL_X509* x509, int loc) x509->ext_sk = wolfSSL_sk_new_x509_ext(); if (wolfSSL_sk_X509_EXTENSION_push(x509->ext_sk, ext) == WOLFSSL_FAILURE) { wolfSSL_X509_EXTENSION_free(ext); + ext = NULL; } FreeDecodedCert(cert); From d796aa12fc100a7b0b235734cbb114501117caa5 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 7 Jun 2023 13:29:41 -0600 Subject: [PATCH 2/7] free up memory with othername object on error --- src/x509.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/x509.c b/src/x509.c index dc8bc4adc..45a58b9ff 100644 --- a/src/x509.c +++ b/src/x509.c @@ -586,6 +586,7 @@ static int wolfssl_dns_entry_othername_to_gn(DNS_entry* dns, /* Next is: [0]. Check tag and length. */ if (GetASNTag(p, &idx, &tag, (word32)len) < 0) { + wolfSSL_ASN1_OBJECT_free(obj); goto err; } if (tag != (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | 0)) { From ebc62f8d1798fa9a53d6e683a4efdd73569fe2f6 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 7 Jun 2023 14:41:47 -0600 Subject: [PATCH 3/7] clear extension string and avoid potential double free --- src/x509.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/x509.c b/src/x509.c index 45a58b9ff..9caf3a626 100644 --- a/src/x509.c +++ b/src/x509.c @@ -223,9 +223,26 @@ WOLFSSL_X509_EXTENSION* wolfSSL_X509_EXTENSION_new(void) return newExt; } + +/* Clear out and free internal pointers of ASN.1 STRING object. + * + * @param [in] asn1 ASN.1 STRING object. + */ +static void wolfSSL_ASN1_STRING_clear(WOLFSSL_ASN1_STRING* asn1) +{ + /* Check we have an object to free. */ + if (asn1 != NULL) { + /* Dispose of dynamic data. */ + if ((asn1->length > 0) && asn1->isDynamic) { + XFREE(asn1->data, NULL, DYNAMIC_TYPE_OPENSSL); + } + XMEMSET(asn1, 0, sizeof(WOLFSSL_ASN1_STRING)); + } +} + + void wolfSSL_X509_EXTENSION_free(WOLFSSL_X509_EXTENSION* x) { - WOLFSSL_ASN1_STRING asn1; WOLFSSL_ENTER("wolfSSL_X509_EXTENSION_free"); if (x == NULL) return; @@ -234,10 +251,7 @@ void wolfSSL_X509_EXTENSION_free(WOLFSSL_X509_EXTENSION* x) wolfSSL_ASN1_OBJECT_free(x->obj); } - asn1 = x->value; - if (asn1.length > 0 && asn1.data != NULL && asn1.isDynamic) - XFREE(asn1.data, NULL, DYNAMIC_TYPE_OPENSSL); - + wolfSSL_ASN1_STRING_clear(&x->value); wolfSSL_sk_pop_free(x->ext_sk, NULL); XFREE(x, NULL, DYNAMIC_TYPE_X509_EXT); @@ -304,7 +318,7 @@ WOLFSSL_X509_EXTENSION* wolfSSL_X509_EXTENSION_create_by_OBJ( /* Prevent potential memory leaks and dangling pointers. */ wolfSSL_ASN1_OBJECT_free(ret->obj); ret->obj = NULL; - wolfSSL_ASN1_STRING_free(&ret->value); + wolfSSL_ASN1_STRING_clear(&ret->value); } if (err == 0) { From fb5413cea0be42d3db1097048cd9f1f3afc01cd6 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 7 Jun 2023 14:59:07 -0600 Subject: [PATCH 4/7] account for null terminator with SEP serail number --- wolfcrypt/src/asn.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 730fdf1a4..e74209411 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -16694,8 +16694,8 @@ static int DecodeSEP(ASNGetData* dataASN, DecodedCert* cert) cert->hwTypeSz = (int)oidLen; /* TODO: check this is the HW serial number OID - no test data. */ - /* Allocate space for HW serial number. */ - cert->hwSerialNum = (byte*)XMALLOC(serialLen, cert->heap, + /* Allocate space for HW serial number, +1 for null terminator. */ + cert->hwSerialNum = (byte*)XMALLOC(serialLen + 1, cert->heap, DYNAMIC_TYPE_X509_EXT); if (cert->hwSerialNum == NULL) { WOLFSSL_MSG("\tOut of Memory"); From 14990ad92dc4669dd59d5c345feb7d6ed7b95afa Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 7 Jun 2023 15:05:52 -0600 Subject: [PATCH 5/7] set return bio to null after free on error --- src/bio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bio.c b/src/bio.c index 595343488..5f845cf0b 100644 --- a/src/bio.c +++ b/src/bio.c @@ -2375,6 +2375,7 @@ int wolfSSL_BIO_flush(WOLFSSL_BIO* bio) if (err == 1) { wolfSSL_free(ssl); wolfSSL_BIO_free(sslBio); + sslBio = NULL; wolfSSL_BIO_free(connBio); } From 84979900a70efbe8a84823ec075ffd196838057d Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 7 Jun 2023 15:13:13 -0600 Subject: [PATCH 6/7] avoid use after free in error case --- src/pk.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pk.c b/src/pk.c index 4b4d99671..57cd5a146 100644 --- a/src/pk.c +++ b/src/pk.c @@ -360,8 +360,7 @@ static int der_to_enc_pem_alloc(unsigned char* der, int derSz, DYNAMIC_TYPE_TMP_BUFFER); if (tmpBuf == NULL) { WOLFSSL_ERROR_MSG("Extending DER buffer failed"); - XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER); - ret = 0; + ret = 0; /* der buffer is free'd at the end of the function */ } else { der = tmpBuf; From 4a4a76951210d1eec38d3d542e4daacee571fc5b Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 7 Jun 2023 15:20:23 -0600 Subject: [PATCH 7/7] check on allocation of new node before dereferencing --- src/x509.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/x509.c b/src/x509.c index 9caf3a626..908448320 100644 --- a/src/x509.c +++ b/src/x509.c @@ -13965,7 +13965,9 @@ int wolfSSL_X509_REQ_add1_attr_by_NID(WOLFSSL_X509 *req, else { if (req->reqAttributes == NULL) { req->reqAttributes = wolfSSL_sk_new_node(req->heap); - req->reqAttributes->type = STACK_TYPE_X509_REQ_ATTR; + if (req->reqAttributes != NULL) { + req->reqAttributes->type = STACK_TYPE_X509_REQ_ATTR; + } } ret = wolfSSL_sk_push(req->reqAttributes, attr); }