Skip to content

Fix PEM input validation and zeroize sensitive key buffers #10231

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

Open
JeremiahM37 wants to merge 3 commits into
base: master
Choose a base branch
from
+36 −3
Open

Fix PEM input validation and zeroize sensitive key buffers #10231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bdebcfc
reject negative pemSz in PEM-to-DER APIs
JeremiahM37 Apr 15, 2026
897ce55
zero HMAC-DRBG K and V in deterministic k
JeremiahM37 Apr 15, 2026
e182645
zero PKCS12 key bag buffers before free
JeremiahM37 Apr 15, 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
14 changes: 14 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11869,6 +11869,10 @@ static int test_wc_CertPemToDer(void)
(int)cert_dersz, CERT_TYPE), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
ExpectIntEQ(wc_CertPemToDer(cert_buf, (int)cert_sz, cert_der, -1,
CERT_TYPE), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
ExpectIntEQ(wc_CertPemToDer(cert_buf, -1, cert_der, (int)cert_dersz,
CERT_TYPE), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
ExpectIntEQ(wc_CertPemToDer(cert_buf, 0, cert_der, (int)cert_dersz,
CERT_TYPE), WC_NO_ERR_TRACE(BAD_FUNC_ARG));

if (cert_der != NULL)
free(cert_der);
Expand Down Expand Up @@ -11925,6 +11929,12 @@ static int test_wc_KeyPemToDer(void)
ExpectIntEQ(wc_KeyPemToDer(cert_buf, cert_sz, (byte*)&cert_der, 0, ""),
WC_NO_ERR_TRACE(BAD_FUNC_ARG));

/* Bad arg: negative or zero pemSz */
ExpectIntEQ(wc_KeyPemToDer(cert_buf, -1, (byte*)&cert_der, cert_sz, ""),
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
ExpectIntEQ(wc_KeyPemToDer(cert_buf, 0, (byte*)&cert_der, cert_sz, ""),
WC_NO_ERR_TRACE(BAD_FUNC_ARG));

/* Test normal operation */
cert_dersz = cert_sz; /* DER will be smaller than PEM */
ExpectNotNull(cert_der = (byte*)malloc((size_t)cert_dersz));
Expand Down Expand Up @@ -11968,6 +11978,10 @@ static int test_wc_PubKeyPemToDer(void)
ExpectIntEQ(load_file(key, &cert_buf, &cert_sz), 0);
cert_dersz = cert_sz; /* DER will be smaller than PEM */
ExpectNotNull(cert_der = (byte*)malloc(cert_dersz));
ExpectIntEQ(wc_PubKeyPemToDer(cert_buf, -1, cert_der, (int)cert_dersz),
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
ExpectIntEQ(wc_PubKeyPemToDer(cert_buf, 0, cert_der, (int)cert_dersz),
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
ExpectIntGE(wc_PubKeyPemToDer(cert_buf, (int)cert_sz, cert_der,
(int)cert_dersz), 0);
if (cert_der != NULL) {
Expand Down
6 changes: 3 additions & 3 deletions wolfcrypt/src/asn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24322,7 +24322,7 @@ int wc_KeyPemToDer(const unsigned char* pem, int pemSz,

WOLFSSL_ENTER("wc_KeyPemToDer");

if (pem == NULL || (buff != NULL && buffSz <= 0)) {
if (pem == NULL || (buff != NULL && buffSz <= 0) || pemSz <= 0) {
WOLFSSL_MSG("Bad pem der args");
return BAD_FUNC_ARG;
}
Expand Down Expand Up @@ -24373,7 +24373,7 @@ int wc_CertPemToDer(const unsigned char* pem, int pemSz,

WOLFSSL_ENTER("wc_CertPemToDer");

if (pem == NULL || buff == NULL || buffSz <= 0) {
if (pem == NULL || buff == NULL || buffSz <= 0 || pemSz <= 0) {
WOLFSSL_MSG("Bad pem der args");
return BAD_FUNC_ARG;
}
Expand Down Expand Up @@ -24420,7 +24420,7 @@ int wc_PubKeyPemToDer(const unsigned char* pem, int pemSz,

WOLFSSL_ENTER("wc_PubKeyPemToDer");

if (pem == NULL || (buff != NULL && buffSz <= 0)) {
if (pem == NULL || (buff != NULL && buffSz <= 0) || pemSz <= 0) {
WOLFSSL_MSG("Bad pem der args");
return BAD_FUNC_ARG;
}
Expand Down
9 changes: 9 additions & 0 deletions wolfcrypt/src/ecc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7666,6 +7666,11 @@ int wc_ecc_gen_deterministic_k(const byte* hash, word32 hashSz,
/* 3.2 c. Set K = 0x00 0x00 ... */
XMEMSET(K, 0x00, KSz);

#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Add("wc_ecc_gen_deterministic_k K", K, KSz);
Comment on lines +7669 to +7670
Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] Size mismatch between wc_MemZero_Add and wc_MemZero_Check for K and V buffers
Category: API contract violations

The new wc_MemZero_Add calls at lines 7669-7670 register K and V with sizes KSz and VSz respectively, but the corresponding wc_MemZero_Check calls at lines 7826-7827 use WC_MAX_DIGEST_SIZE. If KSz or VSz differ from WC_MAX_DIGEST_SIZE (which they will when the hash digest size is smaller than the maximum), the wc_MemZero_Check call passes a different size than what was registered with wc_MemZero_Add. Depending on how the wc_MemZero_Check framework matches entries (by pointer and size), this mismatch could cause the check to fail to find the registered entry or to check the wrong byte range, leading to either false-positive assertions in debug builds or silently skipping the zero-verification.

/* Lines 7669-7670 (Add) */
    wc_MemZero_Add("wc_ecc_gen_deterministic_k K", K, KSz);
    wc_MemZero_Add("wc_ecc_gen_deterministic_k V", V, VSz);

/* Lines 7815-7816 (ForceZero) */
    ForceZero(K, WC_MAX_DIGEST_SIZE);
    ForceZero(V, WC_MAX_DIGEST_SIZE);

/* Lines 7826-7827 (Check) */
    wc_MemZero_Check(K, WC_MAX_DIGEST_SIZE);
    wc_MemZero_Check(V, WC_MAX_DIGEST_SIZE);

Recommendation: Use consistent sizes across wc_MemZero_Add, ForceZero, and wc_MemZero_Check. Either change the Add calls to use WC_MAX_DIGEST_SIZE (since ForceZero zeroes that many bytes and the arrays are that large), or change the ForceZero and Check calls to use KSz/VSz. Using WC_MAX_DIGEST_SIZE everywhere is likely the safest approach since it zeroes and verifies the entire stack buffer:

wc_MemZero_Add("wc_ecc_gen_deterministic_k K", K, WC_MAX_DIGEST_SIZE);
wc_MemZero_Add("wc_ecc_gen_deterministic_k V", V, WC_MAX_DIGEST_SIZE);

wc_MemZero_Add("wc_ecc_gen_deterministic_k V", V, VSz);
#endif

if (ret == 0) {
ret = mp_init(z1); /* always init z1 and free z1 */
}
Expand Down Expand Up @@ -7808,6 +7813,8 @@ int wc_ecc_gen_deterministic_k(const byte* hash, word32 hashSz,
}

ForceZero(x, MAX_ECC_BYTES);
ForceZero(K, WC_MAX_DIGEST_SIZE);
ForceZero(V, WC_MAX_DIGEST_SIZE);
#ifdef WOLFSSL_SMALL_STACK
XFREE(z1, heap, DYNAMIC_TYPE_ECC_BUFFER);
XFREE(x, heap, DYNAMIC_TYPE_PRIVATE_KEY);
Expand All @@ -7816,6 +7823,8 @@ int wc_ecc_gen_deterministic_k(const byte* hash, word32 hashSz,
XFREE(h1, heap, DYNAMIC_TYPE_DIGEST);
#elif defined(WOLFSSL_CHECK_MEM_ZERO)
wc_MemZero_Check(x, MAX_ECC_BYTES);
wc_MemZero_Check(K, WC_MAX_DIGEST_SIZE);
wc_MemZero_Check(V, WC_MAX_DIGEST_SIZE);
#endif

return ret;
Expand Down
10 changes: 10 additions & 0 deletions wolfcrypt/src/pkcs12.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1976,6 +1976,7 @@ static int wc_PKCS12_create_key_bag(WC_PKCS12* pkcs12, WC_RNG* rng,
word32 sz;
word32 i;
word32 tmpSz;
word32 tmpAllocSz;
int ret;

/* get max size for shrouded key */
Expand Down Expand Up @@ -2014,6 +2015,7 @@ static int wc_PKCS12_create_key_bag(WC_PKCS12* pkcs12, WC_RNG* rng,
}

/* shroud key */
tmpAllocSz = length;
tmp = (byte*)XMALLOC(length, heap, DYNAMIC_TYPE_TMP_BUFFER);
if (tmp == NULL) {
return MEMORY_E;
Expand All @@ -2022,11 +2024,13 @@ static int wc_PKCS12_create_key_bag(WC_PKCS12* pkcs12, WC_RNG* rng,
ret = wc_PKCS12_shroud_key(pkcs12, rng, tmp, &length, key, keySz,
algo, pass, passSz, iter);
if (ret < 0) {
ForceZero(tmp, tmpAllocSz);
XFREE(tmp, heap, DYNAMIC_TYPE_TMP_BUFFER);
return ret;
}
length = (word32)ret;
XMEMCPY(out + idx, tmp, (size_t)length);
ForceZero(tmp, tmpAllocSz);
XFREE(tmp, heap, DYNAMIC_TYPE_TMP_BUFFER);
totalSz += length;

Expand Down Expand Up @@ -2357,6 +2361,7 @@ static byte* PKCS12_create_key_content(WC_PKCS12* pkcs12, int nidKey,
{
byte* keyBuf;
word32 keyBufSz = 0;
word32 keyBufAllocSz = 0;
byte* keyCi = NULL;
word32 tmpSz;
int ret;
Expand Down Expand Up @@ -2406,6 +2411,7 @@ static byte* PKCS12_create_key_content(WC_PKCS12* pkcs12, int nidKey,

/* account for sequence around bag */
keyBufSz += MAX_SEQ_SZ;
keyBufAllocSz = keyBufSz;
keyBuf = (byte*)XMALLOC(keyBufSz, heap, DYNAMIC_TYPE_TMP_BUFFER);
if (keyBuf == NULL) {
WOLFSSL_MSG("Memory error creating keyBuf buffer");
Expand All @@ -2415,6 +2421,7 @@ static byte* PKCS12_create_key_content(WC_PKCS12* pkcs12, int nidKey,
ret = wc_PKCS12_create_key_bag(pkcs12, rng, keyBuf + MAX_SEQ_SZ, &keyBufSz,
key, keySz, algo, iter, pass, (int)passSz);
if (ret < 0) {
ForceZero(keyBuf, keyBufAllocSz);
XFREE(keyBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
WOLFSSL_MSG("Error creating key bag");
return NULL;
Expand All @@ -2437,18 +2444,21 @@ static byte* PKCS12_create_key_content(WC_PKCS12* pkcs12, int nidKey,
ret = wc_PKCS12_encrypt_content(pkcs12, rng, NULL, keyCiSz,
NULL, keyBufSz, algo, pass, (int)passSz, iter, WC_PKCS12_DATA);
if (ret != WC_NO_ERR_TRACE(LENGTH_ONLY_E)) {
ForceZero(keyBuf, keyBufAllocSz);
XFREE(keyBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
WOLFSSL_MSG("Error getting key encrypt content size");
return NULL;
}
keyCi = (byte*)XMALLOC(*keyCiSz, heap, DYNAMIC_TYPE_TMP_BUFFER);
if (keyCi == NULL) {
ForceZero(keyBuf, keyBufAllocSz);
XFREE(keyBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
return NULL;
}

ret = wc_PKCS12_encrypt_content(pkcs12, rng, keyCi, keyCiSz,
keyBuf, keyBufSz, algo, pass, (int)passSz, iter, WC_PKCS12_DATA);
ForceZero(keyBuf, keyBufAllocSz);
XFREE(keyBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
if (ret < 0 ) {
XFREE(keyCi, heap, DYNAMIC_TYPE_TMP_BUFFER);
Expand Down
Loading