From b04f573e20658d9ed9dcced1bad7a7cd8d64f45b Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Wed, 10 Jun 2026 14:33:35 +1000 Subject: [PATCH] Regression testing fixes - memory allocation failure testing crl.c, internal.h: leak of sigParams requiring reorder the struct fields to that it is above memcpy part. dtl13.c: free the DRLS fragments buffer in Dtls13FreeFsmResources in case fragment is never sent. ocsp.c: only free cid if locally allocated. tls.c: make sure ecc_kse is zeroized and can be freed. tls13.c: set hsHashesEch after init so isn't lost on failure. evp_pk.c: free key on the BIO error path Fixed various tests to not leak or crash on memory allocation failure. --- src/crl.c | 6 ++++++ src/dtls13.c | 1 + src/ocsp.c | 4 +++- src/tls.c | 13 ++++++++++--- src/tls13.c | 2 +- tests/api.c | 15 +++++++++------ tests/api/test_dtls.c | 6 ++++-- tests/api/test_dtls13.c | 32 +++++++++++++++++++------------- tests/api/test_lms_xmss.c | 5 +++++ tests/api/test_ossl_p7p12.c | 4 +++- tests/api/test_ossl_x509_ext.c | 4 +++- tests/api/test_ossl_x509_str.c | 9 ++++++++- tests/api/test_pkcs7.c | 4 +++- tests/api/test_tls13.c | 2 +- tests/api/test_tls_ext.c | 22 ++++++++++++---------- wolfcrypt/src/evp_pk.c | 1 + wolfcrypt/src/wc_mldsa.c | 9 +++++---- wolfssl/internal.h | 6 ++++-- 18 files changed, 98 insertions(+), 47 deletions(-) diff --git a/src/crl.c b/src/crl.c index ed32de3f36..ddb673a97a 100644 --- a/src/crl.c +++ b/src/crl.c @@ -3014,6 +3014,12 @@ int wolfSSL_X509_CRL_sign(WOLFSSL_X509_CRL* crl, WOLFSSL_EVP_PKEY* pkey, XFREE(entry->signature, crl->heap, DYNAMIC_TYPE_CRL_ENTRY); entry->signature = NULL; } + #ifdef WC_RSA_PSS + if (entry->sigParams != NULL) { + XFREE(entry->sigParams, crl->heap, DYNAMIC_TYPE_CRL_ENTRY); + entry->sigParams = NULL; + } + #endif entry->toBeSigned = newToBeSigned; entry->tbsSz = newTbsSz; diff --git a/src/dtls13.c b/src/dtls13.c index 0de419def8..4fc2bcaf02 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -996,6 +996,7 @@ void Dtls13FreeFsmResources(WOLFSSL* ssl) /* Use 1.2 API to clear 1.2 buffers too */ DtlsMsgPoolReset(ssl); Dtls13RtxFlushBuffered(ssl, 0); + Dtls13FreeFragmentsBuffer(ssl); } static int Dtls13SendOneFragmentRtx(WOLFSSL* ssl, diff --git a/src/ocsp.c b/src/ocsp.c index 221fbd6cca..540d787513 100644 --- a/src/ocsp.c +++ b/src/ocsp.c @@ -1543,7 +1543,9 @@ WOLFSSL_OCSP_CERTID* wolfSSL_d2i_OCSP_CERTID(WOLFSSL_OCSP_CERTID** cidOut, cid->status = (CertStatus*)XMALLOC(sizeof(CertStatus), NULL, DYNAMIC_TYPE_OCSP_STATUS); if (cid->status == NULL) { - XFREE(cid, NULL, DYNAMIC_TYPE_OPENSSL); + if (isAllocated) { + XFREE(cid, NULL, DYNAMIC_TYPE_OPENSSL); + } return NULL; } XMEMSET(cid->status, 0, sizeof(CertStatus)); diff --git a/src/tls.c b/src/tls.c index 411a97271e..813ad44ade 100644 --- a/src/tls.c +++ b/src/tls.c @@ -10922,9 +10922,7 @@ int TLSX_KeyShare_HandlePqcHybridKeyServer(WOLFSSL* ssl, if (ret == 0) { ecc_kse = (KeyShareEntry*)XMALLOC(sizeof(*ecc_kse), ssl->heap, DYNAMIC_TYPE_TLSX); - pqc_kse = (KeyShareEntry*)XMALLOC(sizeof(*pqc_kse), ssl->heap, - DYNAMIC_TYPE_TLSX); - if (ecc_kse == NULL || pqc_kse == NULL) { + if (ecc_kse == NULL) { WOLFSSL_MSG("kse memory allocation failure"); ret = MEMORY_ERROR; } @@ -10932,6 +10930,15 @@ int TLSX_KeyShare_HandlePqcHybridKeyServer(WOLFSSL* ssl, if (ret == 0) { XMEMSET(ecc_kse, 0, sizeof(*ecc_kse)); ecc_kse->group = ecc_group; + + pqc_kse = (KeyShareEntry*)XMALLOC(sizeof(*pqc_kse), ssl->heap, + DYNAMIC_TYPE_TLSX); + if (pqc_kse == NULL) { + WOLFSSL_MSG("kse memory allocation failure"); + ret = MEMORY_ERROR; + } + } + if (ret == 0) { XMEMSET(pqc_kse, 0, sizeof(*pqc_kse)); pqc_kse->group = pqc_group; } diff --git a/src/tls13.c b/src/tls13.c index 5e377f40ef..beaeb4a516 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -3974,9 +3974,9 @@ static int EchCalcAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz, * don't add a cookie */ if (ret == 0) { ret = InitHandshakeHashes(ssl); + ssl->hsHashesEch = ssl->hsHashes; } if (ret == 0) { - ssl->hsHashesEch = ssl->hsHashes; AddTls13HandShakeHeader(messageHashHeader, (word32)hashSz, 0, 0, message_hash, ssl); ret = HashRaw(ssl, messageHashHeader, headerSz); diff --git a/tests/api.c b/tests/api.c index 550a04ba14..01f61917b0 100644 --- a/tests/api.c +++ b/tests/api.c @@ -3740,12 +3740,14 @@ static int test_wolfSSL_add_to_chain_overflow(void) wolfSSL_X509_free(x509); x509 = NULL; - /* Now ctx->certificate is set, next add goes to certChain via - * wolfssl_add_to_chain. Fake a chain whose length is near UINT32_MAX - * so the size calculation (len + CERT_HEADER_SZ + certSz) overflows. */ - fakeChain = (DerBuffer*)XMALLOC(sizeof(DerBuffer) + 16, ctx->heap, - DYNAMIC_TYPE_CERT); - ExpectNotNull(fakeChain); + if (EXPECT_SUCCESS()) { + /* Now ctx->certificate is set, next add goes to certChain via + * wolfssl_add_to_chain. Fake a chain whose length is near UINT32_MAX + * so the size calculation (len + CERT_HEADER_SZ + certSz) overflows. */ + fakeChain = (DerBuffer*)XMALLOC(sizeof(DerBuffer) + 16, ctx->heap, + DYNAMIC_TYPE_CERT); + ExpectNotNull(fakeChain); + } if (EXPECT_SUCCESS()) { XMEMSET(fakeChain, 0, sizeof(DerBuffer) + 16); fakeChain->buffer = (byte*)(fakeChain + 1); @@ -15185,6 +15187,7 @@ static int test_wolfSSL_Tls13_ECH_ch2_no_ech(void) /* one round: client sends CH1, server processes it and sends HRR */ (void)test_ssl_memio_do_handshake(&test_ctx, 1, NULL); + ExpectIntEQ(wolfSSL_get_error(test_ctx.s_ssl, 0), WOLFSSL_ERROR_WANT_READ); /* server must have committed to ECH acceptance in the HRR */ ExpectIntEQ(test_ctx.s_ssl->options.serverState, diff --git a/tests/api/test_dtls.c b/tests/api/test_dtls.c index 572dda6c94..b33bc26916 100644 --- a/tests/api/test_dtls.c +++ b/tests/api/test_dtls.c @@ -3724,7 +3724,7 @@ int test_wolfSSL_dtls_stateless_hrr_group(void) EXPECT_DECLS; #if defined(WOLFSSL_SEND_HRR_COOKIE) size_t i; - word32 initHash; + word32 initHash = 0; struct { method_provider client_meth; method_provider server_meth; @@ -3764,7 +3764,9 @@ int test_wolfSSL_dtls_stateless_hrr_group(void) wolfSSL_SetLoggingPrefix("server"); wolfSSL_dtls_set_using_nonblock(ssl_s, 1); - initHash = test_wolfSSL_dtls_stateless_HashWOLFSSL(ssl_s); + if (EXPECT_SUCCESS()) { + initHash = test_wolfSSL_dtls_stateless_HashWOLFSSL(ssl_s); + } /* Set groups and disable key shares. This ensures that only the given * groups are in the SupportedGroups extension and that an empty key diff --git a/tests/api/test_dtls13.c b/tests/api/test_dtls13.c index b74740eb4c..3be0df3ebf 100644 --- a/tests/api/test_dtls13.c +++ b/tests/api/test_dtls13.c @@ -1100,11 +1100,13 @@ int test_dtls13_ack_order(void) ExpectIntEQ(Dtls13WriteAckMessage(ssl_c, ssl_c->dtls13Rtx.seenRecords, ssl_c->dtls13Rtx.seenRecordsCount, &length), 0); - /* must zero the span reserved for the header to avoid read of uninited - * data. - */ - XMEMSET(ssl_c->buffers.outputBuffer.buffer, 0, - 5 /* DTLS13_UNIFIED_HEADER_SIZE */); + if (EXPECT_SUCCESS()) { + /* must zero the span reserved for the header to avoid read of uninited + * data. + */ + XMEMSET(ssl_c->buffers.outputBuffer.buffer, 0, + 5 /* DTLS13_UNIFIED_HEADER_SIZE */); + } /* N * RecordNumber + 2 extra bytes for length */ ExpectIntEQ(length, sizeof(expected_output) + 2); ExpectNotNull(mymemmem(ssl_c->buffers.outputBuffer.buffer, @@ -1162,14 +1164,18 @@ int test_dtls13_ack_overflow(void) w64From32(0, (word32)DTLS13_ACK_MAX_RECORDS)), 0); ExpectIntEQ(ssl_c->dtls13Rtx.seenRecordsCount, DTLS13_ACK_MAX_RECORDS); - /* Bypass the insert guard to force the list one element over the limit, - * then verify Dtls13WriteAckMessage errors out instead of overflowing */ - ssl_c->dtls13Rtx.seenRecordsCount = 0; - ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 1), - w64From32(0, (word32)DTLS13_ACK_MAX_RECORDS)), 0); - ssl_c->dtls13Rtx.seenRecordsCount = (word16)(DTLS13_ACK_MAX_RECORDS + 1); - ExpectIntEQ(Dtls13WriteAckMessage(ssl_c, ssl_c->dtls13Rtx.seenRecords, - ssl_c->dtls13Rtx.seenRecordsCount, &length), BUFFER_E); + if (EXPECT_SUCCESS()) { + /* Bypass the insert guard to force the list one element over the limit, + * then verify Dtls13WriteAckMessage errors out instead of + * overflowing. */ + ssl_c->dtls13Rtx.seenRecordsCount = 0; + ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 1), + w64From32(0, (word32)DTLS13_ACK_MAX_RECORDS)), 0); + ssl_c->dtls13Rtx.seenRecordsCount = + (word16)(DTLS13_ACK_MAX_RECORDS + 1); + ExpectIntEQ(Dtls13WriteAckMessage(ssl_c, ssl_c->dtls13Rtx.seenRecords, + ssl_c->dtls13Rtx.seenRecordsCount, &length), BUFFER_E); + } wolfSSL_free(ssl_c); wolfSSL_CTX_free(ctx_c); diff --git a/tests/api/test_lms_xmss.c b/tests/api/test_lms_xmss.c index f99b1ed574..24a7d51d10 100644 --- a/tests/api/test_lms_xmss.c +++ b/tests/api/test_lms_xmss.c @@ -115,6 +115,8 @@ int test_wc_LmsKey_sign_verify(void) int i; int numSigs = 5; + XMEMSET(&key, 0, sizeof(key)); + ExpectIntEQ(wc_InitRng(&rng), 0); remove(LMS_TEST_PRIV_KEY_FILE); @@ -169,6 +171,9 @@ int test_wc_LmsKey_reload_cache(void) /* Sign 33 times to advance q past the 32-entry cache window. */ int preSigs = 33; + XMEMSET(&key, 0, sizeof(key)); + XMEMSET(&vkey, 0, sizeof(vkey)); + ExpectIntEQ(wc_InitRng(&rng), 0); /* Phase 1: Generate key and sign past cache window */ diff --git a/tests/api/test_ossl_p7p12.c b/tests/api/test_ossl_p7p12.c index 03590a3ad7..53f3d8749d 100644 --- a/tests/api/test_ossl_p7p12.c +++ b/tests/api/test_ossl_p7p12.c @@ -142,7 +142,9 @@ int test_wolfSSL_PKCS7_certs(void) ExpectNotNull(info = sk_X509_INFO_shift(info_sk)); if (EXPECT_SUCCESS() && info != NULL) { ExpectIntGT(sk_X509_push(sk, info->x509), 0); - info->x509 = NULL; + if (EXPECT_SUCCESS()) { + info->x509 = NULL; + } } X509_INFO_free(info); } diff --git a/tests/api/test_ossl_x509_ext.c b/tests/api/test_ossl_x509_ext.c index fb0967be5e..05d2370a87 100644 --- a/tests/api/test_ossl_x509_ext.c +++ b/tests/api/test_ossl_x509_ext.c @@ -627,7 +627,9 @@ int test_wolfSSL_X509_add_ext_dirname_san_rejected(void) sk->type = STACK_TYPE_GEN_NAME; } ExpectIntGT(wolfSSL_sk_GENERAL_NAME_push(sk, gn), 0); - gn = NULL; /* sk owns gn now */ + if (EXPECT_SUCCESS()) { + gn = NULL; /* sk owns gn now */ + } ExpectNotNull(obj = wolfSSL_OBJ_nid2obj(NID_subject_alt_name)); if (obj != NULL) { diff --git a/tests/api/test_ossl_x509_str.c b/tests/api/test_ossl_x509_str.c index 96218daafa..09bd7b58ea 100644 --- a/tests/api/test_ossl_x509_str.c +++ b/tests/api/test_ossl_x509_str.c @@ -2020,10 +2020,14 @@ int test_wolfSSL_CTX_set_cert_store(void) /* This should push intermediates from store->certs into the CM */ SSL_CTX_set_cert_store(ctx, store); + if (SSL_CTX_get_cert_store(ctx) != store) { + X509_STORE_free(store); + store = NULL; + } /* After set_cert_store, store->certs and store->trusted should be NULLed * to signal CTX ownership */ - if (EXPECT_SUCCESS()) { + if ((store != NULL) && EXPECT_SUCCESS()) { ExpectNull(store->certs); ExpectNull(store->trusted); } @@ -2067,6 +2071,9 @@ int test_wolfSSL_CTX_set_cert_store(void) /* Attach empty store first */ SSL_CTX_set_cert_store(ctx, store); + if (!EXPECT_SUCCESS()) { + X509_STORE_free(store); + } /* Now add certs after ownership transfer */ ExpectNotNull(rootCa = wolfSSL_X509_load_certificate_file(caCert, diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index 4aae3382ce..dc178359f0 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -3997,7 +3997,9 @@ int test_wc_PKCS7_DecodeEncryptedKeyPackage(void) /* Verify that the build_test_EncryptedKeyPackage can format as expected. */ ExpectIntLT(inner_cms_der_size, 124); } - build_test_EncryptedKeyPackage(ekp_cms_der, &ekp_cms_der_size, inner_cms_der, inner_cms_der_size, test_messages[test_msg].msg_content_type, test_vector); + if (EXPECT_SUCCESS()) { + build_test_EncryptedKeyPackage(ekp_cms_der, &ekp_cms_der_size, inner_cms_der, inner_cms_der_size, test_messages[test_msg].msg_content_type, test_vector); + } XFREE(inner_cms_der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index 24950cb6d5..b3ec1ff0d9 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -5557,7 +5557,7 @@ int test_tls13_short_session_ticket(void) * session. The session object is accessible; replicate the exact * vulnerable arithmetic: ticket + length - ID_LEN with length=5. * With the fix, sessIdLen is capped to length so no underflow. */ - { + if (EXPECT_SUCCESS()) { byte shortTicket[5] = { 0xBB, 0xCC, 0xDD, 0xEE, 0xFF }; word32 length = sizeof(shortTicket); word32 sessIdLen = ID_LEN; diff --git a/tests/api/test_tls_ext.c b/tests/api/test_tls_ext.c index 49edaf29f5..edfdb775bf 100644 --- a/tests/api/test_tls_ext.c +++ b/tests/api/test_tls_ext.c @@ -390,16 +390,18 @@ int test_tls13_hrr_cipher_suite_mismatch(void) ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, "TLS13-AES256-GCM-SHA384"), WOLFSSL_SUCCESS); - /* CH2 */ - (void)wolfSSL_connect(ssl_c); - (void)wolfSSL_accept(ssl_s); - (void)wolfSSL_connect(ssl_c); - /* The cipher-suite mismatch is caught server-side; the server's - * alert reaches the client, so either peer can surface it. */ - ret = wolfSSL_get_error(ssl_s, 0); - if (ret != WC_NO_ERR_TRACE(INVALID_PARAMETER)) - ret = wolfSSL_get_error(ssl_c, 0); - ExpectIntEQ(ret, WC_NO_ERR_TRACE(INVALID_PARAMETER)); + if (EXPECT_SUCCESS()) { + /* CH2 */ + (void)wolfSSL_connect(ssl_c); + (void)wolfSSL_accept(ssl_s); + (void)wolfSSL_connect(ssl_c); + /* The cipher-suite mismatch is caught server-side; the server's + * alert reaches the client, so either peer can surface it. */ + ret = wolfSSL_get_error(ssl_s, 0); + if (ret != WC_NO_ERR_TRACE(INVALID_PARAMETER)) + ret = wolfSSL_get_error(ssl_c, 0); + ExpectIntEQ(ret, WC_NO_ERR_TRACE(INVALID_PARAMETER)); + } wolfSSL_free(ssl_c); wolfSSL_free(ssl_s); diff --git a/wolfcrypt/src/evp_pk.c b/wolfcrypt/src/evp_pk.c index f719bb5264..777f012e3c 100644 --- a/wolfcrypt/src/evp_pk.c +++ b/wolfcrypt/src/evp_pk.c @@ -1317,6 +1317,7 @@ WOLFSSL_EVP_PKEY* wolfSSL_d2i_PrivateKey_bio(WOLFSSL_BIO* bio, if (wolfSSL_BIO_get_len(bio) <= 0) { WOLFSSL_MSG("Failed to write memory to bio"); XFREE(mem, bio->heap, DYNAMIC_TYPE_TMP_BUFFER); + wolfSSL_EVP_PKEY_free(key); return NULL; } } diff --git a/wolfcrypt/src/wc_mldsa.c b/wolfcrypt/src/wc_mldsa.c index 3e08dad88a..721242a50d 100644 --- a/wolfcrypt/src/wc_mldsa.c +++ b/wolfcrypt/src/wc_mldsa.c @@ -10004,7 +10004,7 @@ static int mldsa_verify_ctx_msg(wc_MlDsaKey* key, const byte* ctx, byte tr[MLDSA_TR_SZ]; byte* mu = tr; - if (key == NULL) { + if ((key == NULL) || (key->params == NULL)) { ret = BAD_FUNC_ARG; } @@ -10048,7 +10048,7 @@ static int mldsa_verify_msg(wc_MlDsaKey* key, const byte* msg, byte tr[MLDSA_TR_SZ]; byte* mu = tr; - if (key == NULL) { + if ((key == NULL) || (key->params == NULL)) { ret = BAD_FUNC_ARG; } @@ -10098,7 +10098,7 @@ static int mldsa_verify_ctx_hash(wc_MlDsaKey* key, const byte* ctx, byte oidMsgHash[MLDSA_HASH_OID_LEN + WC_MAX_DIGEST_SIZE]; word32 oidMsgHashLen = 0; - if (key == NULL) { + if ((key == NULL) || (key->params == NULL)) { ret = BAD_FUNC_ARG; } /* Check that the input hash length is valid. */ @@ -10738,7 +10738,8 @@ int wc_MlDsaKey_VerifyMu(wc_MlDsaKey* key, const byte* sig, word32 sigLen, int ret = 0; /* Validate parameters. */ - if ((key == NULL) || (sig == NULL) || (mu == NULL) || (res == NULL)) { + if ((key == NULL) || (key->params == NULL) || (sig == NULL) || + (mu == NULL) || (res == NULL)) { ret = BAD_FUNC_ARG; } if ((ret == 0) && (muLen != MLDSA_MU_SZ)) { diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 80dcb24c70..51e73b5ad3 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2519,6 +2519,9 @@ typedef struct CRL_Entry CRL_Entry; struct CRL_Entry { byte* toBeSigned; byte* signature; +#ifdef WC_RSA_PSS + byte* sigParams; /* buffer with signature parameters */ +#endif #if defined(OPENSSL_EXTRA) WOLFSSL_X509_NAME* issuer; /* X509_NAME type issuer */ #endif @@ -2549,11 +2552,10 @@ struct CRL_Entry { int verified; word32 tbsSz; word32 signatureSz; - word32 signatureOID; #ifdef WC_RSA_PSS word32 sigParamsSz; /* length of signature parameters */ - byte* sigParams; /* buffer with signature parameters */ #endif + word32 signatureOID; #if !defined(NO_SKID) && !defined(NO_ASN) byte extAuthKeyId[KEYID_SIZE]; byte extAuthKeyIdSet:1; /* Auth key identifier set indicator */