Coverity fixes for new fwtpm code

[dgarske]

Coverity Fixes (CID 909296–909315)

Address 18 Coverity findings: 4 bug fixes, 2 dead code cleanups, 5 defense-in-depth improvements, and 7 reviewed-no-action.

Bug Fixes

CID	File	Description
909299, 909313	src/fwtpm/fwtpm_command.c	OVERRUN: sensData buffer size check used the wrong constant (FWTPM_MAX_PRIVKEY_DER = 1280 vs. actual buffer size FWTPM_MAX_DATA_BUF = 1024). Corrected in FwCmd_Create and FwCmd_CreateLoaded (3 locations). Defense-in-depth — upstream parser already bounds the size.
909307	src/fwtpm/fwtpm_command.c	CHECKED_RETURN: Added error check for wc_Hash() in FwCmd_NV_Certify.
909302	src/fwtpm/fwtpm_command.c	CHECKED_RETURN: Added error check for wc_HashInit() in FwCmd_Quote and guarded subsequent hash operations.
909314	examples/pcr/policy.c	NEGATIVE_RETURNS: hexToByte() returns int (may be -1) was assigned directly to word32. Added error check with early return.

Dead Code Cleanup

CID	File	Description
909312	src/fwtpm/fwtpm_command.c	UNUSED_VALUE: Removed dead cmdPkt.pos assignment in FWTPM_ProcessCommand (overwritten before use).
909303	src/fwtpm/fwtpm_command.c	DEADCODE: Corrected bound in FwCmd_LoadExternal SYMCIPHER path to match the actual qSz constraint (FWTPM_MAX_DER_SIG_BUF).

Defense-in-Depth

CID	File	Description
909315, 909310	src/fwtpm/fwtpm_main.c, tests/fwtpm_unit_tests.c	CHECKED_RETURN: Cast remove() to (void) — failure is intentional (file may not exist).
909306	src/fwtpm/fwtpm_command.c	OVERRUN: Tightened size validation in FwCmd_MakeCredential to use the actual buffer size (FWTPM_MAX_PUB_BUF).
909305	src/fwtpm/fwtpm_command.c	TAINTED_SCALAR: Added explicit bounds check on credSz after FwCredentialUnwrap() in FwCmd_ActivateCredential.
909301	src/fwtpm/fwtpm_command.c	OVERRUN: Added explicit bindAuth.size clamp in FwCmd_StartAuthSession.

Reviewed – No Action Required

7 additional findings (CID 909296, 909297, 909298, 909300, 909308, 909309, 909311) were reviewed and determined to be false positives or by-design behavior; no code changes required.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Addresses Coverity CID 909315 by tightening bounds checks and handling return values in the new fwTPM code paths.

Changes:

• Silence/handle flagged unchecked return values (e.g., remove, hashing APIs).
• Adjust buffer size limits to safer constants and add additional size guards.
• Improve CLI example robustness by validating hex parsing results.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
tests/fwtpm_unit_tests.c	Explicitly ignores remove() return to satisfy static analysis in tests.
src/fwtpm/fwtpm_main.c	Explicitly ignores remove() return when --clear is used.
src/fwtpm/fwtpm_command.c	Adds bounds checks and some crypto API return checking in several TPM command handlers.
examples/pcr/policy.c	Adds error handling for hexToByte() conversion failures.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[aidangarske]

Skoll Code Review

Scan type: review-security
Overall recommendation: REQUEST_CHANGES
Findings: 4 total — 1 posted, 3 skipped

Posted findings

• [High] Buffer overflow regression: sensDataSize check weakened for NO_RSA builds in FwCmd_Create/FwCmd_CreateLoaded — src/fwtpm/fwtpm_command.c:3590,5462,5499

Skipped findings

• [Low] Odd-length hex digest silently reinterpreted after hexToByte fix in policy.c
• [Info] FwCmd_Quote: wc_HashFree is intentionally skipped on wc_HashInit failure — matches existing pattern
• [Info] FwCmd_StartAuthSession bindAuth.size clamp — analysis confirms existing callers cannot exceed the buffer

Review generated by Skoll