diff --git a/src/crl.c b/src/crl.c index ed32de3f36..51f42bb249 100644 --- a/src/crl.c +++ b/src/crl.c @@ -1337,6 +1337,22 @@ static CRL_Entry* DupCRL_Entry(const CRL_Entry* ent, void* heap) XMEMCPY((byte*)dupl + copyOffset, (byte*)ent + copyOffset, sizeof(CRL_Entry) - copyOffset); + /* The struct copy above aliased ent's owned pointers into dupl. Clear them + * (each is deep-copied below) so a partial-failure CRL_Entry_free(dupl) + * frees only dupl's own allocations, not ent's. */ + dupl->next = NULL; +#ifndef CRL_STATIC_REVOKED_LIST + dupl->certs = NULL; +#endif +#ifdef OPENSSL_EXTRA + dupl->issuer = NULL; +#endif + dupl->toBeSigned = NULL; + dupl->signature = NULL; +#ifdef WC_RSA_PSS + dupl->sigParams = NULL; +#endif + #ifndef CRL_STATIC_REVOKED_LIST dupl->certs = DupRevokedCertList(ent->certs, heap); if (ent->certs != NULL && dupl->certs == NULL) { diff --git a/src/internal.c b/src/internal.c index 8b84e7f2b9..c7d225289f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7482,19 +7482,22 @@ int SetSSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup) int InitHandshakeHashes(WOLFSSL* ssl) { int ret; + HS_Hashes* newHsHashes; - /* make sure existing handshake hashes are free'd */ - if (ssl->hsHashes != NULL) { - FreeHandshakeHashes(ssl); - } - - /* allocate handshake hashes */ - ssl->hsHashes = (HS_Hashes*)XMALLOC(sizeof(HS_Hashes), ssl->heap, + /* Allocate before freeing the old hashes so an allocation failure leaves + * the existing hashes intact instead of a NULL dangling pointer that a + * handshake retry would dereference. */ + newHsHashes = (HS_Hashes*)XMALLOC(sizeof(HS_Hashes), ssl->heap, DYNAMIC_TYPE_HASHES); - if (ssl->hsHashes == NULL) { + if (newHsHashes == NULL) { WOLFSSL_MSG("HS_Hashes Memory error"); return MEMORY_E; } + + if (ssl->hsHashes != NULL) { + FreeHandshakeHashes(ssl); + } + ssl->hsHashes = newHsHashes; XMEMSET(ssl->hsHashes, 0, sizeof(HS_Hashes)); #if !defined(NO_MD5) && !defined(NO_OLD_TLS) @@ -7598,22 +7601,25 @@ int InitHandshakeHashesAndCopy(WOLFSSL* ssl, HS_Hashes* source, HS_Hashes** destination) { int ret; + HS_Hashes* newHashes; if ((ssl == NULL) || (source == NULL) || (destination == NULL)) return BAD_FUNC_ARG; - /* If *destination is already allocated, its constituent hashes need to be - * freed, else they would leak. */ - Free_HS_Hashes(*destination, ssl->heap); - - /* allocate handshake hashes */ - *destination = (HS_Hashes*)XMALLOC(sizeof(HS_Hashes), ssl->heap, + /* Allocate before freeing the old hashes so an allocation failure leaves + * *destination intact instead of a NULL dangling pointer. */ + newHashes = (HS_Hashes*)XMALLOC(sizeof(HS_Hashes), ssl->heap, DYNAMIC_TYPE_HASHES); - if (*destination == NULL) { + if (newHashes == NULL) { WOLFSSL_MSG("HS_Hashes Memory error"); return MEMORY_E; } + /* If *destination is already allocated, its constituent hashes need to be + * freed, else they would leak. */ + Free_HS_Hashes(*destination, ssl->heap); + *destination = newHashes; + /* Note we can't call InitHandshakeHashes() here, because the copy methods * overwrite the entire dest low level hash struct. With some hashes and * settings (e.g. SHA-2 hashes with WOLFSSL_SMALL_STACK_CACHE), internal 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/tests/api.c b/tests/api.c index e81fee2b5d..bea2764ce1 100644 --- a/tests/api.c +++ b/tests/api.c @@ -3746,8 +3746,8 @@ static int test_wolfSSL_add_to_chain_overflow(void) /* 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); + fakeChain = (DerBuffer*)XMALLOC(sizeof(DerBuffer) + 16, + (ctx != NULL) ? ctx->heap : NULL, DYNAMIC_TYPE_CERT); ExpectNotNull(fakeChain); if (EXPECT_SUCCESS()) { XMEMSET(fakeChain, 0, sizeof(DerBuffer) + 16); @@ -14167,7 +14167,8 @@ static int test_wolfSSL_Tls13_ECH_all_algos_ex(void) ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.s_ssl), WOLFSSL_ECH_STATUS_ACCEPTED); - if (echCbTestKemID != 0 && echCbTestKdfID != 0 && echCbTestAeadID != 0) { + if (EXPECT_SUCCESS() && echCbTestKemID != 0 && echCbTestKdfID != 0 && + echCbTestAeadID != 0) { TLSX* echX = TLSX_Find(test_ctx.c_ssl->extensions, TLSX_ECH); ExpectNotNull(echX); if (echX != NULL) { diff --git a/tests/api/test_dtls.c b/tests/api/test_dtls.c index f160cdc594..631fc195a4 100644 --- a/tests/api/test_dtls.c +++ b/tests/api/test_dtls.c @@ -3765,7 +3765,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; @@ -3803,9 +3803,11 @@ 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); + /* guard: ssl_s NULL on setup alloc failure */ + if (ssl_s != NULL) { + wolfSSL_dtls_set_using_nonblock(ssl_s, 1); + 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..dcc1706831 100644 --- a/tests/api/test_dtls13.c +++ b/tests/api/test_dtls13.c @@ -1100,11 +1100,11 @@ 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 */); + /* zero the header span to avoid read of uninited data (guard: ssl_c NULL + * on setup alloc failure) */ + if (ssl_c != NULL) + 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,12 +1162,14 @@ 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; + /* Force the list one over the limit, then verify Dtls13WriteAckMessage + * errors out instead of overflowing (guard: ssl_c NULL on setup failure) */ + if (ssl_c != NULL) + 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); + if (ssl_c != NULL) + 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); diff --git a/tests/api/test_lms_xmss.c b/tests/api/test_lms_xmss.c index 63a0c4ba9a..1eac4b2dba 100644 --- a/tests/api/test_lms_xmss.c +++ b/tests/api/test_lms_xmss.c @@ -118,6 +118,10 @@ int test_wc_LmsKey_sign_verify(void) int i; int numSigs = 5; + /* Zero so cleanup is safe if an early alloc failure skips init. */ + XMEMSET(&key, 0, sizeof(key)); + XMEMSET(&rng, 0, sizeof(rng)); + ExpectIntEQ(wc_InitRng(&rng), 0); remove(LMS_TEST_PRIV_KEY_FILE); @@ -172,6 +176,11 @@ int test_wc_LmsKey_reload_cache(void) /* Sign 33 times to advance q past the 32-entry cache window. */ int preSigs = 33; + /* Zero so cleanup is safe if an early alloc failure skips init. */ + XMEMSET(&key, 0, sizeof(key)); + XMEMSET(&vkey, 0, sizeof(vkey)); + XMEMSET(&rng, 0, sizeof(rng)); + ExpectIntEQ(wc_InitRng(&rng), 0); /* Phase 1: Generate key and sign past cache window */ @@ -857,7 +866,8 @@ static int rfc9802_gen_chain(void* caKey, int caKeyType, int caSigType, ExpectNotNull(leafDer = (byte*)XMALLOC(derCap, NULL, DYNAMIC_TYPE_TMP_BUFFER)); ExpectIntEQ(wc_ecc_init(&leafKey), 0); - leafKeyInit = 1; + if (EXPECT_SUCCESS()) /* only flag for free if init ran and succeeded */ + leafKeyInit = 1; ExpectIntEQ(wc_ecc_make_key(rng, 32, &leafKey), 0); /* Self-signed CA root. */ @@ -967,6 +977,10 @@ int test_rfc9802_lms_x509_gen(void) LmsKey key; WC_RNG rng; + /* Zero so cleanup is safe if an early alloc failure skips init. */ + XMEMSET(&key, 0, sizeof(key)); + XMEMSET(&rng, 0, sizeof(rng)); + ExpectIntEQ(wc_InitRng(&rng), 0); /* Single-level LMS (L1-H5-W8). */ @@ -1172,6 +1186,10 @@ int test_rfc9802_xmss_x509_gen(void) XmssKey key; WC_RNG rng; + /* Zero so cleanup is safe if an early alloc failure skips init. */ + XMEMSET(&key, 0, sizeof(key)); + XMEMSET(&rng, 0, sizeof(rng)); + ExpectIntEQ(wc_InitRng(&rng), 0); /* Single-tree XMSS. */ diff --git a/tests/api/test_mldsa.c b/tests/api/test_mldsa.c index 5a91f220c3..4e5636f58e 100644 --- a/tests/api/test_mldsa.c +++ b/tests/api/test_mldsa.c @@ -29578,12 +29578,14 @@ int test_mldsa_verify_mu_kats(void) ExpectIntEQ(wc_MlDsaKey_VerifyMu(key, sig_44_mu, (word32)sizeof(sig_44_mu), mu_44, (word32)sizeof(mu_44), &res), 0); ExpectIntEQ(res, 1); - /* Tamper mu: verification must fail. */ - XMEMCPY(muBuf, mu_44, sizeof(mu_44)); - muBuf[0] ^= 0x01; - res = 1; - (void)wc_MlDsaKey_VerifyMu(key, sig_44_mu, (word32)sizeof(sig_44_mu), muBuf, (word32)sizeof(mu_44), &res); - ExpectIntEQ(res, 0); + /* Tamper mu: verification must fail. (guard: key freed/uninit on failure) */ + if (EXPECT_SUCCESS()) { + XMEMCPY(muBuf, mu_44, sizeof(mu_44)); + muBuf[0] ^= 0x01; + res = 1; + (void)wc_MlDsaKey_VerifyMu(key, sig_44_mu, (word32)sizeof(sig_44_mu), muBuf, (word32)sizeof(mu_44), &res); + ExpectIntEQ(res, 0); + } /* Tamper signature: verification must fail. */ ExpectNotNull(sigBuf = (byte*)XMALLOC(sizeof(sig_44_mu), @@ -29609,11 +29611,13 @@ int test_mldsa_verify_mu_kats(void) ExpectIntEQ(res, 1); /* Tamper mu: verification must fail. */ - XMEMCPY(muBuf, mu_65, sizeof(mu_65)); - muBuf[0] ^= 0x01; - res = 1; - (void)wc_MlDsaKey_VerifyMu(key, sig_65_mu, (word32)sizeof(sig_65_mu), muBuf, (word32)sizeof(mu_65), &res); - ExpectIntEQ(res, 0); + if (EXPECT_SUCCESS()) { + XMEMCPY(muBuf, mu_65, sizeof(mu_65)); + muBuf[0] ^= 0x01; + res = 1; + (void)wc_MlDsaKey_VerifyMu(key, sig_65_mu, (word32)sizeof(sig_65_mu), muBuf, (word32)sizeof(mu_65), &res); + ExpectIntEQ(res, 0); + } /* Tamper signature: verification must fail. */ ExpectNotNull(sigBuf = (byte*)XMALLOC(sizeof(sig_65_mu), @@ -29639,11 +29643,13 @@ int test_mldsa_verify_mu_kats(void) ExpectIntEQ(res, 1); /* Tamper mu: verification must fail. */ - XMEMCPY(muBuf, mu_87, sizeof(mu_87)); - muBuf[0] ^= 0x01; - res = 1; - (void)wc_MlDsaKey_VerifyMu(key, sig_87_mu, (word32)sizeof(sig_87_mu), muBuf, (word32)sizeof(mu_87), &res); - ExpectIntEQ(res, 0); + if (EXPECT_SUCCESS()) { + XMEMCPY(muBuf, mu_87, sizeof(mu_87)); + muBuf[0] ^= 0x01; + res = 1; + (void)wc_MlDsaKey_VerifyMu(key, sig_87_mu, (word32)sizeof(sig_87_mu), muBuf, (word32)sizeof(mu_87), &res); + ExpectIntEQ(res, 0); + } /* Tamper signature: verification must fail. */ ExpectNotNull(sigBuf = (byte*)XMALLOC(sizeof(sig_87_mu), diff --git a/tests/api/test_ossl_p7p12.c b/tests/api/test_ossl_p7p12.c index 03590a3ad7..485d973a13 100644 --- a/tests/api/test_ossl_p7p12.c +++ b/tests/api/test_ossl_p7p12.c @@ -142,7 +142,8 @@ 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()) /* transfer to sk only if push succeeded */ + 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..f21c7bc50a 100644 --- a/tests/api/test_ossl_x509_ext.c +++ b/tests/api/test_ossl_x509_ext.c @@ -627,7 +627,8 @@ 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()) /* only transfer ownership if the push succeeded */ + 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..eada071b32 100644 --- a/tests/api/test_ossl_x509_str.c +++ b/tests/api/test_ossl_x509_str.c @@ -2048,8 +2048,11 @@ int test_wolfSSL_CTX_set_cert_store(void) X509_free(svrCert); svrCert = NULL; SSL_CTX_free(ctx); + /* set_cert_store transfers store ownership only when ctx != NULL; free it + * here when CTX creation failed under an allocation failure. */ + if (ctx == NULL) + X509_STORE_free(store); ctx = NULL; - /* store is freed by SSL_CTX_free */ store = NULL; X509_free(rootCa); @@ -2085,7 +2088,9 @@ int test_wolfSSL_CTX_set_cert_store(void) svrIntCert, SSL_FILETYPE_PEM), WOLFSSL_SUCCESS); SSL_CTX_free(ctx); - /* store freed by SSL_CTX_free */ + /* store transferred to ctx only when ctx != NULL; free here otherwise */ + if (ctx == NULL) + X509_STORE_free(store); X509_free(rootCa); X509_free(intCa); X509_free(int2Ca); diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index 84ab3c3bad..1b63eb4097 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -4215,7 +4215,10 @@ 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); + /* guard: inner_cms_der NULL on alloc failure */ + if (inner_cms_der != NULL) { + 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..a37c29463a 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -5556,8 +5556,9 @@ int test_tls13_short_session_ticket(void) /* Now directly test SetTicket with a short ticket by poking the * 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. */ - { + * With the fix, sessIdLen is capped to length so no underflow. + * (guard: ssl_c NULL on setup alloc failure) */ + if (ssl_c != NULL) { byte shortTicket[5] = { 0xBB, 0xCC, 0xDD, 0xEE, 0xFF }; word32 length = sizeof(shortTicket); word32 sessIdLen = ID_LEN; diff --git a/wolfcrypt/src/evp_pk.c b/wolfcrypt/src/evp_pk.c index f719bb5264..d9aac635ae 100644 --- a/wolfcrypt/src/evp_pk.c +++ b/wolfcrypt/src/evp_pk.c @@ -1083,6 +1083,7 @@ static WOLFSSL_EVP_PKEY* d2i_evp_pkey_try(WOLFSSL_EVP_PKEY** out, { WOLFSSL_EVP_PKEY* pkey = NULL; int found = 0; + int ownPkey; WOLFSSL_ENTER("d2i_evp_pkey_try"); @@ -1094,6 +1095,9 @@ static WOLFSSL_EVP_PKEY* d2i_evp_pkey_try(WOLFSSL_EVP_PKEY** out, if ((out != NULL) && (*out != NULL)) { pkey = *out; } + /* If pkey is NULL here, any object the d2iTry* helpers allocate is owned by + * this function and must be freed on the no-key-found path below. */ + ownPkey = (pkey == NULL); #if !defined(NO_RSA) if (d2iTryRsaKey(&pkey, *in, inSz, priv) >= 0) { @@ -1162,6 +1166,9 @@ static WOLFSSL_EVP_PKEY* d2i_evp_pkey_try(WOLFSSL_EVP_PKEY** out, } if (!found) { + /* free only a key we allocated; leave a caller-supplied *out intact */ + if (ownPkey) + wolfSSL_EVP_PKEY_free(pkey); return NULL; } @@ -1316,6 +1323,7 @@ WOLFSSL_EVP_PKEY* wolfSSL_d2i_PrivateKey_bio(WOLFSSL_BIO* bio, wolfSSL_BIO_write(bio, mem + key->pkey_sz, memSz - key->pkey_sz); if (wolfSSL_BIO_get_len(bio) <= 0) { WOLFSSL_MSG("Failed to write memory to bio"); + wolfSSL_EVP_PKEY_free(key); XFREE(mem, bio->heap, DYNAMIC_TYPE_TMP_BUFFER); return NULL; }