Skip to content

Add refcounts on key, keyMask, altKey, altKeyMask to prevent UAF #10865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
padelsbach wants to merge 5 commits into from
Closed

Add refcounts on key, keyMask, altKey, altKeyMask to prevent UAF #10865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
60b5dfc
Add test case to swap certificates in SNI callback
padelsbach Jul 7, 2026
b493831
Use refcount on DER buffer to prevent UAF
padelsbach Jul 8, 2026
a76d50d
PR feedback regarding return code checking
padelsbach Jul 8, 2026
0b2304f
Fix coding-no test
padelsbach Jul 9, 2026
160f72f
Add refcounts on key, keyMask, altKey, altKeyMask to prevent UAF
padelsbach Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7173,7 +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;
if (!RefDer(ssl->buffers.certChain)) {
ssl->buffers.certChain = NULL;
return BAD_MUTEX_E;
}
#endif
ssl->buffers.certChainCnt = ctx->certChainCnt;
#ifndef WOLFSSL_BLIND_PRIVATE_KEY
Expand All @@ -7195,6 +7203,12 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
}
#else
ssl->buffers.key = ctx->privateKey;
/* Hold a reference on the shared key so reloading it on the CTX cannot
* free it while this SSL still aliases it. */
if (!RefDer(ssl->buffers.key)) {
ssl->buffers.key = NULL;
return BAD_MUTEX_E;
}
#endif
#else
if (ctx->privateKey != NULL) {
Expand Down Expand Up @@ -7225,6 +7239,11 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
#ifdef WOLFSSL_DUAL_ALG_CERTS
#ifndef WOLFSSL_BLIND_PRIVATE_KEY
ssl->buffers.altKey = ctx->altPrivateKey;
/* Hold a reference on the shared alternative key. */
if (!RefDer(ssl->buffers.altKey)) {
ssl->buffers.altKey = NULL;
return BAD_MUTEX_E;
}
#else
if (ctx->altPrivateKey != NULL) {
ret = AllocCopyDer(&ssl->buffers.altKey, ctx->altPrivateKey->buffer,
Expand Down Expand Up @@ -9218,6 +9237,25 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl)
#ifndef NO_CERTS
ssl->keepCert = 0; /* make sure certificate is free'd */
wolfSSL_UnloadCertsKeys(ssl);
/* Release references on cert and key 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;
FreeDer(&ssl->buffers.key);
ssl->buffers.weOwnKey = 0;
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif
#ifdef WOLFSSL_DUAL_ALG_CERTS
FreeDer(&ssl->buffers.altKey);
ssl->buffers.weOwnAltKey = 0;
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.altKeyMask);
#endif
#endif /* WOLFSSL_DUAL_ALG_CERTS */
#endif
#ifndef NO_RSA
FreeKey(ssl, DYNAMIC_TYPE_RSA, (void**)&ssl->peerRsaKey);
Expand Down
67 changes: 38 additions & 29 deletions src/ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12727,47 +12727,30 @@ 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
if (ssl->buffers.weOwnKey) {
FreeDer(&ssl->buffers.key);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif
ssl->buffers.weOwnKey = 0;
}
ssl->buffers.key = NULL;
/* Release our reference to the key (owned copy or CTX alias). */
FreeDer(&ssl->buffers.key);
ssl->buffers.weOwnKey = 0;
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
ssl->buffers.keyMask = NULL;
FreeDer(&ssl->buffers.keyMask);
#endif
ssl->buffers.keyType = 0;
ssl->buffers.keyId = 0;
ssl->buffers.keyLabel = 0;
ssl->buffers.keySz = 0;
ssl->buffers.keyDevId = 0;
#ifdef WOLFSSL_DUAL_ALG_CERTS
if (ssl->buffers.weOwnAltKey) {
FreeDer(&ssl->buffers.altKey);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.altKeyMask);
#endif
ssl->buffers.weOwnAltKey = 0;
}
ssl->buffers.altKey = NULL;
/* Release our reference to the alternative key (owned copy or CTX alias). */
FreeDer(&ssl->buffers.altKey);
ssl->buffers.weOwnAltKey = 0;
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
ssl->buffers.altKeyMask = NULL;
FreeDer(&ssl->buffers.altKeyMask);
#endif
#endif /* WOLFSSL_DUAL_ALG_CERTS */
}
Expand Down Expand Up @@ -13200,8 +13183,20 @@ 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;
if (!RefDer(ssl->buffers.certificate)) {
ssl->buffers.certificate = NULL;
return NULL;
}
ssl->buffers.certChain = ctx->certChain;
if (!RefDer(ssl->buffers.certChain)) {
ssl->buffers.certChain = NULL;
return NULL;
}
#endif
#ifdef WOLFSSL_TLS13
ssl->buffers.certChainCnt = ctx->certChainCnt;
Expand All @@ -13225,7 +13220,14 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
ssl->buffers.key = ctx->privateKey;
}
#else
/* Release our previous key reference before aliasing the new CTX's. */
FreeDer(&ssl->buffers.key);
ssl->buffers.weOwnKey = 0;
ssl->buffers.key = ctx->privateKey;
if (!RefDer(ssl->buffers.key)) {
ssl->buffers.key = NULL;
return NULL;
}
#endif
#else
if (ctx->privateKey != NULL) {
Expand Down Expand Up @@ -13262,7 +13264,14 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
ssl->options.haveMlDsaSig = ctx->haveMlDsaSig;
#ifdef WOLFSSL_DUAL_ALG_CERTS
#ifndef WOLFSSL_BLIND_PRIVATE_KEY
/* Release our previous alt key reference before aliasing the new CTX's. */
FreeDer(&ssl->buffers.altKey);
ssl->buffers.weOwnAltKey = 0;
ssl->buffers.altKey = ctx->altPrivateKey;
if (!RefDer(ssl->buffers.altKey)) {
ssl->buffers.altKey = NULL;
return NULL;
}
#else
if (ctx->altPrivateKey != NULL) {
ret = AllocCopyDer(&ssl->buffers.altKey, ctx->altPrivateKey->buffer,
Expand Down
47 changes: 20 additions & 27 deletions src/ssl_load.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -1320,13 +1317,12 @@ static int ProcessBufferPrivKeyHandleDer(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
else
#endif /* WOLFSSL_DUAL_ALG_CERTS */
if (ssl != NULL) {
/* Dispose of previous key if not context's. */
if (ssl->buffers.weOwnKey) {
FreeDer(&ssl->buffers.key);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif
}
/* Release our reference to the previous key (owned copy or CTX
* alias) before storing the new one. */
FreeDer(&ssl->buffers.key);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif
ssl->buffers.keyId = 0;
ssl->buffers.keyLabel = 0;
ssl->buffers.keyDevId = INVALID_DEVID;
Expand Down Expand Up @@ -2224,15 +2220,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
Expand Down Expand Up @@ -4625,12 +4620,11 @@ int wolfSSL_use_PrivateKey_Id(WOLFSSL* ssl, const unsigned char* id,
}

/* Dispose of old private key if owned and allocate and copy in id. */
if (ssl->buffers.weOwnKey) {
FreeDer(&ssl->buffers.key);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif
}
/* Release our reference to the old key (owned copy or CTX alias). */
FreeDer(&ssl->buffers.key);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif
if (AllocCopyDer(&ssl->buffers.key, id, (word32)sz, PRIVATEKEY_TYPE,
ssl->heap) != 0) {
ret = 0;
Expand Down Expand Up @@ -4701,12 +4695,11 @@ int wolfSSL_use_PrivateKey_Label(WOLFSSL* ssl, const char* label, int devId)
sz = (word32)XSTRLEN(label) + 1;

/* Dispose of old private key if owned and allocate and copy in label. */
if (ssl->buffers.weOwnKey) {
FreeDer(&ssl->buffers.key);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif
}
/* Release our reference to the old key (owned copy or CTX alias). */
FreeDer(&ssl->buffers.key);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif
if (AllocCopyDer(&ssl->buffers.key, (const byte*)label, (word32)sz,
PRIVATEKEY_TYPE, ssl->heap) != 0) {
ret = 0;
Expand Down
25 changes: 17 additions & 8 deletions src/tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10142,20 +10142,29 @@ static int SendTls13CertificateVerify(WOLFSSL* ssl)
}
ssl->buffers.keyType = ssl->buffers.altKeyType;
ssl->buffers.keySz = ssl->buffers.altKeySz;
/* If we own it, free key before overriding it. */
if (ssl->buffers.weOwnKey) {
FreeDer(&ssl->buffers.key);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif
}
/* Release our reference to the current key before
* overriding it (owned copy or CTX alias). */
FreeDer(&ssl->buffers.key);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif

/* Swap keys */
/* Swap keys. key and altKey now reference the same buffer,
* so take a reference for the new alias. */
ssl->buffers.key = ssl->buffers.altKey;
ssl->buffers.weOwnKey = ssl->buffers.weOwnAltKey;
if (!RefDer(ssl->buffers.key)) {
ssl->buffers.key = NULL;
ssl->buffers.weOwnKey = 0;
ERROR_OUT(BAD_MUTEX_E, exit_scv);
}

#ifdef WOLFSSL_BLIND_PRIVATE_KEY
ssl->buffers.keyMask = ssl->buffers.altKeyMask;
if (!RefDer(ssl->buffers.keyMask)) {
ssl->buffers.keyMask = NULL;
ERROR_OUT(BAD_MUTEX_E, exit_scv);
}
/* Unblind the alternative key before decoding */
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask);
#endif
Expand Down
17 changes: 5 additions & 12 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
Loading