-
Notifications
You must be signed in to change notification settings - Fork 505
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
EDK2 is susceptible to a vulnerability in the Tcg2MeasurePeImage() function, allowing a user to trigger a heap buffer overflow via a local network. Successful exploitation of this vulnerability may result in a compromise of confidentiality, integrity, and/or availability. References: https://nvd.nist.gov/vuln/detail/CVE-2022-36764 Upstream-patches: tianocore/edk2@c7b2794 tianocore/edk2@0d341c0 tianocore/edk2@8f6d343 Signed-off-by: Soumya Sambu <soumya.sambu@windriver.com>
- Loading branch information
1 parent
26db245
commit aba1482
Showing
4 changed files
with
603 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,271 @@ | ||
From c7b27944218130cca3bbb20314ba5b88b5de4aa4 Mon Sep 17 00:00:00 2001 | ||
From: "Douglas Flick [MSFT]" <doug.edk2@gmail.com> | ||
Date: Fri, 12 Jan 2024 02:16:04 +0800 | ||
Subject: [PATCH] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE | ||
2022-36764 | ||
|
||
This commit contains the patch files and tests for DxeTpm2MeasureBootLib | ||
CVE 2022-36764. | ||
|
||
Cc: Jiewen Yao <jiewen.yao@intel.com> | ||
|
||
Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com> | ||
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> | ||
|
||
CVE: CVE-2022-36764 | ||
|
||
Upstream-Status: Backport [https://github.com/tianocore/edk2/commit/c7b27944218130cca3bbb20314ba5b88b5de4aa4] | ||
|
||
Signed-off-by: Soumya Sambu <soumya.sambu@windriver.com> | ||
--- | ||
.../DxeTpm2MeasureBootLib.c | 12 ++-- | ||
.../DxeTpm2MeasureBootLibSanitization.c | 46 +++++++++++++- | ||
.../DxeTpm2MeasureBootLibSanitization.h | 28 ++++++++- | ||
.../DxeTpm2MeasureBootLibSanitizationTest.c | 60 ++++++++++++++++--- | ||
4 files changed, 131 insertions(+), 15 deletions(-) | ||
|
||
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c | ||
index 0475103d6e..714cc8e03e 100644 | ||
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c | ||
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c | ||
@@ -378,7 +378,6 @@ Exit: | ||
@retval EFI_OUT_OF_RESOURCES No enough resource to measure image. | ||
@retval EFI_UNSUPPORTED ImageType is unsupported or PE image is mal-format. | ||
@retval other error value | ||
- | ||
**/ | ||
EFI_STATUS | ||
EFIAPI | ||
@@ -405,6 +404,7 @@ Tcg2MeasurePeImage ( | ||
Status = EFI_UNSUPPORTED; | ||
ImageLoad = NULL; | ||
EventPtr = NULL; | ||
+ Tcg2Event = NULL; | ||
|
||
Tcg2Protocol = MeasureBootProtocols->Tcg2Protocol; | ||
CcProtocol = MeasureBootProtocols->CcProtocol; | ||
@@ -420,18 +420,22 @@ Tcg2MeasurePeImage ( | ||
} | ||
|
||
FilePathSize = (UINT32)GetDevicePathSize (FilePath); | ||
+ Status = SanitizePeImageEventSize (FilePathSize, &EventSize); | ||
+ if (EFI_ERROR (Status)) { | ||
+ return EFI_UNSUPPORTED; | ||
+ } | ||
|
||
// | ||
// Determine destination PCR by BootPolicy | ||
// | ||
- EventSize = sizeof (*ImageLoad) - sizeof (ImageLoad->DevicePath) + FilePathSize; | ||
- EventPtr = AllocateZeroPool (EventSize + sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event)); | ||
+ // from a malicious GPT disk partition | ||
+ EventPtr = AllocateZeroPool (EventSize); | ||
if (EventPtr == NULL) { | ||
return EFI_OUT_OF_RESOURCES; | ||
} | ||
|
||
Tcg2Event = (EFI_TCG2_EVENT *)EventPtr; | ||
- Tcg2Event->Size = EventSize + sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event); | ||
+ Tcg2Event->Size = EventSize; | ||
Tcg2Event->Header.HeaderSize = sizeof (EFI_TCG2_EVENT_HEADER); | ||
Tcg2Event->Header.HeaderVersion = EFI_TCG2_EVENT_HEADER_VERSION; | ||
ImageLoad = (EFI_IMAGE_LOAD_EVENT *)Tcg2Event->Event; | ||
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c | ||
index e2309655d3..2a4d52c6d5 100644 | ||
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c | ||
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c | ||
@@ -151,7 +151,7 @@ SanitizeEfiPartitionTableHeader ( | ||
} | ||
|
||
/** | ||
- This function will validate that the allocation size from the primary header is sane | ||
+ This function will validate that the allocation size from the primary header is sane | ||
It will check the following: | ||
- AllocationSize does not overflow | ||
|
||
@@ -273,3 +273,47 @@ SanitizePrimaryHeaderGptEventSize ( | ||
|
||
return EFI_SUCCESS; | ||
} | ||
+ | ||
+/** | ||
+ This function will validate that the PeImage Event Size from the loaded image is sane | ||
+ It will check the following: | ||
+ - EventSize does not overflow | ||
+ | ||
+ @param[in] FilePathSize - Size of the file path. | ||
+ @param[out] EventSize - Pointer to the event size. | ||
+ | ||
+ @retval EFI_SUCCESS | ||
+ The event size is valid. | ||
+ | ||
+ @retval EFI_OUT_OF_RESOURCES | ||
+ Overflow would have occurred. | ||
+ | ||
+ @retval EFI_INVALID_PARAMETER | ||
+ One of the passed parameters was invalid. | ||
+**/ | ||
+EFI_STATUS | ||
+SanitizePeImageEventSize ( | ||
+ IN UINT32 FilePathSize, | ||
+ OUT UINT32 *EventSize | ||
+ ) | ||
+{ | ||
+ EFI_STATUS Status; | ||
+ | ||
+ // Replacing logic: | ||
+ // sizeof (*ImageLoad) - sizeof (ImageLoad->DevicePath) + FilePathSize; | ||
+ Status = SafeUint32Add (OFFSET_OF (EFI_IMAGE_LOAD_EVENT, DevicePath), FilePathSize, EventSize); | ||
+ if (EFI_ERROR (Status)) { | ||
+ DEBUG ((DEBUG_ERROR, "EventSize would overflow!\n")); | ||
+ return EFI_BAD_BUFFER_SIZE; | ||
+ } | ||
+ | ||
+ // Replacing logic: | ||
+ // EventSize + sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event) | ||
+ Status = SafeUint32Add (*EventSize, OFFSET_OF (EFI_TCG2_EVENT, Event), EventSize); | ||
+ if (EFI_ERROR (Status)) { | ||
+ DEBUG ((DEBUG_ERROR, "EventSize would overflow!\n")); | ||
+ return EFI_BAD_BUFFER_SIZE; | ||
+ } | ||
+ | ||
+ return EFI_SUCCESS; | ||
+} | ||
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h | ||
index 048b738987..8f72ba4240 100644 | ||
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h | ||
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h | ||
@@ -9,6 +9,9 @@ | ||
Tcg2MeasureGptTable() function will receive untrusted GPT partition table, and parse | ||
partition data carefully. | ||
|
||
+ Tcg2MeasurePeImage() function will accept untrusted PE/COFF image and validate its | ||
+ data structure within this image buffer before use. | ||
+ | ||
Copyright (c) Microsoft Corporation.<BR> | ||
SPDX-License-Identifier: BSD-2-Clause-Patent | ||
|
||
@@ -110,4 +113,27 @@ SanitizePrimaryHeaderGptEventSize ( | ||
OUT UINT32 *EventSize | ||
); | ||
|
||
-#endif // DXE_TPM2_MEASURE_BOOT_LIB_SANITATION_ | ||
+/** | ||
+ This function will validate that the PeImage Event Size from the loaded image is sane | ||
+ It will check the following: | ||
+ - EventSize does not overflow | ||
+ | ||
+ @param[in] FilePathSize - Size of the file path. | ||
+ @param[out] EventSize - Pointer to the event size. | ||
+ | ||
+ @retval EFI_SUCCESS | ||
+ The event size is valid. | ||
+ | ||
+ @retval EFI_OUT_OF_RESOURCES | ||
+ Overflow would have occurred. | ||
+ | ||
+ @retval EFI_INVALID_PARAMETER | ||
+ One of the passed parameters was invalid. | ||
+**/ | ||
+EFI_STATUS | ||
+SanitizePeImageEventSize ( | ||
+ IN UINT32 FilePathSize, | ||
+ OUT UINT32 *EventSize | ||
+ ); | ||
+ | ||
+#endif // DXE_TPM2_MEASURE_BOOT_LIB_VALIDATION_ | ||
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c | ||
index 3eb9763e3c..820e99aeb9 100644 | ||
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c | ||
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c | ||
@@ -72,10 +72,10 @@ TestSanitizeEfiPartitionTableHeader ( | ||
PrimaryHeader.Header.Revision = DEFAULT_PRIMARY_TABLE_HEADER_REVISION; | ||
PrimaryHeader.Header.HeaderSize = sizeof (EFI_PARTITION_TABLE_HEADER); | ||
PrimaryHeader.MyLBA = 1; | ||
- PrimaryHeader.AlternateLBA = 2; | ||
- PrimaryHeader.FirstUsableLBA = 3; | ||
- PrimaryHeader.LastUsableLBA = 4; | ||
- PrimaryHeader.PartitionEntryLBA = 5; | ||
+ PrimaryHeader.PartitionEntryLBA = 2; | ||
+ PrimaryHeader.AlternateLBA = 3; | ||
+ PrimaryHeader.FirstUsableLBA = 4; | ||
+ PrimaryHeader.LastUsableLBA = 5; | ||
PrimaryHeader.NumberOfPartitionEntries = DEFAULT_PRIMARY_TABLE_HEADER_NUMBER_OF_PARTITION_ENTRIES; | ||
PrimaryHeader.SizeOfPartitionEntry = DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY; | ||
PrimaryHeader.PartitionEntryArrayCRC32 = 0; // Purposely invalid | ||
@@ -187,11 +187,6 @@ TestSanitizePrimaryHeaderGptEventSize ( | ||
EFI_STATUS Status; | ||
EFI_PARTITION_TABLE_HEADER PrimaryHeader; | ||
UINTN NumberOfPartition; | ||
- EFI_GPT_DATA *GptData; | ||
- EFI_TCG2_EVENT *Tcg2Event; | ||
- | ||
- Tcg2Event = NULL; | ||
- GptData = NULL; | ||
|
||
// Test that a normal PrimaryHeader passes validation | ||
PrimaryHeader.NumberOfPartitionEntries = 5; | ||
@@ -225,6 +220,52 @@ TestSanitizePrimaryHeaderGptEventSize ( | ||
return UNIT_TEST_PASSED; | ||
} | ||
|
||
+/** | ||
+ This function tests the SanitizePeImageEventSize function. | ||
+ It's intent is to test that the untrusted input from a file path when generating a | ||
+ EFI_IMAGE_LOAD_EVENT structure will not cause an overflow when calculating | ||
+ the event size when allocating space | ||
+ | ||
+ @param[in] Context The unit test context. | ||
+ | ||
+ @retval UNIT_TEST_PASSED The test passed. | ||
+ @retval UNIT_TEST_ERROR_TEST_FAILED The test failed. | ||
+**/ | ||
+UNIT_TEST_STATUS | ||
+EFIAPI | ||
+TestSanitizePeImageEventSize ( | ||
+ IN UNIT_TEST_CONTEXT Context | ||
+ ) | ||
+{ | ||
+ UINT32 EventSize; | ||
+ UINTN ExistingLogicEventSize; | ||
+ UINT32 FilePathSize; | ||
+ EFI_STATUS Status; | ||
+ | ||
+ FilePathSize = 255; | ||
+ | ||
+ // Test that a normal PE image passes validation | ||
+ Status = SanitizePeImageEventSize (FilePathSize, &EventSize); | ||
+ UT_ASSERT_EQUAL (Status, EFI_SUCCESS); | ||
+ | ||
+ // Test that the event size is correct compared to the existing logic | ||
+ ExistingLogicEventSize = OFFSET_OF (EFI_IMAGE_LOAD_EVENT, DevicePath) + FilePathSize; | ||
+ ExistingLogicEventSize += OFFSET_OF (EFI_TCG2_EVENT, Event); | ||
+ | ||
+ if (EventSize != ExistingLogicEventSize) { | ||
+ UT_LOG_ERROR ("SanitizePeImageEventSize returned an incorrect event size. Expected %u, got %u\n", ExistingLogicEventSize, EventSize); | ||
+ return UNIT_TEST_ERROR_TEST_FAILED; | ||
+ } | ||
+ | ||
+ // Test that the event size may not overflow | ||
+ Status = SanitizePeImageEventSize (MAX_UINT32, &EventSize); | ||
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE); | ||
+ | ||
+ DEBUG ((DEBUG_INFO, "%a: Test passed\n", __func__)); | ||
+ | ||
+ return UNIT_TEST_PASSED; | ||
+} | ||
+ | ||
// *--------------------------------------------------------------------* | ||
// * Unit Test Code Main Function | ||
// *--------------------------------------------------------------------* | ||
@@ -267,6 +308,7 @@ UefiTestMain ( | ||
AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Validating EFI Partition Table", "Common.Tcg2MeasureBootLibValidation", TestSanitizeEfiPartitionTableHeader, NULL, NULL, NULL); | ||
AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Primary header gpt event checks for overflow", "Common.Tcg2MeasureBootLibValidation", TestSanitizePrimaryHeaderAllocationSize, NULL, NULL, NULL); | ||
AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Primary header allocation size checks for overflow", "Common.Tcg2MeasureBootLibValidation", TestSanitizePrimaryHeaderGptEventSize, NULL, NULL, NULL); | ||
+ AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests PE Image and FileSize checks for overflow", "Common.Tcg2MeasureBootLibValidation", TestSanitizePeImageEventSize, NULL, NULL, NULL); | ||
|
||
Status = RunAllTestSuites (Framework); | ||
|
||
-- | ||
2.40.0 | ||
|
Oops, something went wrong.