From 0fa345ecd9004d1f6b3d1b176ccae234ee4a41b6 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Thu, 16 Mar 2023 14:51:11 -0400 Subject: [PATCH 1/3] TpmFailureMode: Add missing request size checks This is harmless, but it is inconsistent with the non-failure case. --- TPMCmd/tpm/src/support/TpmFail.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/TPMCmd/tpm/src/support/TpmFail.c b/TPMCmd/tpm/src/support/TpmFail.c index c31b809e..fb10cd4e 100644 --- a/TPMCmd/tpm/src/support/TpmFail.c +++ b/TPMCmd/tpm/src/support/TpmFail.c @@ -195,6 +195,8 @@ void TpmFailureMode(uint32_t inRequestSize, // IN: command buffer size goto FailureModeReturn; if(header.tag != TPM_ST_NO_SESSIONS || header.size < 10) goto FailureModeReturn; + if(header.size != inRequestSize || header.size > MAX_COMMAND_SIZE) + goto FailureModeReturn; switch(header.code) { case TPM_CC_GetTestResult: @@ -326,4 +328,4 @@ void UnmarshalFail(void* type, BYTE** buffer, INT32* size) NOT_REFERENCED(buffer); NOT_REFERENCED(size); FAIL(FATAL_ERROR_INTERNAL); -} \ No newline at end of file +} From 14837b04f21d18f30fcfe092003607d492b62b46 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sat, 9 Dec 2023 13:51:21 -0500 Subject: [PATCH 2/3] TpmFailureMode: treat request shorter than 10 bytes as empty This doesn't change the observable behavior of the TPM. However, when combined with the previous commit ("TpmFailureMode: Add missing request size checks"), it guarantees that the unmarshalling functions in TpmFail.c always succeed and could be changed to return void, which would simplify the code. --- TPMCmd/tpm/src/support/TpmFail.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/TPMCmd/tpm/src/support/TpmFail.c b/TPMCmd/tpm/src/support/TpmFail.c index fb10cd4e..35b7b9fe 100644 --- a/TPMCmd/tpm/src/support/TpmFail.c +++ b/TPMCmd/tpm/src/support/TpmFail.c @@ -184,8 +184,9 @@ void TpmFailureMode(uint32_t inRequestSize, // IN: command buffer size UINT8* buffer = inRequest; INT32 size = inRequestSize; - // If there is no command buffer, then just return TPM_RC_FAILURE - if(inRequestSize == 0 || inRequest == NULL) + // If there is no command buffer or it is too small, + // then just return TPM_RC_FAILURE + if(inRequestSize < 10 || inRequest == NULL) goto FailureModeReturn; // If the header is not correct for TPM2_GetCapability() or // TPM2_GetTestResult() then just return the in failure mode response; From 039084afa090276825ccc2c4104b4965db383fcb Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sat, 9 Dec 2023 14:30:40 -0500 Subject: [PATCH 3/3] Remove unneeded checks in failure mode unmarshaling This simplifies the code. --- TPMCmd/tpm/src/support/TpmFail.c | 46 +++++++++----------------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/TPMCmd/tpm/src/support/TpmFail.c b/TPMCmd/tpm/src/support/TpmFail.c index 35b7b9fe..f4de5a6f 100644 --- a/TPMCmd/tpm/src/support/TpmFail.c +++ b/TPMCmd/tpm/src/support/TpmFail.c @@ -94,26 +94,6 @@ static INT32 MarshalUint32(UINT32 integer, BYTE** buffer) return 4; } -//***Unmarshal32() -static BOOL Unmarshal32(UINT32* target, BYTE** buffer, INT32* size) -{ - if((*size -= 4) < 0) - return FALSE; - *target = BYTE_ARRAY_TO_UINT32(*buffer); - *buffer += 4; - return TRUE; -} - -//***Unmarshal16() -static BOOL Unmarshal16(UINT16* target, BYTE** buffer, INT32* size) -{ - if((*size -= 2) < 0) - return FALSE; - *target = BYTE_ARRAY_TO_UINT16(*buffer); - *buffer += 2; - return TRUE; -} - //** Public Functions //*** SetForceFailureMode() @@ -182,7 +162,6 @@ void TpmFailureMode(uint32_t inRequestSize, // IN: command buffer size UINT32 pt; // unmarshaled property type UINT32 count; // unmarshaled property count UINT8* buffer = inRequest; - INT32 size = inRequestSize; // If there is no command buffer or it is too small, // then just return TPM_RC_FAILURE @@ -190,13 +169,11 @@ void TpmFailureMode(uint32_t inRequestSize, // IN: command buffer size goto FailureModeReturn; // If the header is not correct for TPM2_GetCapability() or // TPM2_GetTestResult() then just return the in failure mode response; - if(!(Unmarshal16(&header.tag, &buffer, &size) - && Unmarshal32(&header.size, &buffer, &size) - && Unmarshal32(&header.code, &buffer, &size))) - goto FailureModeReturn; - if(header.tag != TPM_ST_NO_SESSIONS || header.size < 10) - goto FailureModeReturn; - if(header.size != inRequestSize || header.size > MAX_COMMAND_SIZE) + header.tag = BYTE_ARRAY_TO_UINT16(buffer); + header.size = BYTE_ARRAY_TO_UINT32(buffer + 2); + header.code = BYTE_ARRAY_TO_UINT32(buffer + 6); + buffer += 10; + if(header.tag != TPM_ST_NO_SESSIONS || header.size != inRequestSize) goto FailureModeReturn; switch(header.code) { @@ -217,12 +194,13 @@ void TpmFailureMode(uint32_t inRequestSize, // IN: command buffer size case TPM_CC_GetCapability: // make sure that the size of the command is exactly the size // returned for the capability, property, and count - if(header.size != (10 + (3 * sizeof(UINT32))) - // also verify that this is requesting TPM properties - || !Unmarshal32(&capability, &buffer, &size) - || capability != TPM_CAP_TPM_PROPERTIES - || !Unmarshal32(&pt, &buffer, &size) - || !Unmarshal32(&count, &buffer, &size)) + if(header.size != (10 + (3 * sizeof(UINT32)))) + goto FailureModeReturn; + capability = BYTE_ARRAY_TO_UINT32(buffer); + pt = BYTE_ARRAY_TO_UINT32(buffer + 4); + count = BYTE_ARRAY_TO_UINT32(buffer + 8); + // also verify that this is requesting TPM properties + if(capability != TPM_CAP_TPM_PROPERTIES) goto FailureModeReturn; if(count > 0)