Skip to content

Fenrir fixes #10247

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
julek-wolfssl wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Fenrir fixes #10247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
cb49532
Zeroize DER buffer in der_to_enc_pem_alloc before free
julek-wolfssl Apr 17, 2026
a05dd20
Zeroize DSA DER buffer in PEM write before free
julek-wolfssl Apr 17, 2026
dfd37f4
Zeroize EC DER buffer in PEM write error path
julek-wolfssl Apr 17, 2026
4c8adc5
Zeroize RSA DER buffer in To_Der error path
julek-wolfssl Apr 17, 2026
9790719
Zeroize sniffer watch key buffer before free
julek-wolfssl Apr 17, 2026
fa3feb7
Zeroize static ephemeral key buffer before free
julek-wolfssl Apr 17, 2026
a82828b
Zeroize RSA DER buffer in CTX_use_RSAPrivateKey before free
julek-wolfssl Apr 17, 2026
9925f90
Zeroize RSA DER buffer in d2i_RSAPrivateKey_bio before free
julek-wolfssl Apr 17, 2026
87e5c62
Zeroize EC DER buffer in i2d_ECPrivateKey error path
julek-wolfssl Apr 17, 2026
00fff0f
Zeroize PKCS#8 DER staging area in PEM write helper
julek-wolfssl Apr 17, 2026
1e04092
Only zero unused tail of PKCS#8 PEM buffer
julek-wolfssl Apr 17, 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
31 changes: 30 additions & 1 deletion src/pk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,7 @@ static int der_to_enc_pem_alloc(unsigned char* der, int derSz,
byte* tmp = NULL;
byte* cipherInfo = NULL;
int pemSz = 0;
int derAllocSz = derSz;
int hashType = WC_HASH_TYPE_NONE;
#if !defined(NO_MD5)
hashType = WC_MD5;
Expand Down Expand Up @@ -515,6 +516,7 @@ static int der_to_enc_pem_alloc(unsigned char* der, int derSz,
}
else {
der = tmpBuf;
derAllocSz = derSz + blockSz;

/* Encrypt DER inline. */
ret = EncryptDerKey(der, &derSz, cipher, passwd, passwdSz,
Expand Down Expand Up @@ -562,7 +564,10 @@ static int der_to_enc_pem_alloc(unsigned char* der, int derSz,

XFREE(tmp, NULL, DYNAMIC_TYPE_KEY);
XFREE(cipherInfo, NULL, DYNAMIC_TYPE_STRING);
XFREE(der, heap, DYNAMIC_TYPE_TMP_BUFFER);
if (der != NULL) {
ForceZero(der, (word32)derAllocSz);
XFREE(der, heap, DYNAMIC_TYPE_TMP_BUFFER);
}

return ret;
}
Expand Down Expand Up @@ -2104,6 +2109,7 @@ int wolfSSL_PEM_write_mem_DSAPrivateKey(WOLFSSL_DSA* dsa,
derSz = wc_DsaKeyToDer((DsaKey*)dsa->internal, derBuf, (word32)der_max_len);
if (derSz < 0) {
WOLFSSL_MSG("wc_DsaKeyToDer failed");
ForceZero(derBuf, (word32)der_max_len);
XFREE(derBuf, NULL, DYNAMIC_TYPE_DER);
return 0;
}
Expand All @@ -2116,6 +2122,7 @@ int wolfSSL_PEM_write_mem_DSAPrivateKey(WOLFSSL_DSA* dsa,
&cipherInfo, der_max_len, WC_MD5);
if (ret != 1) {
WOLFSSL_MSG("EncryptDerKey failed");
ForceZero(derBuf, (word32)der_max_len);
XFREE(derBuf, NULL, DYNAMIC_TYPE_DER);
return ret;
}
Expand All @@ -2131,6 +2138,7 @@ int wolfSSL_PEM_write_mem_DSAPrivateKey(WOLFSSL_DSA* dsa,
tmp = (byte*)XMALLOC((size_t)*pLen, NULL, DYNAMIC_TYPE_PEM);
if (tmp == NULL) {
WOLFSSL_MSG("malloc failed");
ForceZero(derBuf, (word32)der_max_len);
XFREE(derBuf, NULL, DYNAMIC_TYPE_DER);
XFREE(cipherInfo, NULL, DYNAMIC_TYPE_STRING);
return 0;
Expand All @@ -2141,11 +2149,13 @@ int wolfSSL_PEM_write_mem_DSAPrivateKey(WOLFSSL_DSA* dsa,
type);
if (*pLen <= 0) {
WOLFSSL_MSG("wc_DerToPemEx failed");
ForceZero(derBuf, (word32)der_max_len);
XFREE(derBuf, NULL, DYNAMIC_TYPE_DER);
XFREE(tmp, NULL, DYNAMIC_TYPE_PEM);
XFREE(cipherInfo, NULL, DYNAMIC_TYPE_STRING);
return 0;
}
ForceZero(derBuf, (word32)der_max_len);
XFREE(derBuf, NULL, DYNAMIC_TYPE_DER);
XFREE(cipherInfo, NULL, DYNAMIC_TYPE_STRING);

Expand Down Expand Up @@ -7107,6 +7117,7 @@ static int pem_write_mem_pkcs8privatekey(byte** pem, int* pemSz,
char password[NAME_SZ];
byte* key = NULL;
word32 keySz = 0;
word32 allocSz = 0;
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

The new wiping logic depends on allocSz being captured before *pemSz is updated, but allocSz is only set in one branch. If *pem is allocated and a failure occurs before that branch executes, allocSz may remain 0 and the buffer won’t be cleared even though it may contain sensitive staging data. Consider setting allocSz immediately after the PEM buffer allocation succeeds (at the point where the allocation size is definitively known) so all subsequent failure paths can reliably wipe the allocation.

Copilot uses AI. Check for mistakes.
int type = PKCS8_PRIVATEKEY_TYPE;

/* Validate parameters. */
Expand Down Expand Up @@ -7145,6 +7156,10 @@ static int pem_write_mem_pkcs8privatekey(byte** pem, int* pemSz,
res = 0;
}
else {
/* Remember the allocation size before *pemSz is updated to the
* actual PEM output length, so we can zero any unused tail that
* held the DER staging area. */
allocSz = (word32)*pemSz;
Comment on lines +7159 to +7162
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

The new wiping logic depends on allocSz being captured before *pemSz is updated, but allocSz is only set in one branch. If *pem is allocated and a failure occurs before that branch executes, allocSz may remain 0 and the buffer won’t be cleared even though it may contain sensitive staging data. Consider setting allocSz immediately after the PEM buffer allocation succeeds (at the point where the allocation size is definitively known) so all subsequent failure paths can reliably wipe the allocation.

Copilot uses AI. Check for mistakes.
/* Use end of PEM buffer for key data. */
key = *pem + *pemSz - keySz;
}
Expand Down Expand Up @@ -7198,6 +7213,20 @@ static int pem_write_mem_pkcs8privatekey(byte** pem, int* pemSz,
}
}

/* Zero any remnants of the DER staging area that persist after PEM
* conversion so plaintext private key material is not left in freed heap
* memory. On success, only the bytes past the actual PEM output need
* clearing; on failure, the whole buffer is zeroed since its state is
* indeterminate. */
if (*pem != NULL) {
if (res == 1 && (word32)*pemSz < allocSz) {
ForceZero(*pem + *pemSz, allocSz - (word32)*pemSz);
}
else if (res != 1) {
ForceZero(*pem, allocSz);
}
}
Comment on lines +7216 to +7228
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

The new wiping logic depends on allocSz being captured before *pemSz is updated, but allocSz is only set in one branch. If *pem is allocated and a failure occurs before that branch executes, allocSz may remain 0 and the buffer won’t be cleared even though it may contain sensitive staging data. Consider setting allocSz immediately after the PEM buffer allocation succeeds (at the point where the allocation size is definitively known) so all subsequent failure paths can reliably wipe the allocation.

Copilot uses AI. Check for mistakes.

/* Return appropriate return code. */
return (res == 0) ? 0 : ret;

Expand Down
4 changes: 4 additions & 0 deletions src/pk_ec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3524,6 +3524,9 @@ int wolfSSL_i2d_ECPrivateKey(const WOLFSSL_EC_KEY *key, unsigned char **out)

/* Dispose of any allocated buffer on error. */
if (err && (*out == buf)) {
if (buf != NULL) {
ForceZero(buf, len);
}
XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
*out = NULL;
}
Expand Down Expand Up @@ -4095,6 +4098,7 @@ int wolfSSL_PEM_write_mem_ECPrivateKey(WOLFSSL_EC_KEY* ec,
derSz = wc_EccKeyToDer((ecc_key*)ec->internal, derBuf, der_max_len);
if (derSz < 0) {
WOLFSSL_MSG("wc_EccKeyToDer failed");
ForceZero(derBuf, der_max_len);
XFREE(derBuf, NULL, DYNAMIC_TYPE_DER);
ret = 0;
}
Expand Down
8 changes: 8 additions & 0 deletions src/pk_rsa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,9 @@ WOLFSSL_RSA* wolfSSL_d2i_RSAPrivateKey_bio(WOLFSSL_BIO *bio, WOLFSSL_RSA **out)
key = NULL;
}
/* Dispose of allocated data. */
if (der != NULL) {
ForceZero(der, (word32)derLen);
}
XFREE(der, bio ? bio->heap : NULL, DYNAMIC_TYPE_TMP_BUFFER);
return key;
}
Expand Down Expand Up @@ -779,6 +782,7 @@ static int wolfSSL_RSA_To_Der_ex(WOLFSSL_RSA* rsa, byte** outBuf, int publicKey,
{
int ret = 1;
int derSz = 0;
int derAllocSz = 0;
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

derAllocSz is an int that is cast to word32 when passed to ForceZero(). If derSz can ever be negative (or otherwise invalid) on an error path, the cast can wrap to a very large value and cause an out-of-bounds wipe. Make derAllocSz an unsigned size type (e.g., word32/size_t) and only assign/call ForceZero() when the size is known to be > 0 (and ideally set it to the actual allocation size used for derBuf, not just the DER length).

Suggested change
int derAllocSz = 0;
word32 derAllocSz = 0;

Copilot uses AI. Check for mistakes.
byte* derBuf = NULL;

WOLFSSL_ENTER("wolfSSL_RSA_To_Der");
Expand Down Expand Up @@ -820,6 +824,7 @@ static int wolfSSL_RSA_To_Der_ex(WOLFSSL_RSA* rsa, byte** outBuf, int publicKey,
}
}

derAllocSz = derSz;
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

derAllocSz is an int that is cast to word32 when passed to ForceZero(). If derSz can ever be negative (or otherwise invalid) on an error path, the cast can wrap to a very large value and cause an out-of-bounds wipe. Make derAllocSz an unsigned size type (e.g., word32/size_t) and only assign/call ForceZero() when the size is known to be > 0 (and ideally set it to the actual allocation size used for derBuf, not just the DER length).

Copilot uses AI. Check for mistakes.
if ((ret == 1) && (outBuf != NULL)) {
derBuf = *outBuf;
if (derBuf == NULL) {
Expand Down Expand Up @@ -863,6 +868,9 @@ static int wolfSSL_RSA_To_Der_ex(WOLFSSL_RSA* rsa, byte** outBuf, int publicKey,

if ((outBuf != NULL) && (*outBuf != derBuf)) {
/* Not returning buffer, needs to be disposed of. */
if ((derBuf != NULL) && (publicKey == 0)) {
ForceZero(derBuf, (word32)derAllocSz);
}
Comment on lines +871 to +873
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

derAllocSz is an int that is cast to word32 when passed to ForceZero(). If derSz can ever be negative (or otherwise invalid) on an error path, the cast can wrap to a very large value and cause an out-of-bounds wipe. Make derAllocSz an unsigned size type (e.g., word32/size_t) and only assign/call ForceZero() when the size is known to be > 0 (and ideally set it to the actual allocation size used for derBuf, not just the DER length).

Copilot uses AI. Check for mistakes.
XFREE(derBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
}
WOLFSSL_LEAVE("wolfSSL_RSA_To_Der", ret);
Expand Down
4 changes: 4 additions & 0 deletions src/sniffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7242,12 +7242,16 @@ int ssl_SetWatchKey_file(void* vSniffer, const char* keyFile, int keyType,
ret = LoadKeyFile(&keyBuf, &keyBufSz, keyFile, 0, keyType, password);
if (ret < 0) {
SetError(KEY_FILE_STR, error, NULL, 0);
if (keyBuf != NULL) {
ForceZero(keyBuf, keyBufSz);
}
XFREE(keyBuf, NULL, DYNAMIC_TYPE_X509);
return WOLFSSL_FATAL_ERROR;
}

ret = ssl_SetWatchKey_buffer(vSniffer, keyBuf, keyBufSz, FILETYPE_DER,
error);
ForceZero(keyBuf, keyBufSz);
XFREE(keyBuf, NULL, DYNAMIC_TYPE_X509);

return ret;
Expand Down
1 change: 1 addition & 0 deletions src/ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18645,6 +18645,7 @@ static int SetStaticEphemeralKey(WOLFSSL_CTX* ctx,
#ifndef NO_FILESYSTEM
/* done with keyFile buffer */
if (keyFile && keyBuf) {
ForceZero(keyBuf, keySz);
XFREE(keyBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
}
#endif
Expand Down
3 changes: 3 additions & 0 deletions src/ssl_load.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5365,6 +5365,9 @@ int wolfSSL_CTX_use_RSAPrivateKey(WOLFSSL_CTX* ctx, WOLFSSL_RSA* rsa)
}

/* Dispos of dynamically allocated data. */
if (der != NULL) {
ForceZero(der, (word32)derSize);
}
XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
return ret;
}
Expand Down
Loading