From fd4c742376974b850d16c8af3d1f69aab19e154d Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 22 Jun 2026 15:40:55 +0100 Subject: [PATCH 1/7] Use a local word32 for RSA verify-recover output length WP11_Rsa_Verify_Recover passed the caller's CK_ULONG_PTR outLen to wc_RsaDirect cast as (word32*). On LP64 platforms CK_ULONG is 8 bytes while wc_RsaDirect writes only a 4-byte word32 through the pointer, so the upper bytes of *outLen retained their caller-supplied value. The inflated length then drove the front-padding scan and XMEMMOVE. Use a local word32 for the wc_RsaDirect output length and assign it back to *outLen on success, matching the pattern used by the other RSA functions in this file. F-1611 --- src/internal.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index cf25b6a1..b0fa20fa 100644 --- a/src/internal.c +++ b/src/internal.c @@ -12711,12 +12711,18 @@ int WP11_Rsa_Verify_Recover(CK_MECHANISM_TYPE mechanism, unsigned char* sig, case CKM_RSA_X_509: { byte* data_out = NULL; byte* pos; - ret = wc_RsaDirect(sig, sigLen, out, (word32*)outLen, + /* wc_RsaDirect writes a word32 through the length pointer. Use a + * local word32 rather than casting the CK_ULONG_PTR so that only + * the low word of *outLen is not partially updated; assign the + * result back on success. */ + word32 tmpLen = (word32)*outLen; + ret = wc_RsaDirect(sig, sigLen, out, &tmpLen, pub->data.rsaKey, RSA_PUBLIC_DECRYPT, NULL); if (ret < 0) { ret = CKR_FUNCTION_FAILED; } else { + *outLen = tmpLen; ret = CKR_OK; /* Result is front padded with 0x00 */ for (pos = out; pos < out + *outLen; pos++) { From cab2dde037cc5ab16e7b260802511c8ac40757c1 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 22 Jun 2026 15:41:47 +0100 Subject: [PATCH 2/7] Validate mechanism and length bounds before key use in C_VerifyRecover C_VerifyRecover read the active session object and called WP11_Rsa_KeyLen(obj) before checking the mechanism or that a verify-recover operation had been initialized. Because the active object pointer is set by any *Init function, a non-RSA object left active (for example an HMAC generic-secret key) was dereferenced as an RSA key. Move the mechanism switch and operation-initialized checks ahead of WP11_Rsa_KeyLen, and add an explicit CKK_RSA key-type check. Also add CK_ULONG_FITS_WORD32 guards on ulSignatureLen and *pulDataLen to reject values that would not fit a word32, and set *pulDataLen to the required size when returning CKR_BUFFER_TOO_SMALL as required by PKCS#11. F-3411, F-5152, F-3838 --- src/crypto.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/crypto.c b/src/crypto.c index 8d780cfa..9fbbf7e6 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -6806,14 +6806,10 @@ CK_RV C_VerifyRecover(CK_SESSION_HANDLE hSession, return CKR_OPERATION_NOT_INITIALIZED; } - decDataLen = WP11_Rsa_KeyLen(obj); - if (pData == NULL) { - *pulDataLen = decDataLen; - return CKR_OK; - } - if (decDataLen > (word32)*pulDataLen) - return CKR_BUFFER_TOO_SMALL; - + /* Validate the mechanism and that a verify-recover operation has been + * initialized before treating the active object as an RSA key. The active + * object pointer is also set by other *Init functions, so these checks must + * happen before WP11_Rsa_KeyLen dereferences obj as an RSA key. */ switch (mechanism) { case CKM_RSA_PKCS: if (!WP11_Session_IsOpInitialized(session, @@ -6828,6 +6824,23 @@ CK_RV C_VerifyRecover(CK_SESSION_HANDLE hSession, default: return CKR_MECHANISM_INVALID; } + if (WP11_Object_GetType(obj) != CKK_RSA) + return CKR_KEY_TYPE_INCONSISTENT; + if (!CK_ULONG_FITS_WORD32(ulSignatureLen)) + return CKR_ARGUMENTS_BAD; + + decDataLen = WP11_Rsa_KeyLen(obj); + if (pData == NULL) { + *pulDataLen = decDataLen; + return CKR_OK; + } + if (!CK_ULONG_FITS_WORD32(*pulDataLen)) + return CKR_ARGUMENTS_BAD; + if (decDataLen > (word32)*pulDataLen) { + /* Report the required size as mandated by PKCS#11. */ + *pulDataLen = decDataLen; + return CKR_BUFFER_TOO_SMALL; + } ret = WP11_Rsa_Verify_Recover(mechanism, pSignature, (word32)ulSignatureLen, pData, pulDataLen, obj); From 78500780de338f72e2979f3a3c3cac8cbb78e766 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 22 Jun 2026 15:43:21 +0100 Subject: [PATCH 3/7] Drop active object reference on C_DestroyObject An operation init function (for example C_SignInit) caches the key object in session->curr as a raw pointer. C_DestroyObject freed the object without clearing that pointer, so completing an asymmetric operation (for example C_Sign with an RSA key) read freed memory. Add WP11_Slot_ClearActiveObject and call it from C_DestroyObject before WP11_Object_Free. For asymmetric key types, which are dereferenced while the operation runs, it scans the slot's sessions and clears session->curr and the operation state for any session whose active object is the one being destroyed. Token objects can be active in more than one session, so all sessions on the slot are scanned. Symmetric operations (AES, HMAC/generic-secret, HKDF) copy the key into the operation context during their *Init and complete without referring to the object again, so a caller may legitimately destroy the key before finishing a multi-part operation. Those operations are left untouched so they can still complete. --- src/crypto.c | 3 +++ src/internal.c | 55 +++++++++++++++++++++++++++++++++++++++++++ wolfpkcs11/internal.h | 2 ++ 3 files changed, 60 insertions(+) diff --git a/src/crypto.c b/src/crypto.c index 9fbbf7e6..879e7aba 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -1692,6 +1692,9 @@ CK_RV C_DestroyObject(CK_SESSION_HANDLE hSession, } rv = WP11_Session_RemoveObject(session, obj); + /* Drop any active-operation reference to this object before freeing it so a + * pending operation cannot use freed memory. */ + WP11_Slot_ClearActiveObject(WP11_Session_GetSlot(session), obj); WP11_Object_Free(obj); WOLFPKCS11_LEAVE("C_DestroyObject", rv); diff --git a/src/internal.c b/src/internal.c index b0fa20fa..a15be42f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8895,6 +8895,61 @@ int WP11_Session_RemoveObject(WP11_Session* session, WP11_Object* object) return ret; } +/** + * Drop any session reference to an object that is about to be freed. + * + * The active object pointer (session->curr) is a raw pointer cached when an + * operation is initialized. A token object can be the active object of more + * than one session. + * + * Symmetric operations (AES, HMAC/generic-secret, HKDF) copy the key into the + * operation context during their *Init and run to completion without referring + * to the object again, so a caller may legitimately destroy the key before + * finishing the operation (wolfCrypt's PKCS#11 layer does exactly this for + * multi-part HMAC). Those operations are left untouched. + * + * Asymmetric operations dereference the key object while running, so for those + * key types drop the active reference in every session that holds it and reset + * the operation state, ensuring a later step cannot use freed memory. + * + * @param slot [in] Slot whose sessions are scanned. + * @param object [in] Object being destroyed. + */ +void WP11_Slot_ClearActiveObject(WP11_Slot* slot, WP11_Object* object) +{ + WP11_Session* curr; + CK_KEY_TYPE keyType; + + if (slot == NULL || object == NULL) + return; + + keyType = WP11_Object_GetType(object); + if (keyType == CKK_AES || keyType == CKK_GENERIC_SECRET || + keyType == CKK_HKDF) + return; + + WP11_Lock_LockRW(&slot->lock); + for (curr = slot->session; curr != NULL; curr = curr->next) { + if (curr->curr == object) { +#if !defined(NO_RSA) && !defined(WC_NO_RSA_OAEP) + /* Release the operation-owned RSA-OAEP label before dropping the + * operation state. Otherwise a later init for a different mechanism + * overwrites the params union and the label can no longer be + * freed. */ + if (curr->mechanism == CKM_RSA_PKCS_OAEP && + curr->params.oaep.label != NULL) { + XFREE(curr->params.oaep.label, NULL, DYNAMIC_TYPE_TMP_BUFFER); + curr->params.oaep.label = NULL; + curr->params.oaep.labelSz = 0; + } +#endif + curr->curr = NULL; + curr->init = 0; + } + } + WP11_Lock_UnlockRW(&slot->lock); +} + /** * Get the current object of the session - key for operation. * diff --git a/wolfpkcs11/internal.h b/wolfpkcs11/internal.h index 7dd783b5..fc9aa104 100644 --- a/wolfpkcs11/internal.h +++ b/wolfpkcs11/internal.h @@ -442,6 +442,8 @@ WP11_LOCAL int WP11_Session_SetMldsaParams(WP11_Session* session, CK_VOID_PTR pa WP11_LOCAL int WP11_Session_AddObject(WP11_Session* session, int onToken, WP11_Object* object); WP11_LOCAL int WP11_Session_RemoveObject(WP11_Session* session, WP11_Object* object); +WP11_LOCAL void WP11_Slot_ClearActiveObject(WP11_Slot* slot, + WP11_Object* object); WP11_LOCAL void WP11_Session_GetObject(WP11_Session* session, WP11_Object** object); WP11_LOCAL void WP11_Session_SetObject(WP11_Session* session, WP11_Object* object); From 3064b8dd8ee5ffef1eef18edc28375bdd4de8e65 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 22 Jun 2026 15:44:37 +0100 Subject: [PATCH 4/7] Bound secret key length in WP11_Object_SetSecretKey WP11_Object_SetSecretKey stored the caller-supplied CKA_VALUE_LEN into key->len after narrowing CK_ULONG to word32 and only checked the result against WP11_MAX_SYM_KEY_SZ afterwards. A value above the cap left the oversized length recorded even though BUFFER_E was returned, and a value outside word32 range (for example 0x100000000) narrowed to a small value that passed the check, so the call returned CKR_OK with mismatched length and key bytes. Bound the requested length against WP11_MAX_SYM_KEY_SZ before narrowing to word32, apply the same bound to the CKA_VALUE length, and reset key->len to zero on any error path so teardown does not zeroize or copy past the fixed key->data buffer. F-5157 --- src/internal.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/internal.c b/src/internal.c index a15be42f..3f737952 100644 --- a/src/internal.c +++ b/src/internal.c @@ -9951,6 +9951,7 @@ int WP11_Object_SetSecretKey(WP11_Object* object, unsigned char** data, { int ret = 0; WP11_Data* key; + CK_ULONG keyLen = 0; if (object->onToken) WP11_Lock_LockRW(object->lock); @@ -9962,22 +9963,35 @@ int WP11_Object_SetSecretKey(WP11_Object* object, unsigned char** data, /* First item is the key's length. */ if (ret == 0 && data[0] != NULL && len[0] != (int)sizeof(CK_ULONG)) ret = BAD_FUNC_ARG; + if (ret == 0 && data[0] != NULL) { + keyLen = *(CK_ULONG*)data[0]; + /* Bound the requested length against the fixed key buffer before + * narrowing to word32. This rejects oversized values and values that + * would not fit a word32, neither of which may be stored in key->len + * as that drives copies and zeroization of key->data. */ + if (keyLen > WP11_MAX_SYM_KEY_SZ) + ret = BUFFER_E; + } #if !defined(NO_AES) && !defined(WOLFPKCS11_NSS) if (ret == 0 && object->type == CKK_AES && data[0] != NULL) { - if (*(CK_ULONG*)data[0] != AES_128_KEY_SIZE && - *(CK_ULONG*)data[0] != AES_192_KEY_SIZE && - *(CK_ULONG*)data[0] != AES_256_KEY_SIZE) { + if (keyLen != AES_128_KEY_SIZE && + keyLen != AES_192_KEY_SIZE && + keyLen != AES_256_KEY_SIZE) { ret = BAD_FUNC_ARG; } } #endif if (ret == 0 && data[0] != NULL) - key->len = (word32)*(CK_ULONG*)data[0]; + key->len = (word32)keyLen; /* Second item is the key data. */ if (ret == 0 && data[1] != NULL) { - if (key->len == 0) - key->len = (word32)len[1]; + if (key->len == 0) { + if (len[1] > WP11_MAX_SYM_KEY_SZ) + ret = BUFFER_E; + else + key->len = (word32)len[1]; + } else if (len[1] != (CK_ULONG)key->len) ret = BUFFER_E; } @@ -9986,6 +10000,11 @@ int WP11_Object_SetSecretKey(WP11_Object* object, unsigned char** data, if (ret == 0 && data[1] != NULL) XMEMCPY(key->data, data[1], key->len); + /* On any error, record no length so later teardown does not zeroize or + * copy past the fixed key buffer. */ + if (ret != 0) + key->len = 0; + if (object->onToken) WP11_Lock_UnlockRW(object->lock); From 46b08e6e4f1da886a5e77984727fd85120ec96a4 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 22 Jun 2026 15:45:35 +0100 Subject: [PATCH 5/7] Clamp key zeroize length in WP11_Object_Free WP11_Object_Free passed object->data.symmKey->len (and the DH equivalent) directly to wc_ForceZero. Both key buffers are fixed-size arrays, so a length recorded larger than the buffer would zeroize past its end. Clamp the zeroize length to the size of the backing buffer for both the symmetric-key and DH branches as a backstop. --- src/internal.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index 3f737952..a1303d26 100644 --- a/src/internal.c +++ b/src/internal.c @@ -9228,8 +9228,13 @@ void WP11_Object_Free(WP11_Object* object) #endif #ifndef NO_DH if (object->type == CKK_DH && object->data.dhKey != NULL) { + word32 zeroLen = object->data.dhKey->len; wc_FreeDhKey(&object->data.dhKey->params); - wc_ForceZero(object->data.dhKey->key, object->data.dhKey->len); + /* Clamp to the fixed buffer as a backstop against a length + * recorded larger than the buffer. */ + if (zeroLen > sizeof(object->data.dhKey->key)) + zeroLen = (word32)sizeof(object->data.dhKey->key); + wc_ForceZero(object->data.dhKey->key, zeroLen); XFREE(object->data.dhKey, NULL, DYNAMIC_TYPE_DH); object->data.dhKey = NULL; } @@ -9244,7 +9249,12 @@ void WP11_Object_Free(WP11_Object* object) #endif if ((object->type == CKK_AES || object->type == CKK_GENERIC_SECRET || object->type == CKK_HKDF) && object->data.symmKey != NULL) { - wc_ForceZero(object->data.symmKey->data, object->data.symmKey->len); + word32 zeroLen = object->data.symmKey->len; + /* Clamp to the fixed buffer as a backstop against a length + * recorded larger than the buffer. */ + if (zeroLen > sizeof(object->data.symmKey->data)) + zeroLen = (word32)sizeof(object->data.symmKey->data); + wc_ForceZero(object->data.symmKey->data, zeroLen); XFREE(object->data.symmKey, NULL, DYNAMIC_TYPE_AES); object->data.symmKey = NULL; } From f02f09493b275280363a4934d181ae1bf160a0bd Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 22 Jun 2026 15:46:10 +0100 Subject: [PATCH 6/7] Guard against length underflow in wp11_Object_Decode_SymmKey The length guard computed object->keyDataLen - AES_BLOCK_SIZE in signed arithmetic and compared it to WP11_MAX_SYM_KEY_SZ. A keyDataLen below AES_BLOCK_SIZE produced a negative value that passed the comparison and was then passed as an unsigned length to wp11_DecryptData and used to size a copy into the fixed symmKey->data buffer. Reject keyDataLen below AES_BLOCK_SIZE first, then bound the payload length as unsigned. A keyDataLen of exactly AES_BLOCK_SIZE is the valid encoding of a zero-length secret (the encoder writes symmKey->len + AES_BLOCK_SIZE), so it must continue to load. F-4263 --- src/internal.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index a1303d26..4077b215 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5245,7 +5245,15 @@ static int wp11_Object_Decode_SymmKey(WP11_Object* object) { int ret = 0; - if (object->keyDataLen - AES_BLOCK_SIZE > WP11_MAX_SYM_KEY_SZ) + /* keyDataLen is signed and covers the encrypted payload plus the trailing + * AES_BLOCK_SIZE tag. Reject lengths below the tag so the payload length + * below cannot go negative when used as an unsigned length. A length of + * exactly AES_BLOCK_SIZE is a valid zero-byte secret: the encoder writes + * symmKey->len + AES_BLOCK_SIZE, so a zero-length key must still load. */ + if (object->keyDataLen < AES_BLOCK_SIZE) + ret = BAD_FUNC_ARG; + if (ret == 0 && + (word32)(object->keyDataLen - AES_BLOCK_SIZE) > WP11_MAX_SYM_KEY_SZ) ret = BUFFER_E; if (ret == 0) { ret = wp11_DecryptData(object->data.symmKey->data, object->keyData, From f45137d033833b568c556760d029acc9e8523018 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 22 Jun 2026 15:46:55 +0100 Subject: [PATCH 7/7] Cap HKDF expand output length in C_DeriveKey The CKM_HKDF_DERIVE/CKM_HKDF_DATA expand branch only rejected a requested output length above word32 range, so a length up to nearly 4 GB was allocated and zeroized before wc_HKDF_Expand rejected it per RFC 5869. Cap the requested length to the RFC 5869 maximum (255 * HashLen) before allocation; this also keeps the value within word32 range. F-4806 --- src/crypto.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/crypto.c b/src/crypto.c index 879e7aba..6650db9d 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -49,6 +49,12 @@ #define CK_ULONG_FITS_WORD32(v) \ ((v) <= (CK_ULONG)0xFFFFFFFF - CK_ULONG_MAX_OVERHEAD) +/* RFC 5869 limits HKDF-Expand output to 255 * HashLen octets. Cap the + * requested output length to the largest such value across supported digests + * so an oversized request does not drive a large allocation and zeroization + * before wolfCrypt enforces the per-digest limit. */ +#define WP11_MAX_HKDF_OUTPUT (255 * WC_MAX_DIGEST_SIZE) + #define CHECK_KEYTYPE(kt) \ (kt == CKK_RSA || kt == CKK_EC || kt == CKK_DH || \ kt == CKK_AES || kt == CKK_HKDF || kt == CKK_GENERIC_SECRET) ? \ @@ -8800,8 +8806,9 @@ CK_RV C_DeriveKey(CK_SESSION_HANDLE hSession, return CKR_ATTRIBUTE_VALUE_INVALID; } XMEMCPY(&reqLen, lenAttr->pValue, sizeof(CK_ULONG)); - /* On 64-bit, CK_ULONG may exceed word32 range */ - if (reqLen == 0 || reqLen > (CK_ULONG)0xFFFFFFFF) { + /* Reject zero and cap to the RFC 5869 maximum, which also + * keeps the value within word32 range. */ + if (reqLen == 0 || reqLen > WP11_MAX_HKDF_OUTPUT) { return CKR_ATTRIBUTE_VALUE_INVALID; } keyLen = (word32)reqLen;