From c47f5bc60f651441cc2da26eacee99af6f80f24e Mon Sep 17 00:00:00 2001 From: Emma Stensland Date: Wed, 15 Jul 2026 15:15:20 -0600 Subject: [PATCH] Fix generic bugs, memory leaks, and migrate to wc_ForceZero --- src/sign-verify/clu_sign.c | 154 +++++++++++++++++++++------------ src/sign-verify/clu_verify.c | 62 +++++++++---- src/tools/clu_funcs.c | 9 +- wolfclu/sign-verify/clu_sign.h | 5 ++ 4 files changed, 158 insertions(+), 72 deletions(-) diff --git a/src/sign-verify/clu_sign.c b/src/sign-verify/clu_sign.c index 27201250..831dc8da 100644 --- a/src/sign-verify/clu_sign.c +++ b/src/sign-verify/clu_sign.c @@ -25,67 +25,86 @@ #include #include /* for xmss callback functions */ -/* Fallback for older wolfSSL builds that have HAVE_DILITHIUM but predate the - * PEM size constants. DILITHIUM_LEVEL5_BOTH_KEY_PEM_SIZE (10267) is the - * largest possible key PEM size and is used as a safe upper bound. */ -#ifdef HAVE_DILITHIUM -#ifndef DILITHIUM_MAX_BOTH_KEY_PEM_SIZE -#define DILITHIUM_MAX_BOTH_KEY_PEM_SIZE 10267 -#endif -#endif +#include + +/* Upper bound on DER size out of wolfCLU_KeyPemToDer, covering all key types. */ +#ifndef WOLFCLU_MAX_KEY_PEM_DER_SZ +#define WOLFCLU_MAX_KEY_PEM_DER_SZ 65536 +#endif /* WOLFCLU_MAX_KEY_PEM_DER_SZ */ #ifndef WOLFCLU_NO_FILESYSTEM int wolfCLU_KeyPemToDer(unsigned char** pkeyBuf, int pkeySz, int pubIn) { int ret = 0; byte* der = NULL; - const unsigned char* keyBuf = *pkeyBuf; + const unsigned char* keyBuf; + + if (pkeyBuf == NULL || *pkeyBuf == NULL || pkeySz <= 0) { + return BAD_FUNC_ARG; + } + keyBuf = *pkeyBuf; if (pubIn == 0) { ret = wc_KeyPemToDer(keyBuf, pkeySz, NULL, 0, NULL); if (ret > 0) { - wolfCLU_Log(WOLFCLU_L0, "DER size: %d", ret); int derSz = ret; - der = (byte*)XMALLOC(derSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (der == NULL) { - wolfCLU_Log(WOLFCLU_L0, "Failed to allocate memory for DER"); - ret = MEMORY_E; + if (derSz > WOLFCLU_MAX_KEY_PEM_DER_SZ) { + /* Leave *pkeyBuf intact; caller frees it. */ + wolfCLU_LogError("PEM private key DER size exceeds limit"); + ret = WOLFCLU_FATAL_ERROR; } else { - ret = wc_KeyPemToDer(keyBuf, pkeySz, der, derSz, NULL); - if (ret > 0) { - wolfCLU_Log(WOLFCLU_L0, "DER size2: %d", ret); - /* replace incoming pkeyBuf with new der buf */ - XFREE(*pkeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - *pkeyBuf = der; + der = (byte*)XMALLOC(derSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + if (der == NULL) { + wolfCLU_LogError("Failed to allocate memory for DER"); + ret = MEMORY_E; } else { - wolfCLU_Log(WOLFCLU_L0, "Failed to convert PEM to DER"); - /* failure, so cleanup */ - XFREE(der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + ret = wc_KeyPemToDer(keyBuf, pkeySz, der, derSz, NULL); + if (ret > 0) { + /* *pkeyBuf still points to the original PEM alloc here + * (pkeySz is its size); 'der' is the new DER alloc. */ + wolfCLU_ForceZero(*pkeyBuf, (unsigned int)pkeySz); + XFREE(*pkeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + *pkeyBuf = der; + } + else { + wolfCLU_LogError("Failed to convert PEM to DER"); + /* failure, so cleanup */ + wolfCLU_ForceZero(der, (unsigned int)derSz); + XFREE(der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + } } } } - wolfCLU_Log(WOLFCLU_L0, "DER size3: %d", ret); } else { ret = wc_PubKeyPemToDer(keyBuf, pkeySz, NULL, 0); if (ret > 0) { int derSz = ret; - der = (byte*)XMALLOC(derSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (der == NULL) { - ret = MEMORY_E; + if (derSz > WOLFCLU_MAX_KEY_PEM_DER_SZ) { + wolfCLU_LogError("PEM public key DER size exceeds limit"); + ret = WOLFCLU_FATAL_ERROR; } else { - ret = wc_PubKeyPemToDer(keyBuf, pkeySz, der, derSz); - if (ret > 0) { - /* replace incoming pkeyBuf with new der buf */ - XFREE(*pkeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - *pkeyBuf = der; + der = (byte*)XMALLOC(derSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + if (der == NULL) { + ret = MEMORY_E; } else { - /* failure, so cleanup */ - XFREE(der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + ret = wc_PubKeyPemToDer(keyBuf, pkeySz, der, derSz); + if (ret > 0) { + /* replace incoming pkeyBuf with new der buf */ + wolfCLU_ForceZero(*pkeyBuf, (unsigned int)pkeySz); + XFREE(*pkeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + *pkeyBuf = der; + } + else { + wolfCLU_LogError( + "Failed to convert public key PEM to DER"); + wolfCLU_ForceZero(der, (unsigned int)derSz); + XFREE(der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + } } } } @@ -121,13 +140,14 @@ int wolfCLU_sign_data(char* in, char* out, char* privKey, int keyType, } fSz = (int)fTell; - data = (byte*)XMALLOC(fSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + data = (byte*)XMALLOC((size_t)fSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); if (data == NULL) { XFCLOSE(f); return MEMORY_E; } - if (XFSEEK(f, 0, SEEK_SET) != 0 || (int)XFREAD(data, 1, fSz, f) != fSz) { + if (XFSEEK(f, 0, SEEK_SET) != 0 || + XFREAD(data, 1, (size_t)fSz, f) != (size_t)fSz) { XFREE(data, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); XFCLOSE(f); return WOLFCLU_FATAL_ERROR; @@ -137,20 +157,22 @@ int wolfCLU_sign_data(char* in, char* out, char* privKey, int keyType, switch(keyType) { case RSA_SIG_VER: - ret = wolfCLU_sign_data_rsa(data, out, fSz, privKey, inForm); + ret = wolfCLU_sign_data_rsa(data, out, (word32)fSz, privKey, inForm); break; case ECC_SIG_VER: - ret = wolfCLU_sign_data_ecc(data, out, fSz, privKey, inForm); + ret = wolfCLU_sign_data_ecc(data, out, (word32)fSz, privKey, inForm); break; case ED25519_SIG_VER: - ret = wolfCLU_sign_data_ed25519(data, out, fSz, privKey, inForm); + ret = wolfCLU_sign_data_ed25519(data, out, (word32)fSz, privKey, + inForm); break; #ifdef HAVE_DILITHIUM case DILITHIUM_SIG_VER: - ret = wolfCLU_sign_data_dilithium(data, out, fSz, privKey, inForm); + ret = wolfCLU_sign_data_dilithium(data, out, (word32)fSz, privKey, + inForm); break; #endif @@ -256,7 +278,14 @@ int wolfCLU_sign_data_rsa(byte* data, char* out, word32 dataSz, char* privKey, if (inForm == PEM_FORM && ret == 0) { ret = wolfCLU_KeyPemToDer(&keyBuf, privFileSz, 0); if (ret < 0) { - wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) { + WOLFCLU_LOG(WOLFCLU_L0, + "No PEM header found, treating as DER."); + ret = 0; + } + else { + wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + } } else { privFileSz = ret; @@ -317,7 +346,7 @@ int wolfCLU_sign_data_rsa(byte* data, char* out, word32 dataSz, char* privKey, } if (keyBuf!= NULL) { - wolfCLU_ForceZero(keyBuf, privFileSz); + wolfCLU_ForceZero(keyBuf, (unsigned int)privFileSz); XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); } if (outBuf!= NULL) { @@ -409,7 +438,14 @@ int wolfCLU_sign_data_ecc(byte* data, char* out, word32 fSz, char* privKey, if (inForm == PEM_FORM && ret == 0) { ret = wolfCLU_KeyPemToDer(&keyBuf, privFileSz, 0); if (ret < 0) { - wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) { + WOLFCLU_LOG(WOLFCLU_L0, + "No PEM header found, treating as DER."); + ret = 0; + } + else { + wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + } } else { privFileSz = ret; @@ -494,7 +530,7 @@ int wolfCLU_sign_data_ecc(byte* data, char* out, word32 fSz, char* privKey, } if (keyBuf!= NULL) { - wolfCLU_ForceZero(keyBuf, privFileSz); + wolfCLU_ForceZero(keyBuf, (unsigned int)privFileSz); XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); } if (outBuf!= NULL) { @@ -585,7 +621,14 @@ int wolfCLU_sign_data_ed25519 (byte* data, char* out, word32 fSz, char* privKey, if (inForm == PEM_FORM && ret == 0) { ret = wolfCLU_KeyPemToDer(&keyBuf, privFileSz, 0); if (ret < 0) { - wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) { + WOLFCLU_LOG(WOLFCLU_L0, + "No PEM header found, treating as DER."); + ret = 0; + } + else { + wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + } } else { privFileSz = ret; @@ -659,7 +702,7 @@ int wolfCLU_sign_data_ed25519 (byte* data, char* out, word32 fSz, char* privKey, } if (keyBuf!= NULL) { - wolfCLU_ForceZero(keyBuf, privFileSz); + wolfCLU_ForceZero(keyBuf, (unsigned int)privFileSz); XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); } if (outBuf!= NULL) { @@ -679,7 +722,7 @@ int wolfCLU_sign_data_ed25519 (byte* data, char* out, word32 fSz, char* privKey, int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* privKey, int inForm) { -#ifdef HAVE_DILITHIUM +#if defined(HAVE_DILITHIUM) int ret = 0; int privFileSz = 0; word32 index = 0; @@ -705,7 +748,6 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri dilithium_key key[1]; #endif - /* zero before init for defensive security */ XMEMSET(&rng, 0, sizeof(rng)); XMEMSET(key, 0, sizeof(dilithium_key)); @@ -770,7 +812,14 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri if (inForm == PEM_FORM && ret == 0) { ret = wolfCLU_KeyPemToDer(&privBuf, privFileSz, 0); if (ret < 0) { - wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) { + WOLFCLU_LOG(WOLFCLU_L0, + "No PEM header found, treating as DER."); + ret = 0; + } + else { + wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + } } else { /* update privBuf and privFileSz with the converted DER data */ @@ -827,7 +876,7 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri XFCLOSE(privKeyFile); if (privBuf != NULL) { - wolfCLU_ForceZero(privBuf, privBufSz); + wolfCLU_ForceZero(privBuf, (unsigned int)privBufSz); XFREE(privBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); } @@ -836,9 +885,8 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri } wc_dilithium_free(key); - /* rng was zeroed via XMEMSET before wc_InitRng, so wc_FreeRng is safe - * even if wc_InitRng failed: wolfSSL checks rng->drbg != NULL internally - * before freeing any resources. */ + /* rng zeroed via XMEMSET before wc_InitRng, so even if wc_InitRng failed: + * wolfSSL checks rng->drbg internally before freeing. */ wc_FreeRng(&rng); #ifdef WOLFSSL_SMALL_STACK XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); diff --git a/src/sign-verify/clu_verify.c b/src/sign-verify/clu_verify.c index b0a760a4..da8db85b 100644 --- a/src/sign-verify/clu_verify.c +++ b/src/sign-verify/clu_verify.c @@ -408,7 +408,14 @@ int wolfCLU_verify_signature_rsa(byte* sig, char* out, int sigSz, char* keyPath, if (inForm == PEM_FORM && ret == 0) { ret = wolfCLU_KeyPemToDer(&keyBuf, (int)keyFileSz, pubIn); if (ret < 0) { - wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) { + WOLFCLU_LOG(WOLFCLU_L0, + "No PEM header found, treating as DER."); + ret = 0; + } + else { + wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + } } else { keyFileSz = ret; @@ -561,7 +568,14 @@ int wolfCLU_verify_signature_ecc(byte* sig, int sigSz, byte* hash, int hashSz, if (inForm == PEM_FORM && ret == 0) { ret = wolfCLU_KeyPemToDer(&keyBuf, (int)keyFileSz, pubIn); if (ret < 0) { - wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) { + WOLFCLU_LOG(WOLFCLU_L0, + "No PEM header found, treating as DER."); + ret = 0; + } + else { + wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + } } else { keyFileSz = ret; @@ -729,7 +743,14 @@ int wolfCLU_verify_signature_ed25519(byte* sig, int sigSz, if (inForm == PEM_FORM && ret == 0) { ret = wolfCLU_KeyPemToDer(&keyBuf, (int)keyFileSz, pubIn); if (ret < 0) { - wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) { + WOLFCLU_LOG(WOLFCLU_L0, + "No PEM header found, treating as DER."); + ret = 0; + } + else { + wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + } } else { keyFileSz = ret; @@ -865,8 +886,8 @@ int wolfCLU_verify_signature_dilithium(byte* sig, int sigSz, byte* msg, XFSEEK(keyFile, 0, SEEK_END); keyFileSz = XFTELL(keyFile); - if (keyFileSz < 0) { - wolfCLU_LogError("Failed to get size of public key FILE."); + if (keyFileSz <= 0) { + wolfCLU_LogError("Failed to get valid size of public key FILE."); XFCLOSE(keyFile); wc_dilithium_free(key); #ifdef WOLFSSL_SMALL_STACK @@ -874,9 +895,8 @@ int wolfCLU_verify_signature_dilithium(byte* sig, int sigSz, byte* msg, #endif return BAD_FUNC_ARG; } - if (keyFileSz > WOLFCLU_MAX_FILE_SIZE) { - wolfCLU_LogError("File: %s exceeds max size of 0x%X " - "bytes.", keyPath, (unsigned)WOLFCLU_MAX_FILE_SIZE); + if (keyFileSz > DILITHIUM_MAX_BOTH_KEY_PEM_SIZE) { + wolfCLU_LogError("Incorrect public key file size: %ld", keyFileSz); XFCLOSE(keyFile); wc_dilithium_free(key); #ifdef WOLFSSL_SMALL_STACK @@ -884,7 +904,8 @@ int wolfCLU_verify_signature_dilithium(byte* sig, int sigSz, byte* msg, #endif return WOLFCLU_FATAL_ERROR; } - keyBuf = (byte*)XMALLOC(keyFileSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + + keyBuf = (byte*)XMALLOC(keyFileSz + 1, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); if (keyBuf == NULL) { wolfCLU_LogError("Failed to malloc key buffer."); XFCLOSE(keyFile); @@ -894,7 +915,7 @@ int wolfCLU_verify_signature_dilithium(byte* sig, int sigSz, byte* msg, #endif return MEMORY_E; } - XMEMSET(keyBuf, 0, keyFileSz); + XMEMSET(keyBuf, 0, keyFileSz + 1); if (XFSEEK(keyFile, 0, SEEK_SET) != 0 || (int)XFREAD(keyBuf, 1, keyFileSz, keyFile) != keyFileSz) { @@ -914,13 +935,20 @@ int wolfCLU_verify_signature_dilithium(byte* sig, int sigSz, byte* msg, if (inForm == PEM_FORM) { ret = wolfCLU_KeyPemToDer(&keyBuf, (int)keyFileSz, 1); if (ret < 0) { - wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); - XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - wc_dilithium_free(key); - #ifdef WOLFSSL_SMALL_STACK - XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return ret; + if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) { + WOLFCLU_LOG(WOLFCLU_L0, + "No PEM header found, treating as DER."); + ret = 0; + } + else { + wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); + XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + wc_dilithium_free(key); + #ifdef WOLFSSL_SMALL_STACK + XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + #endif + return ret; + } } else { keyBufSz = ret; diff --git a/src/tools/clu_funcs.c b/src/tools/clu_funcs.c index 2aea51e3..364bef3f 100644 --- a/src/tools/clu_funcs.c +++ b/src/tools/clu_funcs.c @@ -1103,8 +1103,13 @@ void wolfCLU_convertToLower(char* s, int sSz) void wolfCLU_ForceZero(void* mem, unsigned int len) { +#ifndef WOLFSSL_NO_FORCE_ZERO + wc_ForceZero(mem, len); +#else + /* wc_ForceZero unavailable in this build; use a volatile loop instead. */ volatile byte* z = (volatile byte*)mem; while (len--) *z++ = 0; +#endif } #ifndef WOLFCLU_NO_TERM_SUPPORT @@ -1434,7 +1439,7 @@ int wolfCLU_hmacHash(WOLFSSL_HMAC_CTX *ctx, void* key, word32 len, ret = WOLFCLU_FATAL_ERROR; } } - wc_ForceZero(chunk, sizeof(chunk)); + wolfCLU_ForceZero(chunk, sizeof(chunk)); } if (ret == WOLFCLU_SUCCESS) { @@ -1455,6 +1460,6 @@ int wolfCLU_hmacHash(WOLFSSL_HMAC_CTX *ctx, void* key, word32 len, } } - wc_ForceZero(chunk, sizeof(chunk)); + wolfCLU_ForceZero(chunk, sizeof(chunk)); return ret; } diff --git a/wolfclu/sign-verify/clu_sign.h b/wolfclu/sign-verify/clu_sign.h index 22f2be60..86ff9205 100644 --- a/wolfclu/sign-verify/clu_sign.h +++ b/wolfclu/sign-verify/clu_sign.h @@ -34,6 +34,11 @@ #endif #ifdef HAVE_DILITHIUM #include + /* Fallback for older wolfSSL builds predating this constant; 10267 is + * the largest possible key PEM size (DILITHIUM_LEVEL5_BOTH_KEY_PEM_SIZE). */ + #ifndef DILITHIUM_MAX_BOTH_KEY_PEM_SIZE + #define DILITHIUM_MAX_BOTH_KEY_PEM_SIZE 10267 + #endif #endif #ifdef WOLFSSL_HAVE_XMSS #include