Skip to content

Commit a010825

Browse files
committed
Address review comments on Fenrir zeroization fixes
Two follow-ups raised by Copilot review on PR #10247: src/pk_rsa.c: Make derAllocSz a word32 instead of int and only assign it after a successful XMALLOC, so the cleanup path can never call ForceZero with a wrapped-around size derived from a negative derSz. src/pk.c: Capture allocSz at the XMALLOC call site (and clear it back to 0 on allocation failure) so the relationship between the buffer allocation and the recorded size is explicit and cannot drift if the surrounding control flow changes.
1 parent 1e04092 commit a010825

2 files changed

Lines changed: 9 additions & 9 deletions

File tree

src/pk.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7150,16 +7150,14 @@ static int pem_write_mem_pkcs8privatekey(byte** pem, int* pemSz,
71507150
*pemSz += 54;
71517151
}
71527152

7153+
allocSz = (word32)*pemSz;
71537154
/* Allocate enough memory to hold PEM encoded encrypted key. */
7154-
*pem = (byte*)XMALLOC((size_t)*pemSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
7155+
*pem = (byte*)XMALLOC((size_t)allocSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
71557156
if (*pem == NULL) {
7157+
allocSz = 0;
71567158
res = 0;
71577159
}
71587160
else {
7159-
/* Remember the allocation size before *pemSz is updated to the
7160-
* actual PEM output length, so we can zero any unused tail that
7161-
* held the DER staging area. */
7162-
allocSz = (word32)*pemSz;
71637161
/* Use end of PEM buffer for key data. */
71647162
key = *pem + *pemSz - keySz;
71657163
}

src/pk_rsa.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -782,7 +782,7 @@ static int wolfSSL_RSA_To_Der_ex(WOLFSSL_RSA* rsa, byte** outBuf, int publicKey,
782782
{
783783
int ret = 1;
784784
int derSz = 0;
785-
int derAllocSz = 0;
785+
word32 derAllocSz = 0;
786786
byte* derBuf = NULL;
787787

788788
WOLFSSL_ENTER("wolfSSL_RSA_To_Der");
@@ -824,7 +824,6 @@ static int wolfSSL_RSA_To_Der_ex(WOLFSSL_RSA* rsa, byte** outBuf, int publicKey,
824824
}
825825
}
826826

827-
derAllocSz = derSz;
828827
if ((ret == 1) && (outBuf != NULL)) {
829828
derBuf = *outBuf;
830829
if (derBuf == NULL) {
@@ -835,6 +834,9 @@ static int wolfSSL_RSA_To_Der_ex(WOLFSSL_RSA* rsa, byte** outBuf, int publicKey,
835834
WOLFSSL_ERROR_MSG("Memory allocation failed");
836835
ret = MEMORY_ERROR;
837836
}
837+
else {
838+
derAllocSz = (word32)derSz;
839+
}
838840
}
839841
}
840842
if ((ret == 1) && (outBuf != NULL)) {
@@ -868,8 +870,8 @@ static int wolfSSL_RSA_To_Der_ex(WOLFSSL_RSA* rsa, byte** outBuf, int publicKey,
868870

869871
if ((outBuf != NULL) && (*outBuf != derBuf)) {
870872
/* Not returning buffer, needs to be disposed of. */
871-
if ((derBuf != NULL) && (publicKey == 0)) {
872-
ForceZero(derBuf, (word32)derAllocSz);
873+
if ((derBuf != NULL) && (publicKey == 0) && (derAllocSz > 0)) {
874+
ForceZero(derBuf, derAllocSz);
873875
}
874876
XFREE(derBuf, heap, DYNAMIC_TYPE_TMP_BUFFER);
875877
}

0 commit comments

Comments
 (0)