Skip to content

Bound fwTPM child blob sizes before copying from the response - #595

Open
aidangarske wants to merge 1 commit into
wolfSSL:masterfrom
aidangarske:coverity-913517-tainted-child-blobs
Open

Bound fwTPM child blob sizes before copying from the response#595
aidangarske wants to merge 1 commit into
wolfSSL:masterfrom
aidangarske:coverity-913517-tainted-child-blobs

Conversation

@aidangarske

Copy link
Copy Markdown
Member

CID 913517

Copilot AI lite review requested due to automatic review settings September 9, 2026 00:51
@aidangarske aidangarske self-assigned this Sep 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new bounds checks still don’t assert that the response buffer (rspSize) actually contains the claimed blob lengths before reading/copying, which can cause reads past the valid response payload.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens fwTPM unit test parsing of TPM Create responses by adding upper-bound assertions before copying child key blobs, aiming to prevent oversized blob lengths from overflowing the caller-provided buffers.

Changes:

  • Added a new AssertIntLE() helper macro for “less-than-or-equal” integer assertions.
  • Added bounds checks to ensure outPrivate.size and outPublic (size-prefix + data) fit into the caller buffers before memcpy() in CreateChildBlobs().
File summaries
File Description
tests/fwtpm_unit_tests.c Adds assertions to bound parsed child blob sizes before copying them out of the TPM response in fwTPM unit tests.
Review details

Suppressed comments (1)

tests/fwtpm_unit_tests.c:10548

  • CreateChildBlobs() reads/copies outPublic using *pubSz from the response, but does not assert the response length (rspSize) covers the size field and the subsequent 2+pubSz bytes. This can read beyond the valid response and copy uninitialized/stale bytes from gRsp into pub.
    *pubSz = GetU16BE(gRsp + pos);
    AssertIntGT(*pubSz, 0);
    AssertIntLE(2 + *pubSz, sizeof(TPM2B_PUBLIC));    /* fits caller's buffer */
    memcpy(pub, gRsp + pos, 2 + *pubSz);              /* keep the size prefix */
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/fwtpm_unit_tests.c
Comment on lines 10540 to 10544
pos = TPM2_HEADER_SIZE + 4; /* skip parameterSize */
*privSz = GetU16BE(gRsp + pos); pos += 2;
AssertIntGT(*privSz, 0);
AssertIntLE(*privSz, sizeof(TPM2B_PRIVATE)); /* fits caller's buffer */
memcpy(priv, gRsp + pos, *privSz); pos += *privSz;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants