Skip to content

Commit b8f8a10

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 b8f8a10

3 files changed

Lines changed: 42 additions & 61 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: 9 additions & 3 deletions
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 */
@@ -749,10 +751,14 @@ int wolfCLU_certSetup(int argc, char** argv)
749751
}
750752
else {
751753
if (wolfSSL_EVP_PKEY_id(pkey) == EVP_PKEY_RSA) {
752-
const WOLFSSL_BIGNUM *num;
754+
const WOLFSSL_BIGNUM *num = NULL;
755+
WOLFSSL_RSA *rsa;
753756
char *hex;
754757

755-
wolfSSL_RSA_get0_key(EVP_PKEY_get0_RSA(pkey), &num, NULL, NULL);
758+
rsa = EVP_PKEY_get0_RSA(pkey);
759+
if (rsa != NULL) {
760+
wolfSSL_RSA_get0_key(rsa, &num, NULL, NULL);
761+
}
756762
hex = wolfSSL_BN_bn2hex(num);
757763

758764
if (hex != NULL) {

src/x509/clu_x509_sign.c

Lines changed: 31 additions & 57 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);
@@ -1270,64 +1276,32 @@ int wolfCLU_CertSign(WOLFCLU_CERT_SIGN* csign, WOLFSSL_X509* x509)
12701276

12711277
/* set hash for signature */
12721278
if (ret == WOLFCLU_SUCCESS) {
1273-
switch (csign->hashType) {
1274-
case WC_HASH_TYPE_MD5:
1275-
#ifndef NO_MD5
1276-
md = wolfSSL_EVP_md5();
1277-
#else
1278-
wolfCLU_LogError("MD5 not compiled in");
1279-
ret = WOLFCLU_FATAL_ERROR;
1280-
#endif
1281-
break;
1282-
1283-
case WC_HASH_TYPE_SHA:
1284-
md = wolfSSL_EVP_sha1();
1285-
break;
1286-
1287-
case WC_HASH_TYPE_SHA224:
1288-
md = wolfSSL_EVP_sha224();
1289-
break;
1290-
1291-
case WC_HASH_TYPE_SHA256:
1292-
md = wolfSSL_EVP_sha256();
1293-
break;
1294-
1295-
case WC_HASH_TYPE_SHA384:
1296-
md = wolfSSL_EVP_sha384();
1297-
break;
1298-
1299-
case WC_HASH_TYPE_SHA512:
1300-
md = wolfSSL_EVP_sha512();
1301-
break;
1302-
1303-
case WC_HASH_TYPE_NONE:
1304-
case WC_HASH_TYPE_MD2:
1305-
case WC_HASH_TYPE_MD4:
1306-
case WC_HASH_TYPE_MD5_SHA:
1307-
case WC_HASH_TYPE_SHA3_224:
1308-
case WC_HASH_TYPE_SHA3_256:
1309-
case WC_HASH_TYPE_SHA3_384:
1310-
case WC_HASH_TYPE_SHA3_512:
1311-
case WC_HASH_TYPE_BLAKE2B:
1312-
case WC_HASH_TYPE_BLAKE2S:
1313-
1314-
#if LIBWOLFSSL_VERSION_HEX > 0x05001000
1315-
#ifndef WOLFSSL_NOSHA512_224
1316-
case WC_HASH_TYPE_SHA512_224:
1317-
#endif
1318-
#ifndef WOLFSSL_NOSHA512_256
1319-
case WC_HASH_TYPE_SHA512_256:
1320-
#endif
1321-
#ifdef WOLFSSL_SHAKE128
1322-
case WC_HASH_TYPE_SHAKE128:
1323-
#endif
1324-
#ifdef WOLFSSL_SHAKE256
1325-
case WC_HASH_TYPE_SHAKE256:
1279+
if (csign->hashType == WC_HASH_TYPE_MD5) {
1280+
#ifndef NO_MD5
1281+
md = wolfSSL_EVP_md5();
1282+
#else
1283+
wolfCLU_LogError("MD5 not compiled in");
1284+
ret = WOLFCLU_FATAL_ERROR;
13261285
#endif
1327-
#endif
1328-
default:
1329-
wolfCLU_LogError("Unsupported hash type");
1330-
ret = WOLFCLU_FATAL_ERROR;
1286+
}
1287+
else if (csign->hashType == WC_HASH_TYPE_SHA) {
1288+
md = wolfSSL_EVP_sha1();
1289+
}
1290+
else if (csign->hashType == WC_HASH_TYPE_SHA224) {
1291+
md = wolfSSL_EVP_sha224();
1292+
}
1293+
else if (csign->hashType == WC_HASH_TYPE_SHA256) {
1294+
md = wolfSSL_EVP_sha256();
1295+
}
1296+
else if (csign->hashType == WC_HASH_TYPE_SHA384) {
1297+
md = wolfSSL_EVP_sha384();
1298+
}
1299+
else if (csign->hashType == WC_HASH_TYPE_SHA512) {
1300+
md = wolfSSL_EVP_sha512();
1301+
}
1302+
else {
1303+
wolfCLU_LogError("Unsupported hash type");
1304+
ret = WOLFCLU_FATAL_ERROR;
13311305
}
13321306
}
13331307

0 commit comments

Comments
 (0)