Fixes from Skoll review.

Author: dgarske

hal/tpm_io_fwtpm.c

@@ -43,6 +43,7 @@
 #endif
 
 #include <sys/mman.h>
+#include <sys/stat.h>
 #include <semaphore.h>
 
 /* Static client context (one connection per process).
@@ -54,6 +55,7 @@ static int gFwtpmClientInit = 0;
 int FWTPM_TIS_ClientConnect(FWTPM_TIS_CLIENT_CTX* client)
 {
     int fd;
+    struct stat st;
     FWTPM_TIS_REGS* shm;
     sem_t* semCmd;
     sem_t* semRsp;
@@ -75,6 +77,17 @@ int FWTPM_TIS_ClientConnect(FWTPM_TIS_CLIENT_CTX* client)
         return TPM_RC_FAILURE;
     }
 
+    /* Verify file is large enough before mapping */
+    if (fstat(fd, &st) != 0 ||
+            st.st_size < (off_t)sizeof(FWTPM_TIS_REGS)) {
+    #ifdef DEBUG_WOLFTPM
+        printf("fwTPM HAL: shm file too small (expected %lu)\n",
+            (unsigned long)sizeof(FWTPM_TIS_REGS));
+    #endif
+        close(fd);
+        return TPM_RC_FAILURE;
+    }
+
     shm = (FWTPM_TIS_REGS*)mmap(NULL, sizeof(FWTPM_TIS_REGS),
         PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
     if (shm == MAP_FAILED) {
@@ -178,12 +191,16 @@ int TPM2_IoCb_FwTPM(TPM2_CTX* ctx, int isRead, word32 addr,
      * Note: thread safety is provided by the TPM context lock in tpm2_tis.c
      * (TPM2_AcquireLock), so no additional mutex is needed here. */
     if (!gFwtpmClientInit) {
+        static int atexitRegistered = 0;
         int rc = FWTPM_TIS_ClientConnect(client);
         if (rc != TPM_RC_SUCCESS) {
             return rc;
         }
         gFwtpmClientInit = 1;
-        atexit(FWTPM_TIS_ClientCleanup);
+        if (!atexitRegistered) {
+            atexit(FWTPM_TIS_ClientCleanup);
+            atexitRegistered = 1;
+        }
     }
 
     shm = client->shm;

scripts/fwtpm_emu_test.sh

@@ -64,15 +64,6 @@ if [ -z "$M33MU" ] || [ ! -x "$M33MU" ]; then
     exit 1
 fi
 
-# Verify m33mu can actually run (catch missing shared libs)
-if ! "$M33MU" --version > /dev/null 2>&1; then
-    echo "ERROR: m33mu found at $M33MU but failed to execute."
-    echo "  Checking shared library dependencies:"
-    ldd "$M33MU" 2>&1 | grep -i "not found" || echo "  (no missing libraries detected)"
-    echo "  File type: $(file "$M33MU")"
-    exit 1
-fi
-
 echo "=== fwTPM Emulator Test ==="
 echo "  m33mu: $M33MU"
 echo "  TZEN: $TZEN"

src/fwtpm/fwtpm_command.c

@@ -315,7 +315,9 @@ static int FwGetEntityName(FWTPM_CTX* ctx, TPM_HANDLE handle,
         if (nv != NULL) {
             UINT16 nvNameSz = 0;
             FwComputeNvName(nv, nameBuf, &nvNameSz);
-            return (int)nvNameSz;
+            if (nvNameSz <= nameBufSz) {
+                return (int)nvNameSz;
+            }
         }
     }
 #endif /* !FWTPM_NO_NV */
@@ -3367,7 +3369,12 @@ static TPM_RC FwCmd_Create(FWTPM_CTX* ctx, TPM2_Packet* cmd,
     }
     if (rc == 0) {
         TPM2_Packet_ParseU16(cmd, &outsideInfoSize);
-        cmd->pos += outsideInfoSize;
+        if (cmd->pos + outsideInfoSize > cmdSize) {
+            rc = TPM_RC_COMMAND_SIZE;
+        }
+        else {
+            cmd->pos += outsideInfoSize;
+        }
     }
 
     /* Parse creationPCR (TPML_PCR_SELECTION) - skip */
@@ -4455,7 +4462,7 @@ static TPM_RC FwCmd_Import(FWTPM_CTX* ctx, TPM2_Packet* cmd,
     TPM2_ForceZero(privKeyDer, FWTPM_MAX_PRIVKEY_DER);
     FWTPM_FREE_BUF(privKeyDer);
     FWTPM_FREE_BUF(pubAreaBuf);
-    TPM2_ForceZero(plainSens, FWTPM_MAX_DATA_BUF);
+    TPM2_ForceZero(plainSens, FWTPM_MAX_SENSITIVE_SIZE);
     FWTPM_FREE_BUF(plainSens);
     TPM2_ForceZero(primeBuf, FWTPM_MAX_DER_SIG_BUF);
     FWTPM_FREE_BUF(primeBuf);
@@ -5392,6 +5399,7 @@ static TPM_RC FwCmd_CreateLoaded(FWTPM_CTX* ctx, TPM2_Packet* cmd,
 
     TPM2_ForceZero(&userAuth, sizeof(userAuth));
     TPM2_ForceZero(sensData, sizeof(sensData));
+    TPM2_ForceZero(&outPrivate, sizeof(outPrivate));
     TPM2_ForceZero(privKeyDer, FWTPM_MAX_PRIVKEY_DER);
     FWTPM_FREE_BUF(privKeyDer);
     return rc;
@@ -11650,9 +11658,7 @@ static TPM_RC FwCmd_EC_Ephemeral(FWTPM_CTX* ctx, TPM2_Packet* cmd,
     }
 
     if (rc == 0) {
-        UINT32 curveID32;
-        TPM2_Packet_ParseU32(cmd, &curveID32);
-        curveID = (UINT16)curveID32;
+        TPM2_Packet_ParseU16(cmd, &curveID);
 
         wcCurve = FwGetWcCurveId(curveID);
         keySz = FwGetEccKeySize(curveID);

src/fwtpm/fwtpm_crypto.c

@@ -1621,6 +1621,9 @@ TPM_RC FwImportVerifyAndDecrypt(
      * HMAC-nameAlg(hmacKeyBuf, encSens || name.name) */
     if (rc == 0) {
         wcHmacType = FwGetWcHashType(parentNameAlg);
+        rc = wc_HmacInit(hmacObj, NULL, INVALID_DEVID);
+    }
+    if (rc == 0) {
         rc = wc_HmacSetKey(hmacObj, (int)wcHmacType,
             hmacKeyBuf, (word32)digestSz);
         if (rc == 0) {
@@ -2021,12 +2024,18 @@ int FwRsaComputeCRT(RsaKey* rsaKey)
     }
 
     /* phi = (p-1)(q-1) */
-    mp_sub_d(&rsaKey->p, 1, &pm1);
-    mp_sub_d(&rsaKey->q, 1, &qm1);
-    mp_mul(&pm1, &qm1, &phi);
+    rc = mp_sub_d(&rsaKey->p, 1, &pm1);
+    if (rc == 0) {
+        rc = mp_sub_d(&rsaKey->q, 1, &qm1);
+    }
+    if (rc == 0) {
+        rc = mp_mul(&pm1, &qm1, &phi);
+    }
 
     /* d = e^{-1} mod phi */
-    rc = mp_invmod(&rsaKey->e, &phi, &rsaKey->d);
+    if (rc == 0) {
+        rc = mp_invmod(&rsaKey->e, &phi, &rsaKey->d);
+    }
     if (rc == 0) {
         rc = mp_mod(&rsaKey->d, &pm1, &rsaKey->dP);
     }
@@ -2040,6 +2049,9 @@ int FwRsaComputeCRT(RsaKey* rsaKey)
     mp_forcezero(&pm1);
     mp_forcezero(&qm1);
     mp_forcezero(&phi);
+    mp_clear(&pm1);
+    mp_clear(&qm1);
+    mp_clear(&phi);
 
     if (rc != 0) {
         rc = TPM_RC_FAILURE;

src/fwtpm/fwtpm_nv.c

@@ -309,6 +309,9 @@ static int FwNvMarshalPublic(byte* buf, word32* pos, word32 maxSz,
     XMEMCPY(&pub2b.publicArea, pub, sizeof(TPMT_PUBLIC));
     TPM2_Packet_AppendPublic(&pkt, &pub2b);
 
+    if (pkt.pos <= 0 || (word32)pkt.pos > (maxSz - *pos)) {
+        return TPM_RC_FAILURE;
+    }
     *pos += pkt.pos;
     return 0;
 }
@@ -325,6 +328,10 @@ static int FwNvUnmarshalPublic(const byte* buf, word32* pos, word32 maxSz,
     pkt.size = (int)(maxSz - *pos);
 
     TPM2_Packet_ParsePublic(&pkt, &pub2b);
+
+    if (pkt.pos <= 0 || (word32)pkt.pos > (maxSz - *pos)) {
+        return TPM_RC_FAILURE;
+    }
     XMEMCPY(pub, &pub2b.publicArea, sizeof(TPMT_PUBLIC));
 
     *pos += pkt.pos;
@@ -509,7 +516,6 @@ static int FwNvUnmarshalObject(const byte* buf, word32* pos, word32 maxSz,
     UINT16 privSz;
 
     XMEMSET(obj, 0, sizeof(FWTPM_Object));
-    obj->used = 1;
 
     rc = FwNvUnmarshalU32(buf, pos, maxSz, &obj->handle);
     if (rc == 0) {
@@ -523,8 +529,10 @@ static int FwNvUnmarshalObject(const byte* buf, word32* pos, word32 maxSz,
     }
     if (rc == 0) {
         if (privSz > FWTPM_MAX_PRIVKEY_DER) {
-            return TPM_RC_FAILURE;
+            rc = TPM_RC_FAILURE;
         }
+    }
+    if (rc == 0) {
         obj->privKeySize = (int)privSz;
         if (privSz > 0) {
             rc = FwNvUnmarshalBytes(buf, pos, maxSz, obj->privKey, privSz);
@@ -533,6 +541,9 @@ static int FwNvUnmarshalObject(const byte* buf, word32* pos, word32 maxSz,
     if (rc == 0) {
         rc = FwNvUnmarshalName(buf, pos, maxSz, &obj->name);
     }
+    if (rc == 0) {
+        obj->used = 1;
+    }
     return rc;
 }
 
@@ -571,7 +582,6 @@ static int FwNvUnmarshalPrimaryCache(const byte* buf, word32* pos,
     UINT16 privSz;
 
     XMEMSET(cache, 0, sizeof(FWTPM_PrimaryCache));
-    cache->used = 1;
 
     rc = FwNvUnmarshalU32(buf, pos, maxSz, &cache->hierarchy);
     if (rc == 0) {
@@ -586,13 +596,18 @@ static int FwNvUnmarshalPrimaryCache(const byte* buf, word32* pos,
     }
     if (rc == 0) {
         if (privSz > FWTPM_MAX_PRIVKEY_DER) {
-            return TPM_RC_FAILURE;
+            rc = TPM_RC_FAILURE;
         }
+    }
+    if (rc == 0) {
         cache->privKeySize = (int)privSz;
         if (privSz > 0) {
             rc = FwNvUnmarshalBytes(buf, pos, maxSz, cache->privKey, privSz);
         }
     }
+    if (rc == 0) {
+        cache->used = 1;
+    }
     return rc;
 }
 
@@ -603,15 +618,15 @@ static int FwNvUnmarshalPrimaryCache(const byte* buf, word32* pos,
 /* Write NV header at offset 0 */
 static int FwNvWriteHeader(FWTPM_CTX* ctx)
 {
-    FWTPM_NV_HEADER hdr;
+    byte hdr[sizeof(FWTPM_NV_HEADER)]; /* 4 x UINT32 = 16 bytes */
     FWTPM_NV_HAL* hal = &ctx->nvHal;
 
-    hdr.magic = FWTPM_NV_MAGIC;
-    hdr.version = FWTPM_NV_VERSION;
-    hdr.writePos = ctx->nvWritePos;
-    hdr.maxSize = hal->maxSize;
+    FwStoreU32LE(hdr + 0,  FWTPM_NV_MAGIC);
+    FwStoreU32LE(hdr + 4,  FWTPM_NV_VERSION);
+    FwStoreU32LE(hdr + 8,  ctx->nvWritePos);
+    FwStoreU32LE(hdr + 12, hal->maxSize);
 
-    return hal->write(hal->ctx, 0, (const byte*)&hdr, sizeof(hdr));
+    return hal->write(hal->ctx, 0, hdr, sizeof(hdr));
 }
 
 /* Append a single TLV entry to the journal */
@@ -960,6 +975,7 @@ int FWTPM_NV_Init(FWTPM_CTX* ctx)
 {
     int rc;
     FWTPM_NV_HEADER hdr;
+    byte hdrBuf[sizeof(FWTPM_NV_HEADER)];
     FWTPM_NV_HAL* hal;
     word32 pos;
     byte tlvHdr[TLV_HDR_SIZE];
@@ -992,8 +1008,14 @@ int FWTPM_NV_Init(FWTPM_CTX* ctx)
     }
 
     /* Try to read existing NV header */
-    XMEMSET(&hdr, 0, sizeof(hdr));
-    rc = hal->read(hal->ctx, 0, (byte*)&hdr, sizeof(FWTPM_NV_HEADER));
+    XMEMSET(hdrBuf, 0, sizeof(hdrBuf));
+    rc = hal->read(hal->ctx, 0, hdrBuf, sizeof(hdrBuf));
+    if (rc == TPM_RC_SUCCESS) {
+        hdr.magic    = FwLoadU32LE(hdrBuf + 0);
+        hdr.version  = FwLoadU32LE(hdrBuf + 4);
+        hdr.writePos = FwLoadU32LE(hdrBuf + 8);
+        hdr.maxSize  = FwLoadU32LE(hdrBuf + 12);
+    }
 
     if (rc == TPM_RC_SUCCESS &&
         hdr.magic == FWTPM_NV_MAGIC &&

src/fwtpm/fwtpm_tis.c

@@ -264,6 +264,9 @@ static void TisHandleRegAccess(FWTPM_CTX* ctx, FWTPM_TIS_REGS* regs)
                 if (len > avail) {
                     len = avail;
                 }
+                if (len > sizeof(regs->reg_data)) {
+                    len = (UINT32)sizeof(regs->reg_data);
+                }
                 for (i = 0; i < len; i++) {
                     regs->reg_data[i] = regs->rsp_buf[regs->fifo_read_pos++];
                 }

src/tpm2.c

@@ -3173,7 +3173,7 @@ TPM_RC TPM2_EC_Ephemeral(EC_Ephemeral_In* in, EC_Ephemeral_Out* out)
 
         TPM2_Packet_Init(ctx, &packet);
         st = TPM2_Packet_AppendAuth(&packet, ctx, &info);
-        TPM2_Packet_AppendU32(&packet, in->curveID);
+        TPM2_Packet_AppendU16(&packet, in->curveID);
         TPM2_Packet_Finalize(&packet, st, TPM_CC_EC_Ephemeral);
 
         /* send command */

src/tpm2_packet.c

@@ -263,7 +263,10 @@ void TPM2_Packet_ParseU16Buf(TPM2_Packet* packet, UINT16* size, byte* buf,
 
     TPM2_Packet_ParseU16(packet, &wireSize);
     /* Clamp to remaining packet bytes to prevent pos from going past size */
-    if (packet && wireSize > (UINT16)(packet->size - packet->pos)) {
+    if (packet && (packet->pos >= packet->size)) {
+        wireSize = 0;
+    }
+    else if (packet && wireSize > (UINT16)(packet->size - packet->pos)) {
         wireSize = (UINT16)(packet->size - packet->pos);
     }
     copySz = wireSize;
@@ -763,7 +766,7 @@ TPM_RC TPM2_Packet_ParseSensitiveCreate(TPM2_Packet* packet, int maxSize,
 {
     TPM_RC rc = TPM_RC_SUCCESS;
     UINT16 inSensSize;
-    UINT16 dataSz;
+    UINT16 dataSz = 0;
     int sensStartPos;
 
     if (packet->pos + 2 > maxSize) {

src/tpm2_swtpm.c

@@ -161,20 +161,12 @@ static TPM_RC SwTpmConnect(TPM2_CTX* ctx, const char* host, const char* port)
     struct termios tty;
     speed_t baud;
     int baudInt;
-#ifndef NO_GETENV
-    const char* envDev;
-#endif
 
     if (ctx == NULL) {
         return BAD_FUNC_ARG;
     }
-    /* Allow runtime override via TPM2_SWTPM_HOST env var */
-#ifndef NO_GETENV
-    envDev = getenv("TPM2_SWTPM_HOST");
-    if (envDev != NULL && envDev[0] != '\0') {
-        host = envDev;
-    }
-#endif
+    /* Note: TPM2_SWTPM_HOST env var is checked by caller
+     * (TPM2_SWTPM_SendCommand) before invoking SwTpmConnect */
 
     fd = open(host, O_RDWR | O_NOCTTY);
     if (fd < 0) {