From d0587f1b8eaa80efe89cab18ec745565d59e7f2d Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Thu, 11 Jul 2024 15:46:24 -0400 Subject: [PATCH 1/8] tpm2: Preserve more *target and restore them if neede (Coverity) Signed-off-by: Stefan Berger --- src/tpm2/Unmarshal.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/tpm2/Unmarshal.c b/src/tpm2/Unmarshal.c index e2395c3d6..5e31eb49f 100644 --- a/src/tpm2/Unmarshal.c +++ b/src/tpm2/Unmarshal.c @@ -1621,6 +1621,7 @@ TPMU_HA_Unmarshal(TPMU_HA *target, BYTE **buffer, INT32 *size, UINT32 selector) TPM_RC TPMT_HA_Unmarshal(TPMT_HA *target, BYTE **buffer, INT32 *size, BOOL allowNull) { + TPMT_HA orig_target = *target; // libtpms added TPM_RC rc = TPM_RC_SUCCESS; if (rc == TPM_RC_SUCCESS) { @@ -1629,6 +1630,9 @@ TPMT_HA_Unmarshal(TPMT_HA *target, BYTE **buffer, INT32 *size, BOOL allowNull) if (rc == TPM_RC_SUCCESS) { rc = TPMU_HA_Unmarshal(&target->digest, buffer, size, target->hashAlg); } + if (rc != TPM_RC_SUCCESS) { // libtpms added begin + *target = orig_target; + } // libtpms added end return rc; } @@ -2891,6 +2895,7 @@ TPM2B_SENSITIVE_DATA_Unmarshal(TPM2B_SENSITIVE_DATA *target, BYTE **buffer, INT3 TPM_RC TPMS_SENSITIVE_CREATE_Unmarshal(TPMS_SENSITIVE_CREATE *target, BYTE **buffer, INT32 *size) { + TPMS_SENSITIVE_CREATE orig_target = *target; // libtpms added TPM_RC rc = TPM_RC_SUCCESS; if (rc == TPM_RC_SUCCESS) { @@ -2899,6 +2904,9 @@ TPMS_SENSITIVE_CREATE_Unmarshal(TPMS_SENSITIVE_CREATE *target, BYTE **buffer, IN if (rc == TPM_RC_SUCCESS) { rc = TPM2B_SENSITIVE_DATA_Unmarshal(&target->data, buffer, size); } + if (rc != TPM_RC_SUCCESS) { // libtpms added begin + *target = orig_target; + } // libtpms added end return rc; } @@ -3199,6 +3207,7 @@ TPMU_SIG_SCHEME_Unmarshal(TPMU_SIG_SCHEME *target, BYTE **buffer, INT32 *size, U TPM_RC TPMT_SIG_SCHEME_Unmarshal(TPMT_SIG_SCHEME *target, BYTE **buffer, INT32 *size, BOOL allowNull) { + TPMT_SIG_SCHEME orig_target = *target; // libtpms added TPM_RC rc = TPM_RC_SUCCESS; if (rc == TPM_RC_SUCCESS) { @@ -3207,6 +3216,9 @@ TPMT_SIG_SCHEME_Unmarshal(TPMT_SIG_SCHEME *target, BYTE **buffer, INT32 *size, B if (rc == TPM_RC_SUCCESS) { rc = TPMU_SIG_SCHEME_Unmarshal(&target->details, buffer, size, target->scheme); } + if (rc != TPM_RC_SUCCESS) { // libtpms added begin + *target = orig_target; + } // libtpms added end return rc; } @@ -3661,6 +3673,7 @@ TPM2B_ECC_PARAMETER_Unmarshal(TPM2B_ECC_PARAMETER *target, BYTE **buffer, INT32 TPM_RC TPMS_ECC_POINT_Unmarshal(TPMS_ECC_POINT *target, BYTE **buffer, INT32 *size) { + TPMS_ECC_POINT orig_target = *target; // libtpms added TPM_RC rc = TPM_RC_SUCCESS; if (rc == TPM_RC_SUCCESS) { @@ -3669,6 +3682,9 @@ TPMS_ECC_POINT_Unmarshal(TPMS_ECC_POINT *target, BYTE **buffer, INT32 *size) if (rc == TPM_RC_SUCCESS) { rc = TPM2B_ECC_PARAMETER_Unmarshal(&target->y, buffer, size); } + if (rc != TPM_RC_SUCCESS) { // libtpms added being + *target = orig_target; + } // libtpms added end return rc; } @@ -3801,6 +3817,7 @@ TPMI_ECC_CURVE_Unmarshal(TPMI_ECC_CURVE *target, BYTE **buffer, INT32 *size) TPM_RC TPMT_ECC_SCHEME_Unmarshal(TPMT_ECC_SCHEME *target, BYTE **buffer, INT32 *size, BOOL allowNull) { + TPMT_ECC_SCHEME orig_target = *target; // libtpms added TPM_RC rc = TPM_RC_SUCCESS; if (rc == TPM_RC_SUCCESS) { @@ -3809,6 +3826,9 @@ TPMT_ECC_SCHEME_Unmarshal(TPMT_ECC_SCHEME *target, BYTE **buffer, INT32 *size, B if (rc == TPM_RC_SUCCESS) { rc = TPMU_ASYM_SCHEME_Unmarshal(&target->details, buffer, size, target->scheme); } + if (rc != TPM_RC_SUCCESS) { // libtpms added begin + *target = orig_target; + } // libtpms added end return rc; } @@ -4107,6 +4127,7 @@ TPMS_RSA_PARMS_Unmarshal(TPMS_RSA_PARMS *target, BYTE **buffer, INT32 *size) TPM_RC TPMS_ECC_PARMS_Unmarshal(TPMS_ECC_PARMS *target, BYTE **buffer, INT32 *size) { + TPMS_ECC_PARMS orig_target = *target; // libtpms added TPM_RC rc = TPM_RC_SUCCESS; if (rc == TPM_RC_SUCCESS) { @@ -4121,6 +4142,9 @@ TPMS_ECC_PARMS_Unmarshal(TPMS_ECC_PARMS *target, BYTE **buffer, INT32 *size) if (rc == TPM_RC_SUCCESS) { rc = TPMT_KDF_SCHEME_Unmarshal(&target->kdf, buffer, size, YES); } + if (rc != TPM_RC_SUCCESS) { // libtpms added begin + *target = orig_target; + } // libtpms added end return rc; } From 3da8b81d74473efbb0b67269e77c9d95158e3a55 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Thu, 11 Jul 2024 16:04:03 -0400 Subject: [PATCH 2/8] tpm2: Initialize eccPublic before passing to TPMS_ECC_POINT_Unmarshal (Coverity) Resolve the following Coverity complaint: "Using uninitialized value eccPublic when calling TPMS_ECC_POINT_Unmarshal." Signed-off-by: Stefan Berger --- src/tpm2/CryptUtil.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tpm2/CryptUtil.c b/src/tpm2/CryptUtil.c index 19f5cd6dc..da4230e33 100644 --- a/src/tpm2/CryptUtil.c +++ b/src/tpm2/CryptUtil.c @@ -639,6 +639,9 @@ CryptSecretDecrypt( TPMS_ECC_POINT eccSecret; BYTE *buffer = secret->t.secret; INT32 size = secret->t.size; + + MemorySet(&eccPublic, 0, sizeof(eccPublic)); // libtpms added: Coverity + // Retrieve ECC point from secret buffer result = TPMS_ECC_POINT_Unmarshal(&eccPublic, &buffer, &size); if(result == TPM_RC_SUCCESS) From ad5017d502e84c590136cc3644506925a3180db2 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Thu, 11 Jul 2024 16:15:39 -0400 Subject: [PATCH 3/8] tpm2: Remove assigned to value to offset because it is unused (Coverity) Resolve the following Coverity complaint by removing assignment to offset: "Assigning value from offset + 148UL to offset here, but that stored value is overwritten before it can be used." Signed-off-by: Stefan Berger --- src/tpm2/NVMarshal.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tpm2/NVMarshal.c b/src/tpm2/NVMarshal.c index c7cd1e0e9..e8b9c6e3b 100644 --- a/src/tpm2/NVMarshal.c +++ b/src/tpm2/NVMarshal.c @@ -4488,9 +4488,8 @@ USER_NVRAM_Display(const char *msg) fprintf(stderr, " (NV_INDEX) "); /* NV_INDEX has the index again at offset 0! */ NvReadNvIndexInfo(entryRef + offset, &nvi); - offset += sizeof(nvi); datasize = entrysize - sizeof(UINT32) - sizeof(nvi); - fprintf(stderr, " datasize: %u\n",datasize); + fprintf(stderr, " datasize: %u\n", datasize); break; break; case TPM_HT_PERSISTENT: From ac29c0f19b9181164886137028a85f91dd5ea5e0 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Thu, 11 Jul 2024 16:23:32 -0400 Subject: [PATCH 4/8] tpm2: Address a possible unsigned integer underflow (Coverity) Resolve the following Coverity complaint: "Expression command->sessionNum - 1U, which is equal to 4294967295, where command->sessionNum is known to be equal to 0, underflows the type that receives it, an unsigned integer 32 bits wide." Signed-off-by: Stefan Berger --- src/tpm2/SessionProcess.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tpm2/SessionProcess.c b/src/tpm2/SessionProcess.c index c379e41db..eb40b8ef2 100644 --- a/src/tpm2/SessionProcess.c +++ b/src/tpm2/SessionProcess.c @@ -1422,6 +1422,8 @@ ParseSessionBuffer( // Note: for all the TPM 2.0 commands, handles requiring // authorization come first in a command input and there are only ever // two values requiring authorization + if(command->sessionNum == 0) // libtpms added begin (Coverity 1550499) + return TPM_RC_AUTH_MISSING; // libtpms added end if(i > (command->sessionNum - 1)) return TPM_RC_AUTH_MISSING; // Record the handle associated with the authorization session From 363619b0934753e829604bad8fb5ba21d30dd953 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Thu, 11 Jul 2024 16:37:57 -0400 Subject: [PATCH 5/8] tpm2: Filter bad input values to avoid underflow in FindNthSetBit (Coverity) Address the following Coverity complaint (1550494) by filtering out bad input values: "Expression i--, which is equal to 65535, where i is known to be equal to 0, underflows the type that receives it, an unsigned integer 16 bits wide." aSize is typcially 2048 and n is always >= 1 per the input parameter. Therefore no side-effects are expected from this filter. Signed-off-by: Stefan Berger --- src/tpm2/crypto/openssl/CryptPrimeSieve.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tpm2/crypto/openssl/CryptPrimeSieve.c b/src/tpm2/crypto/openssl/CryptPrimeSieve.c index 8c4e52b04..1e8cea105 100644 --- a/src/tpm2/crypto/openssl/CryptPrimeSieve.c +++ b/src/tpm2/crypto/openssl/CryptPrimeSieve.c @@ -215,6 +215,10 @@ FindNthSetBit( int retValue; UINT32 sum = 0; BYTE sel; + + if (n < 1 || aSize < 1) // libtpms added begin: Coverity 1550494 + return -1; // libtpms end + //find the bit for(i = 0; (i < (int)aSize) && (sum < n); i++) sum += BitsInByte(a[i]); From 9d759c134d6f4de73aeb455e807685cc3cee40b0 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Wed, 16 Feb 2022 11:49:11 -0500 Subject: [PATCH 6/8] tpm2: Address Coverity Issue by casting '1' before shift (CID 1470813) Cast the '1' to UINT64 before shifting it. Since the shift value is always below 32 it would have never exceeded the 32bit value it was using before the cast. Signed-off-by: Stefan Berger --- src/tpm2/NVMarshal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tpm2/NVMarshal.c b/src/tpm2/NVMarshal.c index e8b9c6e3b..9c9d7e1ff 100644 --- a/src/tpm2/NVMarshal.c +++ b/src/tpm2/NVMarshal.c @@ -806,7 +806,7 @@ pcrbanks_algs_active(const TPML_PCR_SELECTION *pcrAllocated) for(i = 0; i < pcrAllocated->count; i++) { for (j = 0; j < pcrAllocated->pcrSelections[i].sizeofSelect; j++) { if (pcrAllocated->pcrSelections[i].pcrSelect[j]) { - algs_active |= 1 << pcrAllocated->pcrSelections[i].hash; + algs_active |= ((UINT64)1 << pcrAllocated->pcrSelections[i].hash); break; } } From 412543e6dbb9229fc54ee7432dc5aed5921e71ba Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Wed, 12 Jul 2023 21:15:54 -0400 Subject: [PATCH 7/8] Insert assert ensuring *buflen != BUFLEN_EMPTY_BUFFER (Coverity) Address a false positive issue detect by Coverity (CID 1517797) about *buflen. Per this assignment of buflen cached_blobs[st].buflen = buffer ? buflen : BUFLEN_EMPTY_BUFFER; the following is true: If cached_blobs[].buffer is NULL then *buflen = BUFLEN_EMPTY_BUFFER If cached_blobs[].buffer is not NULL then *buflen != BUFLEN_EMPTY_BUFFER Signed-off-by: Stefan Berger --- src/tpm_library.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tpm_library.c b/src/tpm_library.c index 5dd8fa4ea..20153b7b4 100644 --- a/src/tpm_library.c +++ b/src/tpm_library.c @@ -636,6 +636,8 @@ TPM_RESULT CopyCachedState(enum TPMLIB_StateType st, *is_empty_buffer = (*buflen == BUFLEN_EMPTY_BUFFER); if (cached_blobs[st].buffer) { + assert(*buflen != BUFLEN_EMPTY_BUFFER); + *buffer = malloc(*buflen); if (!*buffer) { TPMLIB_LogError("Could not allocate %u bytes.\n", *buflen); From 89a276820b759104ab759ed7b1cb32f0bca27ccc Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Mon, 22 Jul 2024 11:06:39 -0400 Subject: [PATCH 8/8] Travis: Use swtpm's stable-0.9 branch for testing Since swtpm now depends on libtpms >= 0.10 use swtpm's stable-0.9 branch for testing. Signed-off-by: Stefan Berger --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 287758fa4..3d991e5df 100644 --- a/.travis.yml +++ b/.travis.yml @@ -46,7 +46,7 @@ matrix: ./autogen.sh ${CONFIG} && sudo make -j$(nproc) ${TARGET} && sudo make -j$(nproc) check && - git clone https://github.com/stefanberger/swtpm.git && + git clone -b stable-0.9 https://github.com/stefanberger/swtpm.git && pushd swtpm && sudo rm -rf /dev/tpm* && sudo apt -y install devscripts equivs python3-twisted expect @@ -67,7 +67,7 @@ matrix: ./autogen.sh ${CONFIG} && sudo make -j$(nproc) ${TARGET} && sudo make -j$(nproc) check && - git clone https://github.com/stefanberger/swtpm.git && + git clone -b stable-0.9 https://github.com/stefanberger/swtpm.git && pushd swtpm && sudo rm -rf /dev/tpm* && sudo apt -y install devscripts equivs python3-twisted expect