From 60b5dfc2ef28731a2c53330c8c94cc1190422c72 Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Tue, 7 Jul 2026 11:14:01 -0700 Subject: [PATCH 1/4] Add test case to swap certificates in SNI callback --- tests/api/test_tls13.c | 60 ++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls13.h | 2 ++ 2 files changed, 62 insertions(+) diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index a691c90fc8..0f92e3cbdf 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -3627,6 +3627,66 @@ int test_tls13_pha(void) } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + defined(HAVE_SNI) && !defined(WOLFSSL_COPY_CERT) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_RSA) && !defined(NO_CERTS) +/* React to the client SNI by reloading the certificate on the existing CTX. + * Because the SSL was created without WOLFSSL_COPY_CERT, the cert pointer is + * identical between the CTX and SSL objects. This test validates the + * cert file replacement. + * Note the updated cert will not be used for this connection. */ +static int test_cert_alias_sni_cb(WOLFSSL* ssl, int* ad, void* arg) +{ + WOLFSSL_CTX* ctx = wolfSSL_get_SSL_CTX(ssl); + (void)ad; + (void)arg; + (void)wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, + WOLFSSL_FILETYPE_PEM); + /* 0 means ack the servername and continue the handshake. */ + return 0; +} +#endif + +int test_tls13_cert_alias_uaf_sni(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + defined(HAVE_SNI) && !defined(WOLFSSL_COPY_CERT) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_RSA) && !defined(NO_CERTS) + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + const char* host = "example.com"; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + /* Server cert/key are loaded on ctx_s, so ssl_s->buffers.certificate + * aliases ctx_s->certificate. */ + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0); + + /* Server swaps its CTX certificate when the SNI arrives. */ + wolfSSL_CTX_set_servername_callback(ctx_s, test_cert_alias_sni_cb); + + /* Client offers SNI so the server callback fires while parsing the + * ClientHello. */ + ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, host, + (word16)XSTRLEN(host)), WOLFSSL_SUCCESS); + + /* The callback loads a new certificate on the server CTX, which is + * aliased by the SSL. The handshake must complete without a UAF. */ + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + + #if defined(HAVE_IO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ defined(WOLFSSL_HAVE_MLKEM) && !defined(WOLFSSL_MLKEM_NO_ENCAPSULATE) && \ !defined(WOLFSSL_MLKEM_NO_DECAPSULATE) && \ diff --git a/tests/api/test_tls13.h b/tests/api/test_tls13.h index 4ea8110825..876f03efa0 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -30,6 +30,7 @@ int test_tls13_bad_psk_binder(void); int test_tls13_rpk_handshake(void); int test_tls13_rpk_handshake_no_negotiation(void); int test_tls13_pha(void); +int test_tls13_cert_alias_uaf_sni(void); int test_tls13_pq_groups(void); int test_tls13_multi_pqc_key_share(void); int test_tls13_early_data(void); @@ -108,6 +109,7 @@ int test_tls13_pqc_hybrid_async_server(void); TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake), \ TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake_no_negotiation), \ TEST_DECL_GROUP("tls13", test_tls13_pha), \ + TEST_DECL_GROUP("tls13", test_tls13_cert_alias_uaf_sni), \ TEST_DECL_GROUP("tls13", test_tls13_pq_groups), \ TEST_DECL_GROUP("tls13", test_tls13_multi_pqc_key_share), \ TEST_DECL_GROUP("tls13", test_tls13_early_data), \ From b493831b6c609c6895c3c6ca1a62c32ce3489f9c Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Wed, 8 Jul 2026 10:08:24 -0700 Subject: [PATCH 2/4] Use refcount on DER buffer to prevent UAF --- src/internal.c | 10 +++++++++ src/ssl.c | 21 +++++++++---------- src/ssl_load.c | 12 ++++------- tests/api.c | 17 +++++----------- wolfcrypt/src/asn.c | 37 +++++++++++++++++++++++++++------- wolfssl/wolfcrypt/asn.h | 1 + wolfssl/wolfcrypt/asn_public.h | 5 +++-- 7 files changed, 63 insertions(+), 40 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8aa3fe065e..12ede5bebb 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7174,6 +7174,10 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx) /* ctx still owns certificate, certChain, key, dh, and cm */ ssl->buffers.certificate = ctx->certificate; ssl->buffers.certChain = ctx->certChain; + /* Increment refcount on cert pointers since we've aliased them. This + * prevents early freeing of the cert buffers. */ + RefDer(ssl->buffers.certificate); + RefDer(ssl->buffers.certChain); #endif ssl->buffers.certChainCnt = ctx->certChainCnt; #ifndef WOLFSSL_BLIND_PRIVATE_KEY @@ -9218,6 +9222,12 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl) #ifndef NO_CERTS ssl->keepCert = 0; /* make sure certificate is free'd */ wolfSSL_UnloadCertsKeys(ssl); + /* Release references on cert buffers aliased from the context. Owned + * buffers were already freed and cleared by wolfSSL_UnloadCertsKeys. */ + FreeDer(&ssl->buffers.certificate); + ssl->buffers.weOwnCert = 0; + FreeDer(&ssl->buffers.certChain); + ssl->buffers.weOwnCertChain = 0; #endif #ifndef NO_RSA FreeKey(ssl, DYNAMIC_TYPE_RSA, (void**)&ssl->peerRsaKey); diff --git a/src/ssl.c b/src/ssl.c index 52abaa02c9..c493bcf5d2 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12727,17 +12727,10 @@ void wolfSSL_certs_clear(WOLFSSL* ssl) if (ssl == NULL) return; - /* ctx still owns certificate, certChain, key, dh, and cm */ - if (ssl->buffers.weOwnCert) { - FreeDer(&ssl->buffers.certificate); - ssl->buffers.weOwnCert = 0; - } - ssl->buffers.certificate = NULL; - if (ssl->buffers.weOwnCertChain) { - FreeDer(&ssl->buffers.certChain); - ssl->buffers.weOwnCertChain = 0; - } - ssl->buffers.certChain = NULL; + FreeDer(&ssl->buffers.certificate); + ssl->buffers.weOwnCert = 0; + FreeDer(&ssl->buffers.certChain); + ssl->buffers.weOwnCertChain = 0; #ifdef WOLFSSL_TLS13 ssl->buffers.certChainCnt = 0; #endif @@ -13200,8 +13193,14 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx) } #else /* ctx owns certificate, certChain and key */ + FreeDer(&ssl->buffers.certificate); + ssl->buffers.weOwnCert = 0; + FreeDer(&ssl->buffers.certChain); + ssl->buffers.weOwnCertChain = 0; ssl->buffers.certificate = ctx->certificate; ssl->buffers.certChain = ctx->certChain; + RefDer(ssl->buffers.certificate); + RefDer(ssl->buffers.certChain); #endif #ifdef WOLFSSL_TLS13 ssl->buffers.certChainCnt = ctx->certChainCnt; diff --git a/src/ssl_load.c b/src/ssl_load.c index b63a3b8430..5161f69725 100644 --- a/src/ssl_load.c +++ b/src/ssl_load.c @@ -242,10 +242,7 @@ static int ProcessUserChainRetain(WOLFSSL_CTX* ctx, WOLFSSL* ssl, /* Store in SSL object if available. */ if (ssl != NULL) { - /* Dispose of old chain if not reference to context's. */ - if (ssl->buffers.weOwnCertChain) { - FreeDer(&ssl->buffers.certChain); - } + FreeDer(&ssl->buffers.certChain); /* Allocate and copy the buffer into SSL object. */ ret = AllocCopyDer(&ssl->buffers.certChain, chainBuffer, len, type, heap); @@ -2224,15 +2221,14 @@ static int ProcessBufferCertHandleDer(WOLFSSL_CTX* ctx, WOLFSSL* ssl, /* Leaf certificate - our certificate. */ else if (type == CERT_TYPE) { if (ssl != NULL) { - /* Free previous certificate if we own it. */ + #ifdef KEEP_OUR_CERT if (ssl->buffers.weOwnCert) { - FreeDer(&ssl->buffers.certificate); - #ifdef KEEP_OUR_CERT /* Dispose of X509 version of certificate. */ wolfSSL_X509_free(ssl->ourCert); ssl->ourCert = NULL; - #endif } + #endif + FreeDer(&ssl->buffers.certificate); /* Store certificate as ours. */ ssl->buffers.certificate = der; #ifdef KEEP_OUR_CERT diff --git a/tests/api.c b/tests/api.c index eed600c040..874168db72 100644 --- a/tests/api.c +++ b/tests/api.c @@ -3956,25 +3956,18 @@ static int test_wolfSSL_add_to_chain_overflow(void) 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); + * so the size calculation (len + CERT_HEADER_SZ + certSz) overflows. + * Use AllocDer so the buffer refcount is initialized. */ + ExpectIntEQ(AllocDer(&fakeChain, 16, CERT_TYPE, ctx->heap), 0); } if (EXPECT_SUCCESS()) { - XMEMSET(fakeChain, 0, sizeof(DerBuffer) + 16); - fakeChain->buffer = (byte*)(fakeChain + 1); fakeChain->length = WOLFSSL_MAX_32BIT - 2; /* will overflow with any cert */ - fakeChain->type = CERT_TYPE; - fakeChain->dynType = DYNAMIC_TYPE_CERT; /* Replace the real chain with our fake one. */ - if (ctx->certChain != NULL) { - XFREE(ctx->certChain, ctx->heap, DYNAMIC_TYPE_CERT); - } + FreeDer(&ctx->certChain); ctx->certChain = fakeChain; } else { - XFREE(fakeChain, ctx ? ctx->heap : NULL, DYNAMIC_TYPE_CERT); + FreeDer(&fakeChain); } /* Try to add another cert - this MUST fail due to overflow guard. */ diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 02e018f5d4..b17bf3fa56 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -24693,6 +24693,14 @@ int AllocDer(DerBuffer** pDer, word32 length, int type, void* heap) der->heap = heap; der->buffer = (byte*)der + sizeof(DerBuffer); der->length = length; + wolfSSL_RefInit(&der->ref, &ret); + #ifdef WOLFSSL_REFCNT_ERROR_RETURN + if (ret != 0) { + XFREE(der, heap, dynType); + *pDer = NULL; + return ret; + } + #endif ret = 0; /* Success */ } else { ret = BAD_FUNC_ARG; @@ -24700,6 +24708,15 @@ int AllocDer(DerBuffer** pDer, word32 length, int type, void* heap) return ret; } +void RefDer(DerBuffer* der) +{ + if (der != NULL) { + int ref_err = 0; + wolfSSL_RefInc(&der->ref, &ref_err); + (void)ref_err; + } +} + int AllocCopyDer(DerBuffer** pDer, const unsigned char* buff, word32 length, int type, void* heap) { @@ -24715,15 +24732,21 @@ void FreeDer(DerBuffer** pDer) { if (pDer && *pDer) { DerBuffer* der = (DerBuffer*)*pDer; + int isZero = 1; + int ref_err = 0; - /* ForceZero private keys */ - if (((der->type == PRIVATEKEY_TYPE) || - (der->type == ALT_PRIVATEKEY_TYPE)) && der->buffer != NULL) { - ForceZero(der->buffer, der->length); + wolfSSL_RefDec(&der->ref, &isZero, &ref_err); + (void)ref_err; + if (isZero) { + if (((der->type == PRIVATEKEY_TYPE) || + (der->type == ALT_PRIVATEKEY_TYPE)) && der->buffer != NULL) { + ForceZero(der->buffer, der->length); + } + der->buffer = NULL; + der->length = 0; + wolfSSL_RefFree(&der->ref); + XFREE(der, der->heap, der->dynType); } - der->buffer = NULL; - der->length = 0; - XFREE(der, der->heap, der->dynType); *pDer = NULL; } diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 3a4dc6ded3..91778564c1 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2811,6 +2811,7 @@ WOLFSSL_API int AllocDer(DerBuffer** der, word32 length, int type, WOLFSSL_LOCAL int AllocCopyDer(DerBuffer** der, const unsigned char* buff, word32 length, int type, void* heap); WOLFSSL_API void FreeDer(DerBuffer** der); +WOLFSSL_LOCAL void RefDer(DerBuffer* der); #ifdef WOLFSSL_ASN_PARSE_KEYUSAGE WOLFSSL_LOCAL int ParseKeyUsageStr(const char* value, word16* keyUsage, diff --git a/wolfssl/wolfcrypt/asn_public.h b/wolfssl/wolfcrypt/asn_public.h index d557ac94cd..62a77afca9 100644 --- a/wolfssl/wolfcrypt/asn_public.h +++ b/wolfssl/wolfcrypt/asn_public.h @@ -241,8 +241,9 @@ typedef struct DerBuffer { byte* buffer; void* heap; word32 length; - int type; /* enum CertType */ - int dynType; /* DYNAMIC_TYPE_* */ + int type; /* enum CertType */ + int dynType; /* DYNAMIC_TYPE_* */ + wolfSSL_Ref ref; /* refcount for aliased pointers to the der */ } DerBuffer; typedef struct WOLFSSL_ASN1_TIME { From a76d50daa29cf7249dff07650d612eff62cb3fd6 Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Wed, 8 Jul 2026 14:38:49 -0700 Subject: [PATCH 3/4] PR feedback regarding return code checking --- src/internal.c | 12 ++++++++---- src/ssl.c | 10 ++++++++-- tests/api/test_tls13.c | 9 +++++++-- wolfcrypt/src/asn.c | 17 +++++++++++++---- wolfssl/wolfcrypt/asn.h | 2 +- 5 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/internal.c b/src/internal.c index 12ede5bebb..f65681f39b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7173,11 +7173,15 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx) #else /* ctx still owns certificate, certChain, key, dh, and cm */ ssl->buffers.certificate = ctx->certificate; + if (!RefDer(ssl->buffers.certificate)) { + ssl->buffers.certificate = NULL; + return BAD_MUTEX_E; + } ssl->buffers.certChain = ctx->certChain; - /* Increment refcount on cert pointers since we've aliased them. This - * prevents early freeing of the cert buffers. */ - RefDer(ssl->buffers.certificate); - RefDer(ssl->buffers.certChain); + if (!RefDer(ssl->buffers.certChain)) { + ssl->buffers.certChain = NULL; + return BAD_MUTEX_E; + } #endif ssl->buffers.certChainCnt = ctx->certChainCnt; #ifndef WOLFSSL_BLIND_PRIVATE_KEY diff --git a/src/ssl.c b/src/ssl.c index c493bcf5d2..cb23542d7d 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -13198,9 +13198,15 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx) FreeDer(&ssl->buffers.certChain); ssl->buffers.weOwnCertChain = 0; ssl->buffers.certificate = ctx->certificate; + if (!RefDer(ssl->buffers.certificate)) { + ssl->buffers.certificate = NULL; + return NULL; + } ssl->buffers.certChain = ctx->certChain; - RefDer(ssl->buffers.certificate); - RefDer(ssl->buffers.certChain); + if (!RefDer(ssl->buffers.certChain)) { + ssl->buffers.certChain = NULL; + return NULL; + } #endif #ifdef WOLFSSL_TLS13 ssl->buffers.certChainCnt = ctx->certChainCnt; diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index 0f92e3cbdf..fd7290145c 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -3639,9 +3639,10 @@ int test_tls13_pha(void) static int test_cert_alias_sni_cb(WOLFSSL* ssl, int* ad, void* arg) { WOLFSSL_CTX* ctx = wolfSSL_get_SSL_CTX(ssl); + int* cbRet = (int*)arg; (void)ad; - (void)arg; - (void)wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, + /* Feed the reload result back to the parent function below. */ + *cbRet = wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, WOLFSSL_FILETYPE_PEM); /* 0 means ack the servername and continue the handshake. */ return 0; @@ -3659,6 +3660,7 @@ int test_tls13_cert_alias_uaf_sni(void) WOLFSSL *ssl_c = NULL, *ssl_s = NULL; struct test_memio_ctx test_ctx; const char* host = "example.com"; + int cbRet = WOLFSSL_FATAL_ERROR; XMEMSET(&test_ctx, 0, sizeof(test_ctx)); /* Server cert/key are loaded on ctx_s, so ssl_s->buffers.certificate @@ -3668,6 +3670,7 @@ int test_tls13_cert_alias_uaf_sni(void) /* Server swaps its CTX certificate when the SNI arrives. */ wolfSSL_CTX_set_servername_callback(ctx_s, test_cert_alias_sni_cb); + ExpectIntEQ(wolfSSL_CTX_set_servername_arg(ctx_s, &cbRet), WOLFSSL_SUCCESS); /* Client offers SNI so the server callback fires while parsing the * ClientHello. */ @@ -3677,6 +3680,8 @@ int test_tls13_cert_alias_uaf_sni(void) /* The callback loads a new certificate on the server CTX, which is * aliased by the SSL. The handshake must complete without a UAF. */ ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + /* Fail the test if the callback's cert reload failed or didn't run. */ + ExpectIntEQ(cbRet, WOLFSSL_SUCCESS); wolfSSL_free(ssl_c); wolfSSL_free(ssl_s); diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index b17bf3fa56..f44fb7c975 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -24708,13 +24708,22 @@ int AllocDer(DerBuffer** pDer, word32 length, int type, void* heap) return ret; } -void RefDer(DerBuffer* der) +/* Increments the reference count on a shared DER buffer. + * + * A mutex-backed refcount can fail to lock, leaving the count un-incremented; + * return failure so callers don't proceed with an un-referenced alias. + * + * @param [in, out] der DER buffer. May be NULL. + * @return 1 on success + * @return 0 on error + */ +int RefDer(DerBuffer* der) { + int err = 0; if (der != NULL) { - int ref_err = 0; - wolfSSL_RefInc(&der->ref, &ref_err); - (void)ref_err; + wolfSSL_RefInc(&der->ref, &err); } + return !err; } int AllocCopyDer(DerBuffer** pDer, const unsigned char* buff, word32 length, diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 91778564c1..64aceaac5a 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2811,7 +2811,7 @@ WOLFSSL_API int AllocDer(DerBuffer** der, word32 length, int type, WOLFSSL_LOCAL int AllocCopyDer(DerBuffer** der, const unsigned char* buff, word32 length, int type, void* heap); WOLFSSL_API void FreeDer(DerBuffer** der); -WOLFSSL_LOCAL void RefDer(DerBuffer* der); +WOLFSSL_LOCAL int RefDer(DerBuffer* der); #ifdef WOLFSSL_ASN_PARSE_KEYUSAGE WOLFSSL_LOCAL int ParseKeyUsageStr(const char* value, word16* keyUsage, From 0b2304fedb712b3f393f1a85a2e0a6c82ecf8dbc Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Wed, 8 Jul 2026 22:17:15 -0700 Subject: [PATCH 4/4] Fix coding-no test --- tests/api/test_tls13.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index fd7290145c..393c9db425 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -3642,8 +3642,7 @@ static int test_cert_alias_sni_cb(WOLFSSL* ssl, int* ad, void* arg) int* cbRet = (int*)arg; (void)ad; /* Feed the reload result back to the parent function below. */ - *cbRet = wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, - WOLFSSL_FILETYPE_PEM); + *cbRet = wolfSSL_CTX_use_certificate_file(ctx, svrCertFile, CERT_FILETYPE); /* 0 means ack the servername and continue the handshake. */ return 0; }