More static code analysis fixes

.github/workflows/sanitizer-tests.yml

@@ -18,7 +18,7 @@ jobs:
         sanitizer: [asan, ubsan]
         config:
           - name: "Standard Build"
-            configure_flags: ""
+            configure_flags: "--enable-aeskeywrap"
           - name: "NSS Build"
             configure_flags: "--enable-nss"
           - name: "TPM Build"

src/crypto.c

@@ -7864,10 +7864,18 @@ CK_RV C_DeriveKey(CK_SESSION_HANDLE hSession,
             FindAttributeType(pTemplate, ulAttributeCount, CKA_VALUE_LEN,
                 &lenAttr);
             if (kdfParams->bExpand) {
-                if (!lenAttr) {
-                    return CKR_MECHANISM_PARAM_INVALID;
+                CK_ULONG reqLen;
+                if (!lenAttr || !lenAttr->pValue) {
+                    return CKR_ATTRIBUTE_VALUE_INVALID;
+                }
+                if (lenAttr->ulValueLen != sizeof(CK_ULONG)) {
+                    return CKR_ATTRIBUTE_VALUE_INVALID;
                 }
-                keyLen = *(word32*)lenAttr->pValue;
+                XMEMCPY(&reqLen, lenAttr->pValue, sizeof(CK_ULONG));
+                if (reqLen == 0 || reqLen > (CK_ULONG)0xFFFFFFFF) {
+                    return CKR_ATTRIBUTE_VALUE_INVALID;
+                }
+                keyLen = (word32)reqLen;
             }
             else {
                 keyLen = WC_MAX_DIGEST_SIZE;
@@ -7944,14 +7952,26 @@ CK_RV C_DeriveKey(CK_SESSION_HANDLE hSession,
             if (tlsParams->pReturnedKeyMaterial == NULL)
                 return CKR_MECHANISM_PARAM_INVALID;
 
-            keyLen = (word32)(2 * tlsParams->ulMacSizeInBits) +
-                     (word32)(2 * tlsParams->ulKeySizeInBits) +
-                     (word32)(2 * tlsParams->ulIVSizeInBits);
-            if (keyLen == 0)
-                return CKR_MECHANISM_PARAM_INVALID;
-            if ((keyLen % 8) != 0)
-                return CKR_MECHANISM_PARAM_INVALID;
-            keyLen /= 8;
+            {
+                CK_ULONG totalBits;
+                /* Validate individual fields won't overflow when doubled */
+                if (tlsParams->ulMacSizeInBits > ((CK_ULONG)0xFFFFFFFF / 2) ||
+                    tlsParams->ulKeySizeInBits > ((CK_ULONG)0xFFFFFFFF / 2) ||
+                    tlsParams->ulIVSizeInBits  > ((CK_ULONG)0xFFFFFFFF / 2)) {
+                    return CKR_MECHANISM_PARAM_INVALID;
+                }
+                totalBits = (2 * tlsParams->ulMacSizeInBits) +
+                            (2 * tlsParams->ulKeySizeInBits) +
+                            (2 * tlsParams->ulIVSizeInBits);
+                if (totalBits == 0)
+                    return CKR_MECHANISM_PARAM_INVALID;
+                if ((totalBits % 8) != 0)
+                    return CKR_MECHANISM_PARAM_INVALID;
+                totalBits /= 8;
+                if (totalBits > (CK_ULONG)0xFFFFFFFF)
+                    return CKR_MECHANISM_PARAM_INVALID;
+                keyLen = (word32)totalBits;
+            }
 
             derivedKey = (byte*)XMALLOC(keyLen, NULL, DYNAMIC_TYPE_TMP_BUFFER);
             if (derivedKey == NULL)
@@ -8081,7 +8101,7 @@ CK_RV C_DeriveKey(CK_SESSION_HANDLE hSession,
         }
     }
 
-    if (rv == CKR_OK) {
+    if ((rv == CKR_OK) && (derivedKey != NULL)) {
         rv = SetInitialStates(obj);
     }
 

src/internal.c

@@ -945,6 +945,13 @@ static void wp11_Session_Final(WP11_Session* session)
         session->init = 0;
     }
 #endif
+#ifdef HAVE_AES_KEYWRAP
+    if ((session->mechanism == CKM_AES_KEY_WRAP ||
+                  session->mechanism == CKM_AES_KEY_WRAP_PAD) && session->init) {
+        wc_AesFree(&session->params.kw.aes);
+        session->init = 0;
+    }
+#endif
 #ifdef HAVE_AESCCM
     if (session->mechanism == CKM_AES_CCM) {
         if (session->params.ccm.aad != NULL) {
@@ -2554,7 +2561,7 @@ static long GetRsaExponentValue(unsigned char* eData, word32 eSz)
     long e = 0;
 
     /* Convert big-endian data into number. */
-    for (i = eSz - 1; i >= 0; i--) {
+    for (i = 0; i < (int)eSz; i++) {
         e <<= 8;
         e |= eData[i];
     }
@@ -8281,6 +8288,7 @@ void WP11_Object_Free(WP11_Object* object)
     #ifndef NO_DH
         if (object->type == CKK_DH && object->data.dhKey != NULL) {
             wc_FreeDhKey(&object->data.dhKey->params);
+            wc_ForceZero(object->data.dhKey->key, object->data.dhKey->len);
             XFREE(object->data.dhKey, NULL, DYNAMIC_TYPE_DH);
             object->data.dhKey = NULL;
         }
@@ -8745,6 +8753,7 @@ int WP11_Object_SetMldsaKey(WP11_Object* object, unsigned char** data,
                             ret = BAD_FUNC_ARG;
                         }
                     }
+                    wc_ForceZero(expandedKey, expandedKeyLen);
                     XFREE(expandedKey, NULL, DYNAMIC_TYPE_TMP_BUFFER);
                 }
             }
@@ -8831,7 +8840,7 @@ int WP11_Object_SetSecretKey(WP11_Object* object, unsigned char** data,
 
     key = object->data.symmKey;
     key->len = 0;
-    XMEMSET(key->data, 0, sizeof(key->data));
+    wc_ForceZero(key->data, sizeof(key->data));
 
     /* First item is the key's length. */
     if (ret == 0 && data[0] != NULL && len[0] != (int)sizeof(CK_ULONG))
@@ -13655,6 +13664,7 @@ int WP11_AesKeyWrap_Encrypt(unsigned char* plain, word32 plainSz,
 
     ret = wc_AesKeyWrap_ex(&wrap->aes, plain, plainSz, enc, *encSz,
             wrap->ivSz != 0 ? wrap->iv : NULL);
+    wc_AesFree(&wrap->aes);
     session->init = 0;
     if (ret < 0)
         return ret;
@@ -13670,6 +13680,7 @@ int WP11_AesKeyWrap_Decrypt(unsigned char* enc, word32 encSz,
 
     ret = wc_AesKeyUnWrap_ex(&wrap->aes, enc, encSz, dec, *decSz,
             wrap->ivSz != 0 ? wrap->iv : NULL);
+    wc_AesFree(&wrap->aes);
     session->init = 0;
     if (ret < 0)
         return ret;

tests/include.am

@@ -61,6 +61,11 @@ noinst_PROGRAMS += tests/pkcs11v3test
 tests_pkcs11v3test_SOURCES = tests/pkcs11v3test.c
 tests_pkcs11v3test_LDADD =
 
+check_PROGRAMS += tests/rsa_exponent_test
+noinst_PROGRAMS += tests/rsa_exponent_test
+tests_rsa_exponent_test_SOURCES = tests/rsa_exponent_test.c
+tests_rsa_exponent_test_LDADD =
+
 if BUILD_STATIC
 tests_pkcs11test_LDADD += src/libwolfpkcs11.la
 tests_pkcs11mtt_LDADD  += src/libwolfpkcs11.la
@@ -74,6 +79,7 @@ tests_find_objects_null_template_test_LDADD += src/libwolfpkcs11.la
 tests_aes_cbc_pad_padding_test_LDADD += src/libwolfpkcs11.la
 tests_ecb_check_value_error_test_LDADD += src/libwolfpkcs11.la
 tests_pkcs11v3test_LDADD += src/libwolfpkcs11.la
+tests_rsa_exponent_test_LDADD += src/libwolfpkcs11.la
 else
 tests_object_id_uniqueness_test_LDADD += src/libwolfpkcs11.la
 tests_empty_pin_store_test_LDADD += src/libwolfpkcs11.la

tests/pkcs11test.c

@@ -14827,6 +14827,112 @@ static CK_RV test_hkdf_gen_key(void* args)
 
     return ret;
 }
+
+/* Regression test: HKDF expand with NULL pValue in CKA_VALUE_LEN
+ * previously crashed (Issue #1315: lenAttr->pValue was dereferenced
+ * without NULL check).
+ */
+static CK_RV test_hkdf_derive_expand_null_value_len(void* args)
+{
+    CK_SESSION_HANDLE session = *(CK_SESSION_HANDLE*)args;
+    CK_RV ret;
+    CK_OBJECT_HANDLE hBaseKey = CK_INVALID_HANDLE;
+    CK_OBJECT_HANDLE hPrk = CK_INVALID_HANDLE;
+    CK_OBJECT_HANDLE hExpandKey = CK_INVALID_HANDLE;
+
+    CK_BYTE ikm[] = {
+        0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b,
+        0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b,
+        0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b
+    };
+    CK_ULONG ikm_len = sizeof(ikm);
+    CK_BYTE salt[] = {
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+        0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f
+    };
+    CK_BYTE info[] = { 0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7,
+                        0xf8, 0xf9 };
+    CK_ULONG prk_len = 32;
+
+    /* Create IKM as a secret key object */
+    CK_ATTRIBUTE templateSecret[] = {
+        {CKA_CLASS, &secretKeyClass, sizeof(secretKeyClass)},
+        {CKA_KEY_TYPE, &genericKeyType, sizeof(genericKeyType)},
+        {CKA_TOKEN, &ckFalse, sizeof(ckFalse)},
+        {CKA_PRIVATE, &ckTrue, sizeof(ckTrue)},
+        {CKA_SENSITIVE, &ckFalse, sizeof(ckFalse)},
+        {CKA_EXTRACTABLE, &ckTrue, sizeof(ckTrue)},
+        {CKA_VALUE, ikm, ikm_len},
+        {CKA_VALUE_LEN, &ikm_len, sizeof(ikm_len)},
+        {CKA_DERIVE, &ckTrue, sizeof(ckTrue)}
+    };
+    CK_ULONG templateSecretCount =
+        sizeof(templateSecret) / sizeof(templateSecret[0]);
+
+    /* Extract params */
+    CK_HKDF_PARAMS paramsExtract = {
+        CK_TRUE, CK_FALSE, CKM_SHA256_HMAC,
+        CKF_HKDF_SALT_DATA, salt, sizeof(salt),
+        CK_INVALID_HANDLE, NULL_PTR, 0
+    };
+    CK_MECHANISM mechExtract =
+        { CKM_HKDF_DERIVE, &paramsExtract, sizeof(paramsExtract) };
+    CK_ATTRIBUTE templateExtract[] = {
+        {CKA_CLASS, &secretKeyClass, sizeof(secretKeyClass)},
+        {CKA_KEY_TYPE, &genericKeyType, sizeof(genericKeyType)},
+        {CKA_SENSITIVE, &ckFalse, sizeof(ckFalse)},
+        {CKA_EXTRACTABLE, &ckTrue, sizeof(ckTrue)},
+        {CKA_VALUE_LEN, &prk_len, sizeof(prk_len)}
+    };
+    CK_ULONG templateExtractCount =
+        sizeof(templateExtract) / sizeof(templateExtract[0]);
+
+    /* Expand params - expand only */
+    CK_HKDF_PARAMS paramsExpand = {
+        CK_FALSE, CK_TRUE, CKM_SHA256_HMAC,
+        CKF_HKDF_SALT_NULL, NULL_PTR, 0,
+        CK_INVALID_HANDLE, info, sizeof(info)
+    };
+    CK_MECHANISM mechExpand =
+        { CKM_HKDF_DERIVE, &paramsExpand, sizeof(paramsExpand) };
+
+    /* Expand template with NULL pValue for CKA_VALUE_LEN.
+     * This triggers the NULL dereference at crypto.c:7870.
+     */
+    CK_ATTRIBUTE templateExpand[] = {
+        {CKA_CLASS, &secretKeyClass, sizeof(secretKeyClass)},
+        {CKA_KEY_TYPE, &genericKeyType, sizeof(genericKeyType)},
+        {CKA_SENSITIVE, &ckFalse, sizeof(ckFalse)},
+        {CKA_EXTRACTABLE, &ckTrue, sizeof(ckTrue)},
+        {CKA_VALUE_LEN, NULL_PTR, 0}
+    };
+    CK_ULONG templateExpandCount =
+        sizeof(templateExpand) / sizeof(templateExpand[0]);
+
+    /* Step 1: Create base key */
+    ret = funcList->C_CreateObject(session, templateSecret,
+        templateSecretCount, &hBaseKey);
+    CHECK_CKR(ret, "Create IKM object");
+
+    /* Step 2: Extract to get PRK */
+    if (ret == CKR_OK) {
+        ret = funcList->C_DeriveKey(session, &mechExtract, hBaseKey,
+            templateExtract, templateExtractCount, &hPrk);
+        CHECK_CKR(ret, "HKDF extract");
+    }
+
+    /* Step 3: Expand with NULL CKA_VALUE_LEN pValue - previously crashed */
+    if (ret == CKR_OK) {
+        ret = funcList->C_DeriveKey(session, &mechExpand, hPrk,
+            templateExpand, templateExpandCount, &hExpandKey);
+        /* Before the fix, this line is never reached (NULL deref crash).
+         * After the fix, expect an error return. */
+        CHECK_CKR_FAIL(ret, CKR_ATTRIBUTE_VALUE_INVALID,
+            "HKDF expand with NULL CKA_VALUE_LEN pValue");
+    }
+
+    return ret;
+}
 #endif
 
 #ifndef NO_SHA
@@ -16400,6 +16506,7 @@ static TEST_FUNC testFunc[] = {
     PKCS11TEST_FUNC_SESS_DECL(test_hkdf_derive_expand_with_extract_null_salt),
     PKCS11TEST_FUNC_SESS_DECL(test_hkdf_derive_extract_with_expand_salt_key),
     PKCS11TEST_FUNC_SESS_DECL(test_hkdf_gen_key),
+    PKCS11TEST_FUNC_SESS_DECL(test_hkdf_derive_expand_null_value_len),
 #endif
 #ifdef WOLFSSL_HAVE_PRF
 #ifndef NO_MD5