F-3015 - https://fenrir.wolfssl.com/finding/3015 - Remove short-circuit OR in FwVerifySignatureCore RSA-PKCS1v1.5 check

Author: aidangarske

examples/pcr/policy_sign.c

@@ -194,10 +194,16 @@ static int PolicySign(TPM_ALG_ID alg, const char* keyFile, const char* password,
                 if (rc == 0) {
                     word32 keySz = key.ecc.dp->size;
                     *sigSz = keySz * 2;
-                    /* constant-time fixed-width export of r and s avoids
-                     * leaking the leading-zero count of each component */
+                    /* fixed-width export of r and s avoids leaking the
+                     * leading-zero count of each component via data-dependent
+                     * offsets */
+                #ifdef WOLFSSL_HAVE_SP_ECC
                     mp_to_unsigned_bin_len_ct(&r, &sig[0], keySz);
                     mp_to_unsigned_bin_len_ct(&s, &sig[keySz], keySz);
+                #else
+                    mp_to_unsigned_bin_len(&r, &sig[0], keySz);
+                    mp_to_unsigned_bin_len(&s, &sig[keySz], keySz);
+                #endif
                     mp_clear(&r);
                     mp_clear(&s);
                 }

src/fwtpm/fwtpm_crypto.c

@@ -1835,9 +1835,12 @@ TPM_RC FwImportVerifyAndDecrypt(
         }
     }
     if (rc == 0) {
-        /* Always run TPM2_ConstantCompare so timing doesn't leak size match */
+        /* Always run TPM2_ConstantCompare over min(sizes) so timing doesn't
+         * leak size match and we don't read past integrity[integritySize] */
+        word32 cmpSz = (integritySize < (UINT16)digestSz) ?
+            (word32)integritySize : (word32)digestSz;
         sizeMismatch = (integritySize != (UINT16)digestSz);
-        hmacDiff = TPM2_ConstantCompare(integrity, hmacCalc, (word32)digestSz);
+        hmacDiff = TPM2_ConstantCompare(integrity, hmacCalc, cmpSz);
         if (sizeMismatch | hmacDiff) {
             rc = TPM_RC_INTEGRITY;
         }
@@ -2543,18 +2546,29 @@ TPM_RC FwVerifySignatureCore(FWTPM_Object* obj,
                         (enum wc_HashType)wcHash);
                     int expSz = wc_EncodeSignature(expDI,
                         digest, digestSz, oid);
+                    int sizeMismatch;
+                    int sigDiff;
+                    word32 cmpSz;
 
                     FWTPM_ALLOC_BUF(decSig, FWTPM_MAX_PUB_BUF);
                     wcRc = wc_RsaSSL_Verify(
                         sig->signature.rsassa.sig.buffer,
                         sig->signature.rsassa.sig.size,
                         decSig, (word32)FWTPM_MAX_PUB_BUF, rsaKey);
-                    if (wcRc >= 0) {
-                        if (wcRc != expSz || expSz <= 0 ||
-                            TPM2_ConstantCompare(decSig, expDI, expSz) != 0) {
+                    if (wcRc >= 0 && expSz > 0) {
+                        /* Always run TPM2_ConstantCompare so timing doesn't
+                         * leak decoded-length vs expected-length match */
+                        sizeMismatch = (wcRc != expSz);
+                        cmpSz = (wcRc < expSz) ? (word32)wcRc :
+                            (word32)expSz;
+                        sigDiff = TPM2_ConstantCompare(decSig, expDI, cmpSz);
+                        if (sizeMismatch | sigDiff) {
                             wcRc = -1;
                         }
                     }
+                    else if (wcRc >= 0) {
+                        wcRc = -1;
+                    }
                     FWTPM_FREE_BUF(decSig);
                 }
                 if (wcRc < 0)
@@ -2876,6 +2890,9 @@ TPM_RC FwCredentialUnwrap(
     FWTPM_DECLARE_VAR(hmac, Hmac);
     FWTPM_DECLARE_BUF(decBuf, FWTPM_MAX_NV_DATA + 2);
 
+    /* Zero-init so tail bytes are deterministic when integrityHmacSz < 32 */
+    TPM2_ForceZero(integrityHmac, sizeof(integrityHmac));
+
     FWTPM_ALLOC_VAR(aes, Aes);
     FWTPM_ALLOC_VAR(hmac, Hmac);
     FWTPM_ALLOC_BUF(decBuf, FWTPM_MAX_NV_DATA + 2);

src/tpm2.c

@@ -386,9 +386,11 @@ static int TPM2_ResponseProcess(TPM2_CTX* ctx, TPM2_Packet* packet,
                     return rc;
                 }
 
-                /* Verify HMAC using constant-time comparison — always run
-                 * TPM2_ConstantCompare so code path timing doesn't depend on
-                 * whether the size matched. */
+                /* Verify HMAC using constant-time comparison. The wire-format
+                 * size was already validated above; this branch-free pattern
+                 * hardens the tail check in case a future refactor removes
+                 * that gate (hmac.size and authRsp.hmac.size are both
+                 * algorithm-derived and non-secret at this point). */
                 sizeMismatch = (hmac.size != authRsp.hmac.size);
                 diff = TPM2_ConstantCompare(hmac.buffer, authRsp.hmac.buffer,
                     expectedHmacSz);

src/tpm2_packet.c

@@ -833,6 +833,11 @@ void TPM2_Packet_ParseSensitive(TPM2_Packet* packet, TPM2B_SENSITIVE* sensitive)
     if (sensitive->size == 0) {
         return;
     }
+    /* Clamp outer size to remaining packet bytes so inner parses are bounded */
+    if (packet != NULL && packet->pos < packet->size &&
+            sensitive->size > (UINT16)(packet->size - packet->pos)) {
+        sensitive->size = (UINT16)(packet->size - packet->pos);
+    }
 
     TPM2_Packet_ParseU16(packet, &sensitive->sensitiveArea.sensitiveType);
 

src/tpm2_wrap.c

@@ -1158,8 +1158,9 @@ int wolfTPM2_SpdmEnable(WOLFTPM2_DEV* dev)
     if (rc == 0) {
         rc = wolfTPM2_SPDM_Enable(dev->spdmCtx);
     }
-    /* Restore previous session[0] state */
+    /* Restore previous session[0] state and clear the stack copy */
     XMEMCPY(&dev->session[0], &saveSess, sizeof(dev->session[0]));
+    TPM2_ForceZero(&saveSess, sizeof(saveSess));
     return rc;
 }
 
@@ -1177,8 +1178,9 @@ int wolfTPM2_SpdmDisable(WOLFTPM2_DEV* dev)
     if (rc == 0) {
         rc = wolfTPM2_SPDM_Disable(dev->spdmCtx);
     }
-    /* Restore previous session[0] state */
+    /* Restore previous session[0] state and clear the stack copy */
     XMEMCPY(&dev->session[0], &saveSess, sizeof(dev->session[0]));
+    TPM2_ForceZero(&saveSess, sizeof(saveSess));
     return rc;
 }
 
@@ -1586,8 +1588,9 @@ int wolfTPM2_SpdmNationsIdentityKeySet(WOLFTPM2_DEV* dev, int set)
         }
     }
 
-    /* Restore previous session[0] state */
+    /* Restore previous session[0] state and clear the stack copy */
     XMEMCPY(&dev->session[0], &saveSess, sizeof(dev->session[0]));
+    TPM2_ForceZero(&saveSess, sizeof(saveSess));
 
     return rc;
 }
@@ -2133,10 +2136,14 @@ static int wolfTPM2_EncryptSecret_ECC(WOLFTPM2_DEV* dev, const WOLFTPM2_KEY* tpm
             r, &a, &prime, 1);
     }
     if (rc == 0) {
-        /* export shared secret x - constant-time fixed-size export avoids
-         * leaking the leading-zero count of the ECDH shared secret via
-         * data-dependent offsets */
+        /* export shared secret x - fixed-size export avoids leaking the
+         * leading-zero count of the ECDH shared secret via data-dependent
+         * offsets */
+    #ifdef WOLFSSL_HAVE_SP_ECC
         rc = mp_to_unsigned_bin_len_ct(r->x, secretPoint.point.x.buffer, keySz);
+    #else
+        rc = mp_to_unsigned_bin_len(r->x, secretPoint.point.x.buffer, keySz);
+    #endif
         secretPoint.point.x.size = keySz;
     }
     if (rc == 0) {