From d59405785ec75baaab549186c925dc2fae564e24 Mon Sep 17 00:00:00 2001 From: Tobias Frauenschlaeger Date: Fri, 21 Aug 2026 22:36:54 +0000 Subject: [PATCH] Store the trusted CA identifier with its object TLSX_TCA_New() allocated the object, then the identifier, and freed the object again on every path where the identifier was missing or the wrong size. Check the type and size first and put the identifier in a flexible array at the end of the object, so the failure paths are early returns and there is one buffer to free. TLSX_TCA_New() never makes a SHA-1 or X509 name entry without an identifier, so TLSX_TCA_Write() drops its fallbacks for a missing one. --- src/tls.c | 110 ++++++++++++++--------------------- tests/api/test_tls_msgtype.c | 18 ++---- wolfssl/internal.h | 7 ++- 3 files changed, 56 insertions(+), 79 deletions(-) diff --git a/src/tls.c b/src/tls.c index 3439f425eb..e45c8e3137 100644 --- a/src/tls.c +++ b/src/tls.c @@ -2943,53 +2943,48 @@ int TLSX_SNI_GetFromBuffer(const byte* clientHello, word32 helloSz, /** Creates a new TCA object. */ static TCA* TLSX_TCA_New(byte type, const byte* id, word16 idSz, void* heap) { - TCA* tca = (TCA*)XMALLOC(sizeof(TCA), heap, DYNAMIC_TYPE_TLSX); - - if (tca) { - XMEMSET(tca, 0, sizeof(TCA)); - tca->type = type; - - switch (type) { - case WOLFSSL_TRUSTED_CA_PRE_AGREED: - break; - - #ifndef NO_SHA - case WOLFSSL_TRUSTED_CA_KEY_SHA1: - case WOLFSSL_TRUSTED_CA_CERT_SHA1: - if (idSz == WC_SHA_DIGEST_SIZE && - (tca->id = - (byte*)XMALLOC(idSz, heap, DYNAMIC_TYPE_TLSX))) { - XMEMCPY(tca->id, id, idSz); - tca->idSz = idSz; - } - else { - XFREE(tca, heap, DYNAMIC_TYPE_TLSX); - tca = NULL; - } - break; - #endif - - case WOLFSSL_TRUSTED_CA_X509_NAME: - if (idSz > 0 && - (tca->id = - (byte*)XMALLOC(idSz, heap, DYNAMIC_TYPE_TLSX))) { - XMEMCPY(tca->id, id, idSz); - tca->idSz = idSz; - } - else { - XFREE(tca, heap, DYNAMIC_TYPE_TLSX); - tca = NULL; - } - break; - - default: /* invalid type */ - XFREE(tca, heap, DYNAMIC_TYPE_TLSX); - tca = NULL; - } - } + TCA* tca; + word32 allocSz; (void)heap; + switch (type) { + case WOLFSSL_TRUSTED_CA_PRE_AGREED: + idSz = 0; + break; + + #ifndef NO_SHA + case WOLFSSL_TRUSTED_CA_KEY_SHA1: + case WOLFSSL_TRUSTED_CA_CERT_SHA1: + if (idSz != WC_SHA_DIGEST_SIZE) + return NULL; + break; + #endif + + case WOLFSSL_TRUSTED_CA_X509_NAME: + if (idSz == 0) + return NULL; + break; + + default: /* invalid type */ + return NULL; + } + + allocSz = (word32)sizeof(TCA) + idSz; + /* Fails where size_t is too narrow to hold the sum. */ + if ((word32)(size_t)allocSz != allocSz) + return NULL; + tca = (TCA*)XMALLOC((size_t)allocSz, heap, DYNAMIC_TYPE_TLSX); + if (tca == NULL) + return NULL; + + XMEMSET(tca, 0, sizeof(TCA)); + tca->type = type; + if (idSz > 0) { + XMEMCPY(tca->id, id, idSz); + tca->idSz = idSz; + } + return tca; } @@ -2999,7 +2994,6 @@ static void TLSX_TCA_Free(TCA* tca, void* heap) (void)heap; if (tca) { - XFREE(tca->id, heap, DYNAMIC_TYPE_TLSX); XFREE(tca, heap, DYNAMIC_TYPE_TLSX); } } @@ -3063,29 +3057,15 @@ static word16 TLSX_TCA_Write(TCA* list, byte* output) #ifndef NO_SHA case WOLFSSL_TRUSTED_CA_KEY_SHA1: case WOLFSSL_TRUSTED_CA_CERT_SHA1: - if (tca->id != NULL) { - XMEMCPY(output + offset, tca->id, tca->idSz); - offset += tca->idSz; - } - else { - /* ID missing. Set to an empty string. */ - c16toa(0, output + offset); - offset += OPAQUE16_LEN; - } + XMEMCPY(output + offset, tca->id, tca->idSz); + offset += tca->idSz; break; #endif case WOLFSSL_TRUSTED_CA_X509_NAME: - if (tca->id != NULL) { - c16toa(tca->idSz, output + offset); /* tca length */ - offset += OPAQUE16_LEN; - XMEMCPY(output + offset, tca->id, tca->idSz); - offset += tca->idSz; - } - else { - /* ID missing. Set to an empty string. */ - c16toa(0, output + offset); - offset += OPAQUE16_LEN; - } + c16toa(tca->idSz, output + offset); /* tca length */ + offset += OPAQUE16_LEN; + XMEMCPY(output + offset, tca->id, tca->idSz); + offset += tca->idSz; break; default: /* ID unknown. Set to an empty string. */ diff --git a/tests/api/test_tls_msgtype.c b/tests/api/test_tls_msgtype.c index 6b60671590..484b693006 100644 --- a/tests/api/test_tls_msgtype.c +++ b/tests/api/test_tls_msgtype.c @@ -2492,13 +2492,7 @@ static void* tca_fail_realloc(void* ptr, size_t size) } #endif /* HAVE_TRUSTED_CA && !NO_WOLFSSL_CLIENT && !NO_TLS */ -/* ---- TLSX_TCA_New(): id allocation failure ------------------------------ */ -/* KEY_SHA1/CERT_SHA1: if (idSz == WC_SHA_DIGEST_SIZE && - * (tca->id = XMALLOC(idSz, ...))) {...} - * X509_NAME: if (idSz > 0 && - * (tca->id = XMALLOC(idSz, ...))) {...} - * In both cases the length operand's pair is already covered elsewhere; - * only the allocation succeeding vs. failing is exercised here. */ +/* ---- TLSX_TCA_New(): allocation failure ---------------------------------- */ int test_tls_msgtype_tca_new_alloc(void) { EXPECT_DECLS; @@ -2518,15 +2512,14 @@ int test_tls_msgtype_tca_new_alloc(void) }; const byte nameId[] = { 5, 6, 7, 8 }; - /* CERT_SHA1: the TCA struct itself (allocation #0) succeeds, the id - * buffer (allocation #1) fails. */ + /* CERT_SHA1: the TCA and its identifier share one allocation. */ ExpectNotNull(ctx = wolfSSL_CTX_new(wolfTLSv1_2_client_method())); ExpectNotNull(ssl = wolfSSL_new(ctx)); ExpectIntEQ(wolfSSL_GetAllocators(&prevM, &prevF, &prevR), 0); ExpectIntEQ(wolfSSL_SetAllocators(tca_fail_malloc, tca_fail_free, tca_fail_realloc), 0); tca_alloc_seen = 0; - tca_fail_after = 1; + tca_fail_after = 0; ExpectIntEQ(wolfSSL_UseTrustedCA(ssl, WOLFSSL_TRUSTED_CA_CERT_SHA1, sha1Id, sizeof(sha1Id)), WC_NO_ERR_TRACE(MEMORY_E)); tca_fail_after = -1; @@ -2536,15 +2529,14 @@ int test_tls_msgtype_tca_new_alloc(void) wolfSSL_CTX_free(ctx); ctx = NULL; - /* X509_NAME: same shape - struct allocation succeeds, id buffer - * allocation fails. */ + /* X509_NAME: the same single allocation. */ ExpectNotNull(ctx = wolfSSL_CTX_new(wolfTLSv1_2_client_method())); ExpectNotNull(ssl = wolfSSL_new(ctx)); ExpectIntEQ(wolfSSL_GetAllocators(&prevM, &prevF, &prevR), 0); ExpectIntEQ(wolfSSL_SetAllocators(tca_fail_malloc, tca_fail_free, tca_fail_realloc), 0); tca_alloc_seen = 0; - tca_fail_after = 1; + tca_fail_after = 0; ExpectIntEQ(wolfSSL_UseTrustedCA(ssl, WOLFSSL_TRUSTED_CA_X509_NAME, nameId, (word32)sizeof(nameId)), WC_NO_ERR_TRACE(MEMORY_E)); tca_fail_after = -1; diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 0c89f15f8e..38507be199 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -3718,9 +3718,14 @@ WOLFSSL_LOCAL int TLSX_SNI_GetFromBuffer(const byte* clientHello, typedef struct TCA { byte type; /* TCA Type */ - byte* id; /* TCA identifier */ word16 idSz; /* TCA identifier size */ struct TCA* next; /* List Behavior */ + /* Ignore "nonstandard extension used : zero-sized array in struct/union" + * MSVC warning */ + #ifdef _MSC_VER + #pragma warning(disable: 4200) + #endif + byte id[WC_FLEXIBLE_ARRAY_SIZE]; /* TCA identifier */ } TCA; WOLFSSL_LOCAL int TLSX_UseTrustedCA(TLSX** extensions, byte type,