Skip to content

Commit 2e39b45

Browse files
committed
address copilot, fenrir, and internal review
- Copilot: privkey double-free — Fixed: added privkey = NULL after mid-function free at line 454 - Copilot: ForceZero NULL guard in OCSP — Fixed: added if (signerKeyDer != NULL && signerKeyDerSz > 0) guard - Copilot: ForceZero on key buffers in GenChimeraCertSign — Fixed: added ForceZero on caKeyBuf, altCaKeyBuf, serverKeyBuf before XFREE - Fenrir: pkey vs privkey — No change needed: pkey is a borrowed ref from X509_get0_pubkey, not owned by caller. Removing the free was correct. - Fenrir: Missing ForceZero on heap key buffers — Same as Copilot #3, addressed above - CI: switch-enum errors — Fixed: removed inner #ifdef guards on enum cases that always exist, added SM3 under #ifdef WOLFSSL_SM3, removed WC_HASH_TYPE_MAX (duplicate value) - CI: heap-buffer-overflow in strstr — Fixed: allocate inBufSz + 1 and null-terminate for XSTRSTR safety - CI: heap-use-after-free — Fixed by the privkey NULL fix above
1 parent e1184f1 commit 2e39b45

3 files changed

Lines changed: 13 additions & 9 deletions

File tree

src/ocsp/clu_ocsp.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,8 @@ static int ocspResponder(OcspResponderConfig* config)
884884
freeIndexEntries(indexEntries);
885885
XFREE(caCertDer, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
886886
XFREE(signerCertDer, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
887-
wolfCLU_ForceZero(signerKeyDer, signerKeyDerSz);
887+
if (signerKeyDer != NULL && signerKeyDerSz > 0)
888+
wolfCLU_ForceZero(signerKeyDer, signerKeyDerSz);
888889
XFREE(signerKeyDer, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
889890
XFREE(caSubject, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
890891

src/x509/clu_cert_setup.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,12 +326,13 @@ int wolfCLU_certSetup(int argc, char** argv)
326326
ret = WOLFCLU_FATAL_ERROR;
327327
}
328328
else {
329-
inBufRaw = (byte*)XMALLOC(inBufSz, HEAP_HINT,
329+
inBufRaw = (byte*)XMALLOC(inBufSz + 1, HEAP_HINT,
330330
DYNAMIC_TYPE_TMP_BUFFER);
331331
if (inBufRaw == NULL) {
332332
ret = WOLFCLU_FATAL_ERROR;
333333
}
334334
else {
335+
inBufRaw[inBufSz] = '\0';
335336
if (wolfSSL_BIO_read(inMem, inBufRaw, inBufSz) != inBufSz) {
336337
wolfCLU_LogError("Failed to read input.");
337338
ret = WOLFCLU_FATAL_ERROR;
@@ -452,6 +453,7 @@ int wolfCLU_certSetup(int argc, char** argv)
452453
}
453454
}
454455
wolfSSL_EVP_PKEY_free(privkey);
456+
privkey = NULL;
455457
}
456458

457459
/* try to open output file if set */

src/x509/clu_x509_sign.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -925,6 +925,12 @@ int wolfCLU_GenChimeraCertSign(WOLFSSL_BIO *bioCaKey, WOLFSSL_BIO *bioAltCaKey,
925925
wolfSSL_BIO_free(out);
926926
}
927927

928+
if (caKeyBuf != NULL)
929+
wolfCLU_ForceZero(caKeyBuf, LARGE_TEMP_SZ);
930+
if (altCaKeyBuf != NULL)
931+
wolfCLU_ForceZero(altCaKeyBuf, LARGE_TEMP_SZ);
932+
if (serverKeyBuf != NULL)
933+
wolfCLU_ForceZero(serverKeyBuf, LARGE_TEMP_SZ);
928934
XFREE(caKeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
929935
XFREE(altCaKeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
930936
XFREE(sapkiBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
@@ -1312,17 +1318,12 @@ int wolfCLU_CertSign(WOLFCLU_CERT_SIGN* csign, WOLFSSL_X509* x509)
13121318
case WC_HASH_TYPE_BLAKE2S:
13131319

13141320
#if LIBWOLFSSL_VERSION_HEX > 0x05001000
1315-
#ifndef WOLFSSL_NOSHA512_224
13161321
case WC_HASH_TYPE_SHA512_224:
1317-
#endif
1318-
#ifndef WOLFSSL_NOSHA512_256
13191322
case WC_HASH_TYPE_SHA512_256:
1320-
#endif
1321-
#ifdef WOLFSSL_SHAKE128
13221323
case WC_HASH_TYPE_SHAKE128:
1323-
#endif
1324-
#ifdef WOLFSSL_SHAKE256
13251324
case WC_HASH_TYPE_SHAKE256:
1325+
#ifdef WOLFSSL_SM3
1326+
case WC_HASH_TYPE_SM3:
13261327
#endif
13271328
#endif
13281329
default:

0 commit comments

Comments
 (0)