Coverity fixes Harden TPM bounds and configuration paths

src/fwtpm/fwtpm_crypto.c

@@ -418,31 +418,36 @@ int FwAppendCreationHashAndTicket(FWTPM_CTX* ctx, TPM2_Packet* rsp,
 int FwGetWcCurveId(UINT16 tpmCurve)
 {
     int curveIdx;
-    int keyBits;
     int wcCurve;
 
     switch (tpmCurve) {
         case TPM_ECC_NIST_P256:
-            keyBits = 256;
+        #if ECC_MIN_KEY_SZ > 256
+            return -1;
+        #else
             wcCurve = ECC_SECP256R1;
             break;
+        #endif
         case TPM_ECC_NIST_P384:
-            keyBits = 384;
+        #if ECC_MIN_KEY_SZ > 384
+            return -1;
+        #else
             wcCurve = ECC_SECP384R1;
             break;
+        #endif
     #ifdef FWTPM_HAVE_ECC521
         case TPM_ECC_NIST_P521:
-            keyBits = 521;
+        #if ECC_MIN_KEY_SZ > 521
+            return -1;
+        #else
             wcCurve = ECC_SECP521R1;
             break;
+        #endif
     #endif
         default:
             return -1;
     }
 
-    if (keyBits < ECC_MIN_KEY_SZ) {
-        return -1;
-    }
     curveIdx = wc_ecc_get_curve_idx(wcCurve);
     if (curveIdx < 0 || wc_ecc_get_curve_params(curveIdx) == NULL) {
         return -1;

src/tpm2.c

@@ -88,6 +88,11 @@ static THREAD_LS_T TPM2_CTX* gActiveTPM;
 
 #define TPM2_LOCALITY_UNINITIALIZED (-1)
 
+#if !defined(WOLFTPM2_NO_WOLFCRYPT) && !defined(WOLFTPM_NO_LOCK) && \
+    !defined(SINGLE_THREADED) && !defined(WOLFSSL_MUTEX_INITIALIZER)
+#define TPM2_DYNAMIC_HW_LOCK
+#endif
+
 /******************************************************************************/
 /* --- Local Functions -- */
 /******************************************************************************/
@@ -1019,33 +1024,44 @@ TPM_RC TPM2_Init(TPM2_CTX* ctx, TPM2HalIoCb ioCb, void* userCtx)
 TPM_RC TPM2_Cleanup(TPM2_CTX* ctx)
 {
     TPM_RC rc;
-    int lockCtx = 1;
 #ifndef WOLFTPM2_NO_WOLFCRYPT
     int wolfCryptInit;
+    #ifdef TPM2_DYNAMIC_HW_LOCK
+    int lockCtx;
+    #endif
 #endif
 
     if (ctx == NULL)
         return BAD_FUNC_ARG;
 
 #ifndef WOLFTPM2_NO_WOLFCRYPT
     wolfCryptInit = (ctx->locality != TPM2_LOCALITY_UNINITIALIZED);
-    #if !defined(WOLFTPM_NO_LOCK) && !defined(SINGLE_THREADED) && \
-        !defined(WOLFSSL_MUTEX_INITIALIZER)
+    #ifdef TPM2_DYNAMIC_HW_LOCK
     lockCtx = wolfCryptInit;
     #endif
 #endif
 
     /* clear global */
-    if (lockCtx)
+    #ifdef TPM2_DYNAMIC_HW_LOCK
+    if (lockCtx) {
         rc = TPM2_AcquireLock(ctx);
-    else
+    }
+    else {
         rc = TPM_RC_SUCCESS;
+    }
+    #else
+    rc = TPM2_AcquireLock(ctx);
+    #endif
     if (rc == TPM_RC_SUCCESS) {
 
         if (TPM2_GetActiveCtx() == ctx) {
+        #ifdef TPM2_DYNAMIC_HW_LOCK
             if (lockCtx) {
                 TPM2_INTERNAL_CLEANUP(ctx);
             }
+        #else
+            TPM2_INTERNAL_CLEANUP(ctx);
+        #endif
             /* set non-active */
             TPM2_SetActiveCtx(NULL);
         }
@@ -1054,8 +1070,13 @@ TPM_RC TPM2_Cleanup(TPM2_CTX* ctx)
          * auth values, decrypted parameters) */
         TPM2_ForceZero(ctx->cmdBuf, sizeof(ctx->cmdBuf));
 
-        if (lockCtx)
+    #ifdef TPM2_DYNAMIC_HW_LOCK
+        if (lockCtx) {
             TPM2_ReleaseLock(ctx);
+        }
+    #else
+        TPM2_ReleaseLock(ctx);
+    #endif
     }
     else {
         TPM2_ForceZero(ctx->cmdBuf, sizeof(ctx->cmdBuf));

src/tpm2_wrap.c

@@ -2065,13 +2065,26 @@ int wolfTPM2_SetAuthHandle(WOLFTPM2_DEV* dev, int index,
         if (handle->policyAuth) {
             TPM2_AUTH_SESSION* session = &dev->session[index];
             int authDigestSz = TPM2_GetHashDigestSize(session->authHash);
+            word32 authSz;
+
+            /* Validate bounds before reading caller data or mutating the
+             * session so it isn't left inconsistent on BUFFER_E. */
+            if (authDigestSz <= 0 ||
+                (word32)authDigestSz >
+                    (word32)sizeof(session->auth.buffer) ||
+                handle->auth.size > (word32)sizeof(session->auth.buffer) -
+                    (word32)authDigestSz ||
+                handle->name.size > sizeof(session->name.name)) {
+                return BUFFER_E;
+            }
+            authSz = (word32)authDigestSz + handle->auth.size;
         #ifdef WOLFTPM_DEBUG_VERBOSE
             printf("Session %d: Edit (PolicyAuth)\n", index);
             printf("\tHandle 0x%x (not touching)\n", session->sessionHandle);
             printf("\tPolicyAuth %d->%d\n",
                 session->policyAuth, handle->policyAuth);
-            printf("\tAuth Sz %d -> %d\n", session->auth.size,
-                authDigestSz + handle->auth.size);
+            printf("\tAuth Sz %d -> %u\n", session->auth.size,
+                (unsigned int)authSz);
         #ifdef WOLFTPM_DEBUG_SECRETS
             TPM2_PrintBin(session->auth.buffer, session->auth.size);
             TPM2_PrintBin(handle->auth.buffer, handle->auth.size);
@@ -2080,22 +2093,13 @@ int wolfTPM2_SetAuthHandle(WOLFTPM2_DEV* dev, int index,
             TPM2_PrintBin(session->name.name, session->name.size);
             TPM2_PrintBin(handle->name.name, handle->name.size);
         #endif
-            /* Validate bounds before any session-state mutation so the
-             * session isn't left inconsistent on BUFFER_E. */
-            if (authDigestSz <= 0 ||
-                (handle->auth.size + authDigestSz) >
-                    (int)sizeof(session->auth.buffer) ||
-                handle->name.size > sizeof(session->name.name)) {
-                return BUFFER_E;
-            }
             session->policyAuth = handle->policyAuth;
-            session->auth.size = authDigestSz + handle->auth.size;
+            session->auth.size = (word16)authSz;
             XMEMMOVE(&session->auth.buffer[authDigestSz], handle->auth.buffer,
                 handle->auth.size);
-            if (session->auth.size < sizeof(session->auth.buffer)) {
-                TPM2_ForceZero(&session->auth.buffer[session->auth.size],
-                    (word32)sizeof(session->auth.buffer) -
-                        session->auth.size);
+            if (authSz < sizeof(session->auth.buffer)) {
+                TPM2_ForceZero(&session->auth.buffer[authSz],
+                    (word32)sizeof(session->auth.buffer) - authSz);
             }
             session->name.size = handle->name.size;
             XMEMCPY(session->name.name, handle->name.name, session->name.size);
@@ -11441,11 +11445,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.overflow || packet.pos > packet.size ||
-        pcrDigestSz > (word32)(packet.size - packet.pos)) {
+    if (packet.overflow || packet.pos > packet.size) {
         return BUFFER_E;
     }
     if (pcrDigestSz > 0) {
+        if (packet.pos >= packet.size ||
+            pcrDigestSz > (word32)(packet.size - packet.pos)) {
+            return BUFFER_E;
+        }
         XMEMCPY(buf + packet.pos, pcrDigest, pcrDigestSz);
         packet.pos += (int)pcrDigestSz;
     }
@@ -12384,8 +12391,7 @@ int wolfTPM2_ST33_GetFwUpgradeCommands(const WOLFTPM2_CAPS* caps,
             stdImpl);
     }
 #endif
-    return wolfTPM2_ST33_FwUpgradeCommands(
-        (caps != NULL) ? caps->fwVerMinor : 0, (caps != NULL),
+    return wolfTPM2_ST33_FwUpgradeCommands(caps->fwVerMinor, 1,
         manifestMajor, ccStart, ccData);
 }
 

tests/fwtpm_unit_tests.c

@@ -11176,8 +11176,8 @@ static void test_fwtpm_pcr_properties_capability(void)
         }
         tag = GetU32BE(gRsp + p); p += 4;
         wireSz = gRsp[p]; p += 1;
-        if (wireSz <= 0 || p > rspSize || wireSz > rspSize - p) {
-            AssertTrue(wireSz > 0 && p <= rspSize &&
+        if (wireSz <= 0 || p >= rspSize || wireSz > rspSize - p) {
+            AssertTrue(wireSz > 0 && p < rspSize &&
                 wireSz <= rspSize - p);
             FWTPM_Cleanup(&ctx);
             return;

tests/unit_tests.c

@@ -1604,11 +1604,14 @@ static void test_wolfTPM2_SetAuthHandle_PolicyAuthOffset(void)
     int rc;
     WOLFTPM2_DEV dev;
     WOLFTPM2_HANDLE handle;
+    TPM2_AUTH_SESSION sessionBefore;
     int authDigestSz;
+    int maxAuthSz;
     int i;
 
     XMEMSET(&dev, 0, sizeof(dev));
     XMEMSET(&handle, 0, sizeof(handle));
+    XMEMSET(&sessionBefore, 0, sizeof(sessionBefore));
 
     (void)wolfTPM2_Init(&dev, TPM2_IoCb, NULL);
 
@@ -1669,6 +1672,51 @@ static void test_wolfTPM2_SetAuthHandle_PolicyAuthOffset(void)
     AssertIntEQ(XMEMCMP(&dev.session[0].auth.buffer[authDigestSz],
         handle.auth.buffer, handle.auth.size), 0);
 
+    /* Verify the largest combined digest and authorization fits exactly. */
+    maxAuthSz = (int)sizeof(dev.session[0].auth.buffer) - authDigestSz;
+    AssertIntGT(maxAuthSz, 0);
+    handle.auth.size = (word16)maxAuthSz;
+    XMEMSET(handle.auth.buffer, 0x5A, handle.auth.size);
+    handle.name.size = (word16)sizeof(handle.name.name);
+    XMEMSET(handle.name.name, 0xA5, handle.name.size);
+
+    rc = wolfTPM2_SetAuthHandle(&dev, 0, &handle);
+    AssertIntEQ(rc, TPM_RC_SUCCESS);
+    AssertIntEQ(dev.session[0].auth.size,
+        (int)sizeof(dev.session[0].auth.buffer));
+    AssertIntEQ(XMEMCMP(&dev.session[0].auth.buffer[authDigestSz],
+        handle.auth.buffer, handle.auth.size), 0);
+    AssertIntEQ(dev.session[0].name.size,
+        (int)sizeof(dev.session[0].name.name));
+    AssertIntEQ(XMEMCMP(dev.session[0].name.name, handle.name.name,
+        handle.name.size), 0);
+
+    /* Reject one byte beyond the remaining auth capacity without mutation. */
+    handle.auth.size = (word16)(maxAuthSz + 1);
+    XMEMCPY(&sessionBefore, &dev.session[0], sizeof(sessionBefore));
+    rc = wolfTPM2_SetAuthHandle(&dev, 0, &handle);
+    AssertIntEQ(rc, BUFFER_E);
+    AssertIntEQ(XMEMCMP(&dev.session[0], &sessionBefore,
+        sizeof(sessionBefore)), 0);
+
+    /* Reject an oversized name without mutating the session. */
+    handle.auth.size = 4;
+    handle.name.size = (word16)(sizeof(handle.name.name) + 1U);
+    XMEMCPY(&sessionBefore, &dev.session[0], sizeof(sessionBefore));
+    rc = wolfTPM2_SetAuthHandle(&dev, 0, &handle);
+    AssertIntEQ(rc, BUFFER_E);
+    AssertIntEQ(XMEMCMP(&dev.session[0], &sessionBefore,
+        sizeof(sessionBefore)), 0);
+
+    /* Reject a non-hash session algorithm without mutating the session. */
+    handle.name.size = 2;
+    dev.session[0].authHash = TPM_ALG_NULL;
+    XMEMCPY(&sessionBefore, &dev.session[0], sizeof(sessionBefore));
+    rc = wolfTPM2_SetAuthHandle(&dev, 0, &handle);
+    AssertIntEQ(rc, BUFFER_E);
+    AssertIntEQ(XMEMCMP(&dev.session[0], &sessionBefore,
+        sizeof(sessionBefore)), 0);
+
     wolfTPM2_Cleanup(&dev);
 
     printf("Test TPM Wrapper: %-40s Passed\n", "SetAuthHandle PolicyAuth:");