From 791309f66720148fb5b1d7f96296baaf61308c2e Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Mon, 5 Oct 2026 12:38:05 -0700 Subject: [PATCH 1/2] Reject PCR property capability counts larger than the response --- src/tpm2.c | 79 +++++++++++++++++++++++++------------------ tests/unit_tests.c | 62 +++++++++++++++++++++++++++++++++ wolftpm/tpm2_packet.h | 2 ++ 3 files changed, 111 insertions(+), 32 deletions(-) diff --git a/src/tpm2.c b/src/tpm2.c index 76bb28dc..4621492c 100644 --- a/src/tpm2.c +++ b/src/tpm2.c @@ -1432,6 +1432,51 @@ int TPM2_ParseSpdmSessionInfo(TPM2_Packet* packet, } #endif /* WOLFTPM_SPDM */ +/* Parse a TPML_TAGGED_PCR_PROPERTY capability list, rejecting a count that + * claims more entries than the response holds. */ +int TPM2_ParsePcrProperties(TPM2_Packet* packet, + TPML_TAGGED_PCR_PROPERTY* pcrProp) +{ + UINT32 wireCount = 0; + UINT32 i; + UINT32 tag; + UINT8 wireSizeofSelect; + + if (packet == NULL || pcrProp == NULL) + return BAD_FUNC_ARG; + + TPM2_Packet_ParseU32(packet, &wireCount); + pcrProp->count = wireCount; + if (pcrProp->count > MAX_PCR_PROPERTIES) + pcrProp->count = MAX_PCR_PROPERTIES; + for (i = 0; i < wireCount && !packet->overflow; i++) { + TPM2_Packet_ParseU32(packet, &tag); + TPM2_Packet_ParseU8(packet, &wireSizeofSelect); + if (i < pcrProp->count) { + TPMS_TAGGED_PCR_SELECT* sel = &pcrProp->pcrProperty[i]; + sel->tag = tag; + sel->sizeofSelect = wireSizeofSelect; + if (sel->sizeofSelect > PCR_SELECT_MAX) + sel->sizeofSelect = PCR_SELECT_MAX; + TPM2_Packet_ParseBytes(packet, sel->pcrSelect, + sel->sizeofSelect); + if (wireSizeofSelect > sel->sizeofSelect) { + TPM2_Packet_ParseBytes(packet, NULL, + wireSizeofSelect - sel->sizeofSelect); + } + } + else { + /* Skip entries beyond array capacity */ + TPM2_Packet_ParseBytes(packet, NULL, wireSizeofSelect); + } + } + if (packet->overflow) { + pcrProp->count = 0; + return TPM_RC_SIZE; + } + return TPM_RC_SUCCESS; +} + TPM_RC TPM2_GetCapability(GetCapability_In* in, GetCapability_Out* out) { TPM_RC rc; @@ -1535,38 +1580,8 @@ TPM_RC TPM2_GetCapability(GetCapability_In* in, GetCapability_Out* out) } case TPM_CAP_PCR_PROPERTIES: { - TPML_TAGGED_PCR_PROPERTY* pcrProp = - &out->capabilityData.data.pcrProperties; - UINT32 wireCount; - UINT32 tag; - UINT8 wireSizeofSelect; - TPM2_Packet_ParseU32(&packet, &wireCount); - pcrProp->count = wireCount; - if (pcrProp->count > MAX_PCR_PROPERTIES) - pcrProp->count = MAX_PCR_PROPERTIES; - for (i=0; i<(int)wireCount; i++) { - TPM2_Packet_ParseU32(&packet, &tag); - TPM2_Packet_ParseU8(&packet, &wireSizeofSelect); - if (i < (int)pcrProp->count) { - TPMS_TAGGED_PCR_SELECT* sel = - &pcrProp->pcrProperty[i]; - sel->tag = tag; - sel->sizeofSelect = wireSizeofSelect; - if (sel->sizeofSelect > PCR_SELECT_MAX) - sel->sizeofSelect = PCR_SELECT_MAX; - TPM2_Packet_ParseBytes(&packet, sel->pcrSelect, - sel->sizeofSelect); - if (wireSizeofSelect > sel->sizeofSelect) { - TPM2_Packet_ParseBytes(&packet, NULL, - wireSizeofSelect - sel->sizeofSelect); - } - } - else { - /* Skip entries beyond array capacity */ - TPM2_Packet_ParseBytes(&packet, NULL, - wireSizeofSelect); - } - } + rc = TPM2_ParsePcrProperties(&packet, + &out->capabilityData.data.pcrProperties); break; } case TPM_CAP_ECC_CURVES: diff --git a/tests/unit_tests.c b/tests/unit_tests.c index a0b03b09..2875857f 100644 --- a/tests/unit_tests.c +++ b/tests/unit_tests.c @@ -4664,6 +4664,67 @@ static void test_TPM2_Packet_ParseU16BufStrict(void) printf("Test TPM Wrapper:\tParseU16BufStrict:\t\tPassed\n"); } +static void test_TPM2_ParsePcrProperties_Count(void) +{ + static const byte twoProps[] = { + 0x00, 0x00, 0x00, 0x02, + 0x00, 0x00, 0x00, 0x01, 0x03, 0x01, 0x02, 0x03, + 0x00, 0x00, 0x00, 0x02, 0x03, 0x04, 0x05, 0x06 + }; + static const UINT32 badCounts[] = { 3, 0x80000000UL, 0xFFFFFFFFUL }; + byte manyProps[4 + (MAX_PCR_PROPERTIES + 1) * 5]; + byte buf[sizeof(twoProps)]; + TPM2_Packet packet; + TPML_TAGGED_PCR_PROPERTY props; + word32 i; + int rc; + + XMEMSET(&packet, 0, sizeof(packet)); + XMEMSET(&props, 0, sizeof(props)); + packet.buf = (byte*)twoProps; + packet.size = (int)sizeof(twoProps); + rc = TPM2_ParsePcrProperties(&packet, &props); + AssertIntEQ(rc, TPM_RC_SUCCESS); + AssertIntEQ(props.count, 2); + AssertIntEQ(props.pcrProperty[1].tag, 2); + AssertIntEQ(props.pcrProperty[1].sizeofSelect, 3); + AssertIntEQ(props.pcrProperty[1].pcrSelect[2], 0x06); + AssertIntEQ(packet.pos, packet.size); + + /* Entries past the local array are skipped, not rejected */ + XMEMSET(manyProps, 0, sizeof(manyProps)); + manyProps[0] = (byte)((word32)(MAX_PCR_PROPERTIES + 1) >> 24); + manyProps[1] = (byte)((word32)(MAX_PCR_PROPERTIES + 1) >> 16); + manyProps[2] = (byte)((word32)(MAX_PCR_PROPERTIES + 1) >> 8); + manyProps[3] = (byte)(MAX_PCR_PROPERTIES + 1); + XMEMSET(&packet, 0, sizeof(packet)); + XMEMSET(&props, 0, sizeof(props)); + packet.buf = manyProps; + packet.size = (int)sizeof(manyProps); + rc = TPM2_ParsePcrProperties(&packet, &props); + AssertIntEQ(rc, TPM_RC_SUCCESS); + AssertIntEQ(props.count, MAX_PCR_PROPERTIES); + AssertIntEQ(packet.pos, packet.size); + + /* A count claiming more entries than the response holds is rejected */ + for (i = 0; i < (word32)(sizeof(badCounts) / sizeof(badCounts[0])); i++) { + XMEMCPY(buf, twoProps, sizeof(buf)); + buf[0] = (byte)(badCounts[i] >> 24); + buf[1] = (byte)(badCounts[i] >> 16); + buf[2] = (byte)(badCounts[i] >> 8); + buf[3] = (byte)badCounts[i]; + XMEMSET(&packet, 0, sizeof(packet)); + XMEMSET(&props, 0, sizeof(props)); + packet.buf = buf; + packet.size = (int)sizeof(buf); + rc = TPM2_ParsePcrProperties(&packet, &props); + AssertIntEQ(rc, TPM_RC_SIZE); + AssertIntEQ(props.count, 0); + } + + printf("Test TPM Wrapper:\tPCR properties count:\t\tPassed\n"); +} + /* TPM2_Packet_ParsePoint must resync to outerStart + point->size so a * malformed wire blob with inner x.size / y.size disagreement can't * desynchronize subsequent fields. */ @@ -10088,6 +10149,7 @@ int unit_tests(int argc, char *argv[]) test_TPM2_ParseSpdmSessionInfo_Truncated(); #endif test_TPM2_Packet_ParseU16BufStrict(); + test_TPM2_ParsePcrProperties_Count(); test_wolfTPM2_Init(); test_wolfTPM2_OpenExisting(); test_wolfTPM2_GetCapabilities(); diff --git a/wolftpm/tpm2_packet.h b/wolftpm/tpm2_packet.h index bedc632c..665ffcc8 100644 --- a/wolftpm/tpm2_packet.h +++ b/wolftpm/tpm2_packet.h @@ -166,6 +166,8 @@ WOLFTPM_LOCAL void TPM2_Packet_ParseBytes(TPM2_Packet* packet, byte* buf, int si WOLFTPM_TEST_API int TPM2_ParseSpdmSessionInfo(TPM2_Packet* packet, TPML_SPDM_SESSION_INFO* sessInfo); #endif /* WOLFTPM_SPDM */ +WOLFTPM_TEST_API int TPM2_ParsePcrProperties(TPM2_Packet* packet, + TPML_TAGGED_PCR_PROPERTY* pcrProp); /*! \brief Parse a UINT16-prefixed buffer from a TPM2 packet. Reads a 16-bit size followed by that many bytes into buf, clamped to maxBufSz. From 20c271fb65f6eec455a08b3fd0b2e436ebd0fd87 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Mon, 5 Oct 2026 14:10:02 -0700 Subject: [PATCH 2/2] Use single-byte PCR selects in the PCR properties count test --- tests/unit_tests.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit_tests.c b/tests/unit_tests.c index 2875857f..8fd5e3c8 100644 --- a/tests/unit_tests.c +++ b/tests/unit_tests.c @@ -4668,8 +4668,8 @@ static void test_TPM2_ParsePcrProperties_Count(void) { static const byte twoProps[] = { 0x00, 0x00, 0x00, 0x02, - 0x00, 0x00, 0x00, 0x01, 0x03, 0x01, 0x02, 0x03, - 0x00, 0x00, 0x00, 0x02, 0x03, 0x04, 0x05, 0x06 + 0x00, 0x00, 0x00, 0x01, 0x01, 0x03, + 0x00, 0x00, 0x00, 0x02, 0x01, 0x06 }; static const UINT32 badCounts[] = { 3, 0x80000000UL, 0xFFFFFFFFUL }; byte manyProps[4 + (MAX_PCR_PROPERTIES + 1) * 5]; @@ -4687,8 +4687,8 @@ static void test_TPM2_ParsePcrProperties_Count(void) AssertIntEQ(rc, TPM_RC_SUCCESS); AssertIntEQ(props.count, 2); AssertIntEQ(props.pcrProperty[1].tag, 2); - AssertIntEQ(props.pcrProperty[1].sizeofSelect, 3); - AssertIntEQ(props.pcrProperty[1].pcrSelect[2], 0x06); + AssertIntEQ(props.pcrProperty[1].sizeofSelect, 1); + AssertIntEQ(props.pcrProperty[1].pcrSelect[0], 0x06); AssertIntEQ(packet.pos, packet.size); /* Entries past the local array are skipped, not rejected */