Several hardening fixes

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) ? \
@@ -1692,6 +1698,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);
@@ -6806,14 +6815,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 +6833,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);
@@ -8784,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;

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,
@@ -8895,6 +8903,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.
  *
@@ -9173,8 +9236,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;
         }
@@ -9189,7 +9257,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;
         }
@@ -9896,6 +9969,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);
@@ -9907,22 +9981,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;
     }
@@ -9931,6 +10018,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);
 
@@ -12711,12 +12803,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++) {

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);