Skip to content

Commit 0b2a1c3

Browse files
aidangarskeaidangarske
authored
Merge pull request #483 from dgarske/coverity_20260417
Coverity fixes for new fwtpm code
2 parents a7c3395 + 34ae62e commit 0b2a1c3

6 files changed

Lines changed: 83 additions & 34 deletions

File tree

‎.github/workflows/fwtpm-test.yml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,13 +210,28 @@ jobs:
210210
WOLFSSL_PATH: ./wolfssl
211211
run: make check
212212

213+
- name: Print test-suite.log on failure
214+
if: ${{ failure() && !matrix.build_only }}
215+
run: |
216+
if [ -f test-suite.log ]; then
217+
echo "=== test-suite.log ==="
218+
cat test-suite.log
219+
fi
220+
for f in tests/*.log; do
221+
[ -f "$f" ] || continue
222+
echo "=== $f ==="
223+
cat "$f"
224+
done
225+
213226
- name: Upload failure logs
214227
if: ${{ failure() && !matrix.build_only }}
215228
uses: actions/upload-artifact@v4
216229
with:
217230
name: fwtpm-logs-${{ matrix.name }}
218231
path: |
219232
/tmp/fwtpm_check_*.log
233+
test-suite.log
234+
tests/*.log
220235
retention-days: 5
221236

222237
# ----------------------------------------------------------------

‎examples/pcr/policy.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ int TPM2_PCR_Policy_Test(void* userCtx, int argc, char *argv[])
6161
TPM_ALG_ID paramEncAlg = TPM_ALG_NULL;
6262
WOLFTPM2_SESSION tpmSession;
6363
int i;
64+
int hexRet;
6465
byte digest[32];
6566
word32 digestLen = 0;
6667
union {
@@ -108,7 +109,13 @@ int TPM2_PCR_Policy_Test(void* userCtx, int argc, char *argv[])
108109
usage();
109110
return 0;
110111
}
111-
digestLen = hexToByte(digestStr, digest, digestLen / 2);
112+
hexRet = hexToByte(digestStr, digest, digestLen / 2);
113+
if (hexRet < 0) {
114+
printf("Invalid hex digest string\n");
115+
usage();
116+
return 0;
117+
}
118+
digestLen = (word32)hexRet;
112119
}
113120
else if (argv[argc-1][0] != '-') {
114121
/* TODO: Allow selection of multiple PCR's SHA-1 or SHA2-256 */

‎src/fwtpm/fwtpm_command.c‎

Lines changed: 54 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3587,7 +3587,7 @@ static TPM_RC FwCmd_Create(FWTPM_CTX* ctx, TPM2_Packet* cmd,
35873587

35883588
if (rc == 0 && sensDataSize > 0) {
35893589
/* Use caller-supplied key material */
3590-
if (sensDataSize > (UINT16)FWTPM_MAX_PRIVKEY_DER) {
3590+
if (sensDataSize > (UINT16)FWTPM_MAX_DATA_BUF) {
35913591
rc = TPM_RC_SIZE;
35923592
}
35933593
if (rc == 0) {
@@ -4090,7 +4090,7 @@ static TPM_RC FwCmd_LoadExternal(FWTPM_CTX* ctx, TPM2_Packet* cmd,
40904090
if (rc == 0 && inPrivSize > 0 && sensitiveType == TPM_ALG_SYMCIPHER &&
40914091
qSz > 0) {
40924092
/* For SYMCIPHER, qBuf contains the raw AES key bytes */
4093-
if (qSz > (UINT16)FWTPM_MAX_PRIVKEY_DER) {
4093+
if (qSz > (UINT16)FWTPM_MAX_DER_SIG_BUF) {
40944094
rc = TPM_RC_SIZE;
40954095
}
40964096
if (rc == 0) {
@@ -5459,7 +5459,7 @@ static TPM_RC FwCmd_CreateLoaded(FWTPM_CTX* ctx, TPM2_Packet* cmd,
54595459
}
54605460

54615461
if (rc == 0 && sensDataSize > 0) {
5462-
if (sensDataSize > (UINT16)FWTPM_MAX_PRIVKEY_DER) {
5462+
if (sensDataSize > (UINT16)FWTPM_MAX_DATA_BUF) {
54635463
rc = TPM_RC_SIZE;
54645464
}
54655465
if (rc == 0) {
@@ -5496,7 +5496,7 @@ static TPM_RC FwCmd_CreateLoaded(FWTPM_CTX* ctx, TPM2_Packet* cmd,
54965496
}
54975497

54985498
if (rc == 0 && sensDataSize > 0) {
5499-
if (sensDataSize > (UINT16)FWTPM_MAX_PRIVKEY_DER) {
5499+
if (sensDataSize > (UINT16)FWTPM_MAX_DATA_BUF) {
55005500
rc = TPM_RC_SIZE;
55015501
}
55025502
if (rc == 0) {
@@ -7339,7 +7339,10 @@ static TPM_RC FwCmd_StartAuthSession(FWTPM_CTX* ctx, TPM2_Packet* cmd,
73397339
XMEMCPY(&bindAuth, &bindObj->authValue, sizeof(TPM2B_AUTH));
73407340
}
73417341
}
7342-
if (bindAuth.size > 0) {
7342+
if (bindAuth.size > sizeof(bindAuth.buffer)) {
7343+
rc = TPM_RC_FAILURE;
7344+
}
7345+
if (rc == 0 && bindAuth.size > 0) {
73437346
if (keyInSz + bindAuth.size <= (int)sizeof(keyIn)) {
73447347
XMEMCPY(keyIn + keyInSz, bindAuth.buffer, bindAuth.size);
73457348
keyInSz += bindAuth.size;
@@ -11108,31 +11111,46 @@ static TPM_RC FwCmd_Quote(FWTPM_CTX* ctx, TPM2_Packet* cmd,
1110811111
wcH = FwGetWcHashType(pcrHashAlg);
1110911112
dSz = TPM2_GetHashDigestSize(pcrHashAlg);
1111011113
if (wcH != WC_HASH_TYPE_NONE && dSz > 0) {
11111-
wc_HashInit(hashCtx, wcH);
11112-
for (s = 0; s < numSel; s++) {
11113-
int bank = FwGetPcrBankIndex(selections[s].hashAlg);
11114-
int bankDSz = TPM2_GetHashDigestSize(
11115-
selections[s].hashAlg);
11116-
UINT32 j;
11117-
if (bank < 0 || bankDSz == 0)
11118-
continue;
11119-
for (j = 0; j < selections[s].sizeOfSelect; j++) {
11120-
int pcr;
11121-
for (pcr = 0; pcr < 8; pcr++) {
11122-
if (selections[s].pcrSelect[j] & (1 << pcr)) {
11123-
int pcrIdx = j * 8 + pcr;
11124-
if (pcrIdx < IMPLEMENTATION_PCR) {
11125-
wc_HashUpdate(hashCtx, wcH,
11126-
ctx->pcrDigest[pcrIdx][bank],
11127-
bankDSz);
11114+
if (wc_HashInit(hashCtx, wcH) != 0) {
11115+
rc = TPM_RC_FAILURE;
11116+
}
11117+
if (rc == 0) {
11118+
for (s = 0; s < numSel && rc == 0; s++) {
11119+
int bank = FwGetPcrBankIndex(selections[s].hashAlg);
11120+
int bankDSz = TPM2_GetHashDigestSize(
11121+
selections[s].hashAlg);
11122+
UINT32 j;
11123+
if (bank < 0 || bankDSz == 0)
11124+
continue;
11125+
for (j = 0; j < selections[s].sizeOfSelect &&
11126+
rc == 0; j++) {
11127+
int pcr;
11128+
for (pcr = 0; pcr < 8; pcr++) {
11129+
if (selections[s].pcrSelect[j] & (1 << pcr)) {
11130+
int pcrIdx = j * 8 + pcr;
11131+
if (pcrIdx < IMPLEMENTATION_PCR) {
11132+
if (wc_HashUpdate(hashCtx, wcH,
11133+
ctx->pcrDigest[pcrIdx][bank],
11134+
bankDSz) != 0) {
11135+
rc = TPM_RC_FAILURE;
11136+
break;
11137+
}
11138+
}
1112811139
}
1112911140
}
1113011141
}
1113111142
}
11143+
if (rc == 0) {
11144+
if (wc_HashFinal(hashCtx, wcH,
11145+
pcrDigestBuf) != 0) {
11146+
rc = TPM_RC_FAILURE;
11147+
}
11148+
else {
11149+
pcrDigestSz = dSz;
11150+
}
11151+
}
11152+
wc_HashFree(hashCtx, wcH);
1113211153
}
11133-
wc_HashFinal(hashCtx, wcH, pcrDigestBuf);
11134-
wc_HashFree(hashCtx, wcH);
11135-
pcrDigestSz = dSz;
1113611154
}
1113711155
TPM2_Packet_AppendU16(&attestPkt, (UINT16)pcrDigestSz);
1113811156
TPM2_Packet_AppendBytes(&attestPkt, pcrDigestBuf, pcrDigestSz);
@@ -11547,8 +11565,13 @@ static TPM_RC FwCmd_NV_Certify(FWTPM_CTX* ctx, TPM2_Packet* cmd,
1154711565
dSz = TPM2_GetHashDigestSize(nv->nvPublic.nameAlg);
1154811566
if (wcH != WC_HASH_TYPE_NONE && dSz > 0) {
1154911567
FwStoreU16BE(nvName.name, nv->nvPublic.nameAlg);
11550-
wc_Hash(wcH, nvPubBuf, tmpPkt.pos, nvName.name + 2, dSz);
11551-
nvName.size = (UINT16)(2 + dSz);
11568+
if (wc_Hash(wcH, nvPubBuf, tmpPkt.pos,
11569+
nvName.name + 2, dSz) == 0) {
11570+
nvName.size = (UINT16)(2 + dSz);
11571+
}
11572+
else {
11573+
rc = TPM_RC_FAILURE;
11574+
}
1155211575
}
1155311576
}
1155411577

@@ -11738,7 +11761,7 @@ static TPM_RC FwCmd_MakeCredential(FWTPM_CTX* ctx, TPM2_Packet* cmd,
1173811761
/* patch blob size */
1173911762
blobSz = rsp->pos - blobStart;
1174011763
if (blobSz < 0 || blobSz > 0xFFFF ||
11741-
encSeedSz < 0 || encSeedSz > 0xFFFF) {
11764+
encSeedSz < 0 || encSeedSz > (int)FWTPM_MAX_PUB_BUF) {
1174211765
rc = TPM_RC_SIZE;
1174311766
}
1174411767
if (rc == 0) {
@@ -11914,6 +11937,9 @@ static TPM_RC FwCmd_ActivateCredential(FWTPM_CTX* ctx, TPM2_Packet* cmd,
1191411937
objName->name, objName->size,
1191511938
credOut, (int)sizeof(credOut), &credSz);
1191611939
}
11940+
if (rc == 0 && credSz > (UINT16)sizeof(credOut)) {
11941+
rc = TPM_RC_SIZE;
11942+
}
1191711943

1191811944
/* Build response: TPM2B_DIGEST */
1191911945
if (rc == 0) {
@@ -12849,7 +12875,6 @@ int FWTPM_ProcessCommand(FWTPM_CTX* ctx,
1284912875
cmdAuthCnt++;
1285012876
}
1285112877

12852-
cmdPkt.pos = authEnd;
1285312878
cpStart = authEnd; /* cpBuffer starts after auth area */
1285412879
}
1285512880
}

‎src/fwtpm/fwtpm_main.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ int main(int argc, char* argv[])
108108
/* Delete NV state file if --clear was requested */
109109
if (clearNv) {
110110
printf("Clearing NV state file: %s\n", FWTPM_NV_FILE);
111-
remove(FWTPM_NV_FILE);
111+
(void)remove(FWTPM_NV_FILE);
112112
}
113113

114114
/* Initialize fwTPM */

‎tests/fwtpm_check.sh‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,9 +429,11 @@ elif [ -x "$TPM2_TOOLS_SCRIPT" ]; then
429429
sleep 0.3
430430
fi
431431
rm -f "$BUILD_DIR/fwtpm_nv.bin"
432+
echo "--- fwtpm_server restart for tpm2-tools ---" \
433+
>> /tmp/fwtpm_check_$$.log
432434
"$FWTPM_SERVER" --port "$FWTPM_PORT" \
433435
--platform-port "$FWTPM_PLAT_PORT" \
434-
> /tmp/fwtpm_check_$$.log 2>&1 &
436+
>> /tmp/fwtpm_check_$$.log 2>&1 &
435437
echo $! > "$PID_FILE"
436438
if ! wait_for_port "$FWTPM_PORT" 500; then
437439
echo "FAIL: fwtpm_server restart failed before tpm2-tools tests"

‎tests/fwtpm_unit_tests.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2095,7 +2095,7 @@ int fwtpm_unit_tests(int argc, char *argv[])
20952095
printf("fwTPM Unit Tests\n");
20962096

20972097
/* Remove stale NV state to ensure clean test runs */
2098-
remove(FWTPM_NV_FILE);
2098+
(void)remove(FWTPM_NV_FILE);
20992099

21002100
/* Lifecycle */
21012101
test_fwtpm_init_cleanup();
@@ -2142,7 +2142,7 @@ int fwtpm_unit_tests(int argc, char *argv[])
21422142
* state, so remove FWTPM_NV_FILE afterwards. */
21432143
test_fwtpm_clock_sethal();
21442144
test_fwtpm_nv_sethal_mock();
2145-
remove(FWTPM_NV_FILE);
2145+
(void)remove(FWTPM_NV_FILE);
21462146

21472147
/* Key operations */
21482148
#if !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN)

0 commit comments

Comments
 (0)