Harden PCR policy bounds checks

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);

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 */
     }
 

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:");
 }
 

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