From 7d50ca1eef82a808dda8f6cec52fcd6d7c9ab1f1 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Wed, 19 Aug 2026 12:36:19 -0700 Subject: [PATCH] Harden PCR policy bounds checks --- src/tpm2_wrap.c | 12 +++++++----- tests/fwtpm_unit_tests.c | 37 +++++++++++++++++++++++++++---------- tests/unit_tests.c | 12 ++++++++++++ wolftpm/tpm2_wrap.h | 3 ++- 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/tpm2_wrap.c b/src/tpm2_wrap.c index 4828e612..8798d80f 100644 --- a/src/tpm2_wrap.c +++ b/src/tpm2_wrap.c @@ -11074,7 +11074,7 @@ int wolfTPM2_PolicyPCRMake(TPM_ALG_ID pcrAlg, byte* pcrArray, word32 pcrArraySz, TPML_PCR_SELECTION pcr; if (digest == NULL || digestSz == NULL || pcrArray == NULL || - pcrArraySz == 0) { + pcrArraySz == 0 || (pcrDigest == NULL && pcrDigestSz > 0)) { return BAD_FUNC_ARG; } @@ -11087,12 +11087,14 @@ int wolfTPM2_PolicyPCRMake(TPM_ALG_ID pcrAlg, byte* pcrArray, word32 pcrArraySz, TPM2_Packet_AppendPCR(&packet, &pcr); /* Copy the pcrDigest to the end of buffer */ - if (packet.pos < 0 || (word32)packet.pos > (word32)sizeof(buf) || - pcrDigestSz > (word32)sizeof(buf) - (word32)packet.pos) { + if (packet.overflow || packet.pos > packet.size || + pcrDigestSz > (word32)(packet.size - packet.pos)) { return BUFFER_E; } - XMEMCPY(buf + packet.pos, pcrDigest, pcrDigestSz); - packet.pos += pcrDigestSz; + if (pcrDigestSz > 0) { + XMEMCPY(buf + packet.pos, pcrDigest, pcrDigestSz); + packet.pos += (int)pcrDigestSz; + } rc = wolfTPM2_PolicyHash(pcrAlg, digest, digestSz, TPM_CC_PolicyPCR, buf, packet.pos); diff --git a/tests/fwtpm_unit_tests.c b/tests/fwtpm_unit_tests.c index dd60a64a..73050203 100644 --- a/tests/fwtpm_unit_tests.c +++ b/tests/fwtpm_unit_tests.c @@ -9494,7 +9494,13 @@ static void test_fwtpm_pcr_properties_capability(void) rspSize = 0; FWTPM_ProcessCommand(&ctx, gCmd, pos, gRsp, &rspSize, 0); AssertIntEQ(GetRspRC(gRsp), TPM_RC_SUCCESS); - AssertTrue(rspSize > 0 && rspSize <= (int)sizeof(gRsp)); + if (rspSize < TPM2_HEADER_SIZE + 9 || + rspSize > (int)sizeof(gRsp)) { + AssertTrue(rspSize >= TPM2_HEADER_SIZE + 9 && + rspSize <= (int)sizeof(gRsp)); + FWTPM_Cleanup(&ctx); + return; + } /* header(10) + moreData(1) + capability(4) + count(4) + properties */ p = TPM2_HEADER_SIZE + 1; @@ -9502,28 +9508,39 @@ static void test_fwtpm_pcr_properties_capability(void) AssertIntEQ(cap, TPM_CAP_PCR_PROPERTIES); count = GetU32BE(gRsp + p); p += 4; AssertIntGT((int)count, 0); - AssertTrue((int)count <= 32); /* bounded by the 32 records requested above */ + if (count == 0 || count > 32) { + AssertTrue(count > 0 && count <= 32); + FWTPM_Cleanup(&ctx); + return; + } for (i = 0; i < (int)count; i++) { - AssertTrue(p + 5 <= rspSize); /* room for tag(4)+size(1) */ + if (p > rspSize || rspSize - p < 5) { + AssertTrue(p <= rspSize && rspSize - p >= 5); + FWTPM_Cleanup(&ctx); + return; + } tag = GetU32BE(gRsp + p); p += 4; wireSz = gRsp[p]; p += 1; + if (wireSz <= 0 || p > rspSize || wireSz > rspSize - p) { + AssertTrue(wireSz > 0 && p <= rspSize && + wireSz <= rspSize - p); + FWTPM_Cleanup(&ctx); + return; + } selSz = (wireSz > 8) ? 8 : wireSz; - AssertTrue(selSz > 0); /* select bytes present; keeps p inside gRsp */ - AssertTrue(p + selSz <= rspSize); if (tag == TPM_PT_PCR_RESET_L0) { - memcpy(resetL0, gRsp + p, selSz); gotResetL0 = 1; + XMEMCPY(resetL0, gRsp + p, selSz); gotResetL0 = 1; } else if (tag == TPM_PT_PCR_RESET_L4) { - memcpy(resetL4, gRsp + p, selSz); gotResetL4 = 1; + XMEMCPY(resetL4, gRsp + p, selSz); gotResetL4 = 1; } else if (tag == TPM_PT_PCR_EXTEND_L0) { - memcpy(extendL0, gRsp + p, selSz); gotExtendL0 = 1; + XMEMCPY(extendL0, gRsp + p, selSz); gotExtendL0 = 1; } else if (tag == TPM_PT_PCR_DRTM_RESET) { - memcpy(drtm, gRsp + p, selSz); gotDrtm = 1; + XMEMCPY(drtm, gRsp + p, selSz); gotDrtm = 1; } - AssertTrue(p + wireSz <= rspSize); /* full record present on the wire */ p += wireSz; /* advance past the select bytes */ } diff --git a/tests/unit_tests.c b/tests/unit_tests.c index 46fcc8b1..a0d47f77 100644 --- a/tests/unit_tests.c +++ b/tests/unit_tests.c @@ -1164,6 +1164,11 @@ static void test_TPM2_PCRSel(void) static void test_TPM2_Policy_NULL_Args(void) { int rc; + #ifndef WOLFTPM2_NO_WOLFCRYPT + byte pcrArray[1] = {TPM2_DEMO_PCR_INDEX}; + byte digest[TPM_SHA256_DIGEST_SIZE]; + word32 digestSz = (word32)sizeof(digest); + #endif /* Test NULL input handling for policy commands */ rc = TPM2_PolicyPhysicalPresence(NULL); @@ -1175,6 +1180,13 @@ static void test_TPM2_Policy_NULL_Args(void) rc = TPM2_PolicyPassword(NULL); AssertIntEQ(rc, BAD_FUNC_ARG); + #ifndef WOLFTPM2_NO_WOLFCRYPT + /* A nonzero PCR digest size requires backing digest data. */ + rc = wolfTPM2_PolicyPCRMake(TPM_ALG_SHA256, pcrArray, + (word32)sizeof(pcrArray), NULL, 1, digest, &digestSz); + AssertIntEQ(rc, BAD_FUNC_ARG); + #endif + printf("Test TPM2: %-40s Passed\n", "Policy NULL Args:"); } diff --git a/wolftpm/tpm2_wrap.h b/wolftpm/tpm2_wrap.h index 681c261c..f1b82d6b 100644 --- a/wolftpm/tpm2_wrap.h +++ b/wolftpm/tpm2_wrap.h @@ -4904,7 +4904,8 @@ WOLFTPM_API int wolfTPM2_PolicyRefMake(TPM_ALG_ID pcrAlg, byte* digest, word32* \param pcrAlg the hash algorithm to use with pcr policy \param pcrArray optional array of pcrs to be used when creating the tpm object \param pcrArraySz length of the pcrArray - \param pcrDigest digest for the PCR(s) collected (can get using wolfTPM2_PCRGetDigest) + \param pcrDigest digest for the PCR(s) collected (can get using + wolfTPM2_PCRGetDigest); required when pcrDigestSz is nonzero \param pcrDigestSz size of the PCR digest \param digest input/out digest \param digestSz input/out digest size