From 38867e23e2aa0e2809f06f1cfdf623f2fd9e91a1 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Wed, 26 Aug 2026 09:13:05 -0400 Subject: [PATCH 01/10] tpm2: Cast size_t variable to ssize_t to check for negative values Cast size_t i to ssize_t i to check for underrun. An underrun could never occur since it is not possible that all commands are disabled. Signed-off-by: Stefan Berger --- src/tpm2/RuntimeCommands.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tpm2/RuntimeCommands.c b/src/tpm2/RuntimeCommands.c index 3edd31f57..260eae6c5 100644 --- a/src/tpm2/RuntimeCommands.c +++ b/src/tpm2/RuntimeCommands.c @@ -490,7 +490,7 @@ RuntimeCommandsGetArraySize(struct RuntimeCommands *RuntimeCommands) /* search for byte where command with highest code is enabled */ for (i = sizeof(RuntimeCommands->enabledCommandsByIdx) - 1; - i >= 0; + (ssize_t)i >= 0; i--) { if (RuntimeCommands->enabledCommandsByIdx[i]) return i + 1; From af8f9b9332faff6365f79643ecf763c5e9cabd5d Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Thu, 27 Aug 2026 15:15:18 -0400 Subject: [PATCH 02/10] tpm2: Use ~0 for max. StateFormatLevel when rolling back failure When rolling back after an unsuccessful switch to the new attributes/algorithms/commands profile, use the maximum possible StateFormatLevel value to set the old profile. Signed-off-by: Stefan Berger --- src/tpm2/RuntimeAlgorithm.c | 2 +- src/tpm2/RuntimeAttributes.c | 2 +- src/tpm2/RuntimeCommands.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tpm2/RuntimeAlgorithm.c b/src/tpm2/RuntimeAlgorithm.c index f7ab2ee7c..6af9f4c93 100644 --- a/src/tpm2/RuntimeAlgorithm.c +++ b/src/tpm2/RuntimeAlgorithm.c @@ -514,7 +514,7 @@ RuntimeAlgorithmSwitchProfile(struct RuntimeAlgorithm *RuntimeAlgorithm, &stateFormatLevel, maxStateFormatLevel); if (retVal != TPM_RC_SUCCESS) { RuntimeAlgorithmSetProfile(RuntimeAlgorithm, *oldProfile, - &stateFormatLevel, maxStateFormatLevel); + &stateFormatLevel, ~0); *oldProfile = NULL; } return retVal; diff --git a/src/tpm2/RuntimeAttributes.c b/src/tpm2/RuntimeAttributes.c index e2e5b8ad1..a50f0fd12 100644 --- a/src/tpm2/RuntimeAttributes.c +++ b/src/tpm2/RuntimeAttributes.c @@ -185,7 +185,7 @@ RuntimeAttributesSwitchProfile(struct RuntimeAttributes *RuntimeAttributes, &stateFormatLevel, maxStateFormatLevel); if (retVal != TPM_RC_SUCCESS) { RuntimeAttributesSetProfile(RuntimeAttributes, *oldProfile, - &stateFormatLevel, maxStateFormatLevel); + &stateFormatLevel, ~0); *oldProfile = NULL; } return retVal; diff --git a/src/tpm2/RuntimeCommands.c b/src/tpm2/RuntimeCommands.c index 260eae6c5..cc2b29639 100644 --- a/src/tpm2/RuntimeCommands.c +++ b/src/tpm2/RuntimeCommands.c @@ -394,7 +394,7 @@ RuntimeCommandsSwitchProfile(struct RuntimeCommands *RuntimeCommands, &stateFormatLevel, maxStateFormatLevel); if (retVal != TPM_RC_SUCCESS) { RuntimeCommandsSetProfile(RuntimeCommands, *oldProfile, - &stateFormatLevel, maxStateFormatLevel); + &stateFormatLevel, ~0); *oldProfile = NULL; } return retVal; From 9eb1b44f1b4815537988e4111f42ea33dc24b904 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Wed, 2 Sep 2026 12:01:51 -0400 Subject: [PATCH 03/10] tpm2: Replace a pAssert with an error log message and return error code Rather than asserting, display an error log message in case an ANY_OBJECT could not be unmarshalled. Signed-off-by: Stefan Berger --- src/tpm2/NVMarshal.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/tpm2/NVMarshal.c b/src/tpm2/NVMarshal.c index 7c14bb02c..7368392bf 100644 --- a/src/tpm2/NVMarshal.c +++ b/src/tpm2/NVMarshal.c @@ -5002,7 +5002,11 @@ USER_NVRAM_Unmarshal(BYTE **buffer, INT32 *size) memset(&obj, 0, sizeof(obj)); rc = ANY_OBJECT_Unmarshal(&obj, buffer, size, true); - pAssert(rc == TPM_RC_SUCCESS); + if (rc != TPM_RC_SUCCESS) { + TPMLIB_LogTPM2Error("USER_NVRAM: Failed to unmarshal ANY_OBJECT: 0x%08x\n", + rc); + return rc; + } // convert the OBJECT into a buffer to copy into NVRAM marshalledObjectSize = NvObjectToBuffer(&obj, objBuffer, sizeof(objBuffer)); From 763d7484160acab65d31d1536d19d17777bba503 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Wed, 2 Sep 2026 13:32:02 -0400 Subject: [PATCH 04/10] tpm2: Check unmarshalled array_size against a reasonable maximum (1024b) Check the untrusted array_size of AuditCommandList and PPList against a reasonable maximum value of 1024. Artificially setting BYTE buf[0xffff], which would be the maximum value array_size could get with the unmarshalling, does not cause any issues (crash due to large on-stack buffer) on my Linux system. Signed-off-by: Stefan Berger --- src/tpm2/NVMarshal.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/tpm2/NVMarshal.c b/src/tpm2/NVMarshal.c index 7368392bf..c53882c17 100644 --- a/src/tpm2/NVMarshal.c +++ b/src/tpm2/NVMarshal.c @@ -4148,6 +4148,9 @@ PERSISTENT_DATA_PPList_Unmarshal(PERSISTENT_DATA *data, BYTE **buffer, INT32 *si if (rc == TPM_RC_SUCCESS) { rc = UINT16_Unmarshal(&array_size, buffer, size); + /* check array_size for a reasonable maximum that should never be reached */ + if (rc == TPM_RC_SUCCESS && array_size > 1024) + rc = TPM_RC_SIZE; } if (rc == TPM_RC_SUCCESS) { BYTE buf[array_size]; @@ -4216,6 +4219,9 @@ PERSISTENT_DATA_AuditCommands_Unmarshal(PERSISTENT_DATA *data, BYTE **buffer, IN if (rc == TPM_RC_SUCCESS) { rc = UINT16_Unmarshal(&array_size, buffer, size); + /* check array_size for a reasonable maximum that should never be reached */ + if (rc == TPM_RC_SUCCESS && array_size > 1024) + rc = TPM_RC_SIZE; } if (rc == TPM_RC_SUCCESS) { BYTE buf[array_size]; From ba73ab173c7b991369df8b5f7241fce816924910 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Thu, 3 Sep 2026 12:36:56 -0400 Subject: [PATCH 05/10] tpm2: Limit the size of the profile to MAX_PROFILE_SIZE (32kb) Limit the size of a profile to 32kb. There is no reason that a valid profile would have even close to 32kb. This also prevents issues in String_Marshal() that assumes a reasonably sized string below 64kb. Place the check for the size of the profile into GetParametersFromJSON since this function is used by RuntimeProfileSet and RuntimeProfileTest. Signed-off-by: Stefan Berger --- src/tpm2/RuntimeProfile.c | 3 +++ src/tpm2/RuntimeProfile_fp.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/tpm2/RuntimeProfile.c b/src/tpm2/RuntimeProfile.c index 59c99e98c..23a5c20e0 100644 --- a/src/tpm2/RuntimeProfile.c +++ b/src/tpm2/RuntimeProfile.c @@ -479,6 +479,9 @@ GetParametersFromJSON(const char *jsonProfile, return TPM_RC_SUCCESS; } + if (strlen(jsonProfile) > MAX_PROFILE_SIZE) + return TPM_RC_SIZE; + retVal = RuntimeProfileCheckJSON(jsonProfile); if (retVal != TPM_RC_SUCCESS) return retVal; diff --git a/src/tpm2/RuntimeProfile_fp.h b/src/tpm2/RuntimeProfile_fp.h index 37d6cae9d..a72ea947a 100644 --- a/src/tpm2/RuntimeProfile_fp.h +++ b/src/tpm2/RuntimeProfile_fp.h @@ -46,6 +46,8 @@ #include "RuntimeCommands_fp.h" #include "RuntimeAttributes_fp.h" +#define MAX_PROFILE_SIZE (32 * 1024) + struct RuntimeProfile { struct RuntimeAlgorithm RuntimeAlgorithm; struct RuntimeCommands RuntimeCommands; From 9e1475ff478a4a2bdd21f0b8d8b35d4d251f5fea Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Thu, 3 Sep 2026 14:47:01 -0400 Subject: [PATCH 06/10] tpm2: Add checks for *size < 0 before casting it to UINT32 Signed-off-by: Stefan Berger --- src/tpm2/Unmarshal.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tpm2/Unmarshal.c b/src/tpm2/Unmarshal.c index e2aa0215b..5527247c1 100644 --- a/src/tpm2/Unmarshal.c +++ b/src/tpm2/Unmarshal.c @@ -47,7 +47,7 @@ TPM_RC UINT8_Unmarshal(UINT8 *target, BYTE **buffer, INT32 *size) { - if ((UINT32)*size < sizeof(UINT8)) { + if (*size < 0 || (UINT32)*size < sizeof(UINT8)) { return TPM_RC_INSUFFICIENT; } *target = (*buffer)[0]; @@ -65,7 +65,7 @@ INT8_Unmarshal(INT8 *target, BYTE **buffer, INT32 *size) TPM_RC UINT16_Unmarshal(UINT16 *target, BYTE **buffer, INT32 *size) { - if ((UINT32)*size < sizeof(UINT16)) { + if (*size < 0 || (UINT32)*size < sizeof(UINT16)) { return TPM_RC_INSUFFICIENT; } *target = ((UINT16)((*buffer)[0]) << 8) | @@ -78,7 +78,7 @@ UINT16_Unmarshal(UINT16 *target, BYTE **buffer, INT32 *size) TPM_RC UINT32_Unmarshal(UINT32 *target, BYTE **buffer, INT32 *size) { - if ((UINT32)*size < sizeof(UINT32)) { + if (*size < 0 || (UINT32)*size < sizeof(UINT32)) { return TPM_RC_INSUFFICIENT; } *target = ((UINT32)((*buffer)[0]) << 24) | @@ -93,7 +93,7 @@ UINT32_Unmarshal(UINT32 *target, BYTE **buffer, INT32 *size) TPM_RC UINT64_Unmarshal(UINT64 *target, BYTE **buffer, INT32 *size) { - if ((UINT32)*size < sizeof(UINT64)) { + if (*size < 0 || (UINT32)*size < sizeof(UINT64)) { return TPM_RC_INSUFFICIENT; } *target = ((UINT64)((*buffer)[0]) << 56) | @@ -114,7 +114,7 @@ Array_Unmarshal(BYTE *targetBuffer, UINT16 targetSize, BYTE **buffer, INT32 *siz { TPM_RC rc = TPM_RC_SUCCESS; - if (targetSize > *size) { + if (*size < 0 || targetSize > (UINT32)*size) { rc = TPM_RC_INSUFFICIENT; } else { From 5668fde0d6f907ed47f6c56cd26d23c060b5efbb Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Thu, 3 Sep 2026 16:20:16 -0400 Subject: [PATCH 07/10] tpm2: Add check rc == TPM_RC_SUCCESS before calling function Check for rc == TPM_RC_SUCCESS before calling function to check whether the EC algorithm is allowed by the profile. This is necessary since rc could already have been set to TPM_RC_CURVE before. In case the function call failed, it would again be set to TPM_RC_CURVE, which is unnecessary. Also do not call the function when TPM_ECC_NONE was unmarshalled since this would always fail and unmarshalling TPM_ECC_NONE would never work. Signed-off-by: Stefan Berger --- src/tpm2/Unmarshal.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tpm2/Unmarshal.c b/src/tpm2/Unmarshal.c index 5527247c1..6a575f083 100644 --- a/src/tpm2/Unmarshal.c +++ b/src/tpm2/Unmarshal.c @@ -249,9 +249,10 @@ TPM_ECC_CURVE_Unmarshal(TPM_ECC_CURVE *target, BYTE **buffer, INT32 *size) # endif // ECC_CURVE_448 if (*target != TPM_ECC_NONE && // libtpms added begin !CryptEccIsCurveRuntimeUsable(*target)) { - rc = TPM_RC_CURVE; + rc = TPM_RC_CURVE; } - if (!RuntimeAlgorithmKeySizeCheckEnabled(&g_RuntimeProfile.RuntimeAlgorithm, + if (rc == TPM_RC_SUCCESS && *target != TPM_ECC_NONE && + !RuntimeAlgorithmKeySizeCheckEnabled(&g_RuntimeProfile.RuntimeAlgorithm, TPM_ALG_ECC, CryptEccGetKeySizeForCurve(*target), *target, From c8050d13b1651d9f82fb0296c6c93bd1322de2e2 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Tue, 15 Sep 2026 10:28:03 -0400 Subject: [PATCH 08/10] tpm2: Read attributes from proper NVRAM offset in NvFlushHierarchy The attributes of an OBJECT were previously at a fixed offset from the beginning of an OBJECT in the NVRAM byte array. However, since ANY_OBJECT_Marshal is now being used to write an OBJECT into the NVRAM, the offset of the OBJECT attributes may be different depending on the current StateFormatLevel and the layout of the OBJECT. Therefore, ANY_OBJECT_Unmarshal has to be used to get the attributes of an OBJECT. Fixes: #626 Reported-by: Kun Qin Signed-off-by: Stefan Berger --- src/tpm2/NVDynamic.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/tpm2/NVDynamic.c b/src/tpm2/NVDynamic.c index aaf227be5..7226150af 100644 --- a/src/tpm2/NVDynamic.c +++ b/src/tpm2/NVDynamic.c @@ -668,6 +668,16 @@ void NvReadObject(NV_REF ref, // IN: points to NV where index is located return; // libtpms changed end } + // libtpms added begin +static void NvReadObjectAttributes(NV_REF ref, // IN: points to NV where index is located, + OBJECT_ATTRIBUTES* attributes // OUT: place to receive the object attributes +) +{ + OBJECT obj = { 0 }; + + NvReadObject(ref, &obj); + *attributes = obj.attributes; +} // libtpms added end //*** NvFindEvict() // This function will return the NV offset of an evict object @@ -1414,10 +1424,13 @@ NvFlushHierarchy(TPMI_RH_HIERARCHY hierarchy // IN: hierarchy to be flushed. { OBJECT_ATTRIBUTES attributes; // +#if 0 // libtpms changed begin NvRead(&attributes, (UINT32)(currentAddr + sizeof(TPM_HANDLE) + offsetof(OBJECT, attributes)), sizeof(OBJECT_ATTRIBUTES)); +#endif + NvReadObjectAttributes(currentAddr, &attributes); // libtpms changed end // If the evict object belongs to the hierarchy to be flushed... if((hierarchy == TPM_RH_PLATFORM && attributes.ppsHierarchy == SET) || (hierarchy == TPM_RH_OWNER && attributes.spsHierarchy == SET) From 10848d786241d22e3416c9b262186da49dd38dc8 Mon Sep 17 00:00:00 2001 From: sameera Date: Fri, 16 Oct 2026 21:08:24 +0530 Subject: [PATCH 09/10] tpm12: reject non-block-aligned CBC input in TPM_SymmetricKeyData_Decrypt TPM_SymmetricKeyData_Decrypt (AES path in src/tpm12/tpm_crypto.c, OpenSSL backend; and the NSS/FreeBL backend in src/tpm12/tpm_crypto_freebl.c) only checked encrypt_length < TPM_AES_BLOCK_SIZE before calling AES_cbc_encrypt(..., AES_DECRYPT) on a buffer allocated to exactly encrypt_length bytes. CBC decryption always consumes a whole final block, so a length that is not a multiple of 16 makes OpenSSL read up to 15 bytes past the end of the input buffer. That length is TPM_CONTEXT_BLOB.sensitiveData.size from the command stream. TPM_Process_LoadKeyContext and TPM_Process_LoadAuthContext run under TPM_CHECK_ALLOW_NO_OWNER and decrypt the blob with the permanent contextKey before the integrity HMAC is checked, so the over-read is reachable with a single unauthenticated command. Reproduced with a TPM_LoadKeyContext command whose sensitiveData.size is 17; AddressSanitizer reports a heap over-read: READ of size 4 ... #0 AES_decrypt aes_core.c #1 CRYPTO_cbc128_decrypt cbc128.c #2 TPM_SymmetricKeyData_Decrypt tpm_crypto.c #3 TPM_Process_LoadKeyContext tpm_session.c The fix rejects a length that is not a whole number of blocks, the same requirement the DES backend already enforces. The command then returns TPM_DECRYPT_ERROR. A zero length is still rejected by TPM_Malloc(). Signed-off-by: sameera --- src/tpm12/tpm_crypto.c | 5 +++-- src/tpm12/tpm_crypto_freebl.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/tpm12/tpm_crypto.c b/src/tpm12/tpm_crypto.c index add71c40d..acf9b9259 100644 --- a/src/tpm12/tpm_crypto.c +++ b/src/tpm12/tpm_crypto.c @@ -2862,9 +2862,10 @@ TPM_RESULT TPM_SymmetricKeyData_Decrypt(unsigned char **decrypt_data, /* outpu (TPM_SYMMETRIC_KEY_DATA *)tpm_symmetric_key_token; printf(" TPM_SymmetricKeyData_Decrypt: Length %u\n", encrypt_length); - /* sanity check encrypted length */ + /* sanity check encrypted length; CBC requires a whole number of blocks, + otherwise AES_cbc_encrypt() reads past the end of the input buffer */ if (rc == 0) { - if (encrypt_length < TPM_AES_BLOCK_SIZE) { + if ((encrypt_length % TPM_AES_BLOCK_SIZE) != 0) { printf("TPM_SymmetricKeyData_Decrypt: Error, bad length\n"); rc = TPM_DECRYPT_ERROR; } diff --git a/src/tpm12/tpm_crypto_freebl.c b/src/tpm12/tpm_crypto_freebl.c index 272c26472..5a249897b 100644 --- a/src/tpm12/tpm_crypto_freebl.c +++ b/src/tpm12/tpm_crypto_freebl.c @@ -2323,9 +2323,9 @@ TPM_RESULT TPM_SymmetricKeyData_Decrypt(unsigned char **decrypt_data, /* outpu printf(" TPM_SymmetricKeyData_Decrypt: Length %u\n", encrypt_length); cx = NULL; /* freed @1 */ - /* sanity check encrypted length */ + /* sanity check encrypted length; CBC requires a whole number of blocks */ if (rc == 0) { - if (encrypt_length < TPM_AES_BLOCK_SIZE) { + if ((encrypt_length % TPM_AES_BLOCK_SIZE) != 0) { printf("TPM_SymmetricKeyData_Decrypt: Error, bad length\n"); rc = TPM_DECRYPT_ERROR; } From d7cd69b72a91abc81f17f0dfea20e05876de474c Mon Sep 17 00:00:00 2001 From: sameera Date: Sat, 24 Oct 2026 14:59:22 +0530 Subject: [PATCH 10/10] tpm12: validate SHA1 context num on load in TPM_Sha1Context_Load TPM_Sha1Context_Load() (OpenSSL backend, src/tpm12/tpm_crypto.c) deserializes the OpenSSL SHA_CTX from a TPMLIB_SetState(VOLATILE) blob and restores sha_ctx->num with TPM_Load32() without any range check. num is the number of bytes buffered in sha_ctx->data and OpenSSL uses it as the write offset in SHA1_Update()/SHA1_Final(): the update path runs memcpy(p + num, data, SHA_CBLOCK - num). A restored num >= SHA_CBLOCK makes SHA_CBLOCK - num wrap to a huge size_t, so the next update writes far past the heap-allocated context, and SHA1_Final() writes the 0x80 pad byte at p[num] well outside it. The restored sha1_context / sha1_context_tis are later driven by the TPM_SHA1Update command and by the host-side TPM_IO_Hash_Data(), both of which reach TPM_SHA1UpdateCmd() -> SHA1_Update(). The volatile-state blob is only protected by an unkeyed SHA1 integrity digest computed after parsing, so num is attacker-controlled across a state restore. Reproduced by loading a SHA1 context with num = 256 and then issuing one update; AddressSanitizer reports a wrapped copy: ERROR: AddressSanitizer: negative-size-param: (size=-192) #0 memcpy #1 SHA1_Update #2 TPM_SHA1UpdateCmd tpm_crypto.c:2028 Reject num >= SHA_CBLOCK with TPM_BAD_PARAMETER and free the partially loaded context, the same num bound the TPM 2.0 hash-state unmarshalers already enforce. The FreeBL backend loads into a fixed-layout struct guarded by a flatten-size check and does not restore a write offset, so only the OpenSSL path is affected. Signed-off-by: sameera --- src/tpm12/tpm_crypto.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/tpm12/tpm_crypto.c b/src/tpm12/tpm_crypto.c index acf9b9259..ead5889e6 100644 --- a/src/tpm12/tpm_crypto.c +++ b/src/tpm12/tpm_crypto.c @@ -2136,6 +2136,20 @@ TPM_RESULT TPM_Sha1Context_Load(void **context, if ((rc== 0) && contextPresent) { rc = TPM_Load32(&(sha_ctx->num), stream, stream_size); } + /* 'num' is the number of bytes buffered in sha_ctx->data and is used by + OpenSSL as the write offset in SHA1_Update()/SHA1_Final(); it must be + less than the block size, else a later update walks off the context. */ + if ((rc== 0) && contextPresent) { + if (sha_ctx->num >= SHA_CBLOCK) { + printf("TPM_Sha1Context_Load: Error, num %u out of range\n", + sha_ctx->num); + rc = TPM_BAD_PARAMETER; + } + } + if ((rc != 0) && contextPresent && (*context != NULL)) { + free(*context); + *context = NULL; + } return rc; }