From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id A6807941915 for ; Fri, 12 Jan 2024 19:16:07 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=MZ2/M9OhH75udu9V5E2NlsyHc1F4PpYW+KUCrzl8I/I=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1705086966; v=1; b=E9RfwQAxtPlEHRg8pIozUJ/oKOGiWtPVKIYupXNJgu37c4IYvRaFms2ynp5+MchCIXO1P9JN 7TnZSYrwZao6zN0NjTMajisUETSIR+LXdS5WxBnRQSJLmg1Wccnou9TmKWvAbZUPQIbrBqpHdFJ HgtjQ7M0ZqV1QdEkRw1ldMC0= X-Received: by 127.0.0.2 with SMTP id XGdSYY7687511xkqFqfExvUP; Fri, 12 Jan 2024 11:16:06 -0800 X-Received: from mail-pl1-f180.google.com (mail-pl1-f180.google.com [209.85.214.180]) by mx.groups.io with SMTP id smtpd.web10.18730.1704997019038243760 for ; Thu, 11 Jan 2024 10:16:59 -0800 X-Received: by mail-pl1-f180.google.com with SMTP id d9443c01a7336-1d40eec5e12so46282275ad.1 for ; Thu, 11 Jan 2024 10:16:59 -0800 (PST) X-Gm-Message-State: LritEkO7f5A8C7gzWcfa2rFxx7686176AA= X-Google-Smtp-Source: AGHT+IHKZ1M8C9NXnTUWK68f+m0oIfflHA0my132zQORrcSyiIydBJCJ7ByGv50G6nzZlIzth90SAw== X-Received: by 2002:a17:903:1306:b0:1d5:6a34:fb5c with SMTP id iy6-20020a170903130600b001d56a34fb5cmr124645plb.101.1704997017703; Thu, 11 Jan 2024 10:16:57 -0800 (PST) X-Received: from localhost.localdomain ([131.107.1.208]) by smtp.gmail.com with ESMTPSA id kd13-20020a17090313cd00b001d4752f5403sm1453414plb.206.2024.01.11.10.16.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Jan 2024 10:16:57 -0800 (PST) From: "Doug Flick via groups.io" To: devel@edk2.groups.io Cc: "Douglas Flick [MSFT]" , Jiewen Yao Subject: [edk2-devel] [PATCH 4/6] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE 2022-36764 Date: Thu, 11 Jan 2024 10:16:04 -0800 Message-ID: <32af82809ace69a2711f0096cee91e7a7ad3aaae.1704996627.git.doug.edk2@gmail.com> In-Reply-To: References: MIME-Version: 1.0 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,dougflick@microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=E9RfwQAx; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io This commit contains the patch files and tests for DxeTpm2MeasureBootLib CVE 2022-36764. Cc: Jiewen Yao Signed-off-by: Doug Flick [MSFT] --- .../DxeTpm2MeasureBootLibSanitization.h | 28 ++++++++- .../DxeTpm2MeasureBootLib.c | 12 ++-- .../DxeTpm2MeasureBootLibSanitization.c | 46 +++++++++++++- .../DxeTpm2MeasureBootLibSanitizationTest.c | 60 ++++++++++++++++--- 4 files changed, 131 insertions(+), 15 deletions(-) diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLi= bSanitization.h b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureB= ootLibSanitization.h index 048b73898744..8f72ba42401f 100644 --- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSaniti= zation.h +++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSaniti= zation.h @@ -9,6 +9,9 @@ Tcg2MeasureGptTable() function will receive untrusted GPT partition tabl= e, and parse=0D partition data carefully.=0D =0D + Tcg2MeasurePeImage() function will accept untrusted PE/COFF image and va= lidate its=0D + data structure within this image buffer before use.=0D +=0D Copyright (c) Microsoft Corporation.
=0D SPDX-License-Identifier: BSD-2-Clause-Patent=0D =0D @@ -110,4 +113,27 @@ SanitizePrimaryHeaderGptEventSize ( OUT UINT32 *EventSize=0D );=0D =0D -#endif // DXE_TPM2_MEASURE_BOOT_LIB_SANITATION_=0D +/**=0D + This function will validate that the PeImage Event Size from the loaded = image is sane=0D + It will check the following:=0D + - EventSize does not overflow=0D +=0D + @param[in] FilePathSize - Size of the file path.=0D + @param[out] EventSize - Pointer to the event size.=0D +=0D + @retval EFI_SUCCESS=0D + The event size is valid.=0D +=0D + @retval EFI_OUT_OF_RESOURCES=0D + Overflow would have occurred.=0D +=0D + @retval EFI_INVALID_PARAMETER=0D + One of the passed parameters was invalid.=0D +**/=0D +EFI_STATUS=0D +SanitizePeImageEventSize (=0D + IN UINT32 FilePathSize,=0D + OUT UINT32 *EventSize=0D + );=0D +=0D +#endif // DXE_TPM2_MEASURE_BOOT_LIB_VALIDATION_=0D diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLi= b.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c index 0475103d6ef8..714cc8e03e80 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.=0D @retval EFI_UNSUPPORTED ImageType is unsupported or PE image is m= al-format.=0D @retval other error value=0D -=0D **/=0D EFI_STATUS=0D EFIAPI=0D @@ -405,6 +404,7 @@ Tcg2MeasurePeImage ( Status =3D EFI_UNSUPPORTED;=0D ImageLoad =3D NULL;=0D EventPtr =3D NULL;=0D + Tcg2Event =3D NULL;=0D =0D Tcg2Protocol =3D MeasureBootProtocols->Tcg2Protocol;=0D CcProtocol =3D MeasureBootProtocols->CcProtocol;=0D @@ -420,18 +420,22 @@ Tcg2MeasurePeImage ( }=0D =0D FilePathSize =3D (UINT32)GetDevicePathSize (FilePath);=0D + Status =3D SanitizePeImageEventSize (FilePathSize, &EventSize);=0D + if (EFI_ERROR (Status)) {=0D + return EFI_UNSUPPORTED;=0D + }=0D =0D //=0D // Determine destination PCR by BootPolicy=0D //=0D - EventSize =3D sizeof (*ImageLoad) - sizeof (ImageLoad->DevicePath) + Fil= ePathSize;=0D - EventPtr =3D AllocateZeroPool (EventSize + sizeof (EFI_TCG2_EVENT) - si= zeof (Tcg2Event->Event));=0D + // from a malicious GPT disk partition=0D + EventPtr =3D AllocateZeroPool (EventSize);=0D if (EventPtr =3D=3D NULL) {=0D return EFI_OUT_OF_RESOURCES;=0D }=0D =0D Tcg2Event =3D (EFI_TCG2_EVENT *)EventPtr;=0D - Tcg2Event->Size =3D EventSize + sizeof (EFI_TCG2_EVENT) = - sizeof (Tcg2Event->Event);=0D + Tcg2Event->Size =3D EventSize;=0D Tcg2Event->Header.HeaderSize =3D sizeof (EFI_TCG2_EVENT_HEADER);=0D Tcg2Event->Header.HeaderVersion =3D EFI_TCG2_EVENT_HEADER_VERSION;=0D ImageLoad =3D (EFI_IMAGE_LOAD_EVENT *)Tcg2Event->E= vent;=0D diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLi= bSanitization.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureB= ootLibSanitization.c index e2309655d384..2a4d52c6d5cf 100644 --- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSaniti= zation.c +++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSaniti= zation.c @@ -151,7 +151,7 @@ SanitizeEfiPartitionTableHeader ( }=0D =0D /**=0D - This function will validate that the allocation size from the primary he= ader is sane=0D + This function will validate that the allocation size from the primary hea= der is sane=0D It will check the following:=0D - AllocationSize does not overflow=0D =0D @@ -273,3 +273,47 @@ SanitizePrimaryHeaderGptEventSize ( =0D return EFI_SUCCESS;=0D }=0D +=0D +/**=0D + This function will validate that the PeImage Event Size from the loaded = image is sane=0D + It will check the following:=0D + - EventSize does not overflow=0D +=0D + @param[in] FilePathSize - Size of the file path.=0D + @param[out] EventSize - Pointer to the event size.=0D +=0D + @retval EFI_SUCCESS=0D + The event size is valid.=0D +=0D + @retval EFI_OUT_OF_RESOURCES=0D + Overflow would have occurred.=0D +=0D + @retval EFI_INVALID_PARAMETER=0D + One of the passed parameters was invalid.=0D +**/=0D +EFI_STATUS=0D +SanitizePeImageEventSize (=0D + IN UINT32 FilePathSize,=0D + OUT UINT32 *EventSize=0D + )=0D +{=0D + EFI_STATUS Status;=0D +=0D + // Replacing logic:=0D + // sizeof (*ImageLoad) - sizeof (ImageLoad->DevicePath) + FilePathSize;= =0D + Status =3D SafeUint32Add (OFFSET_OF (EFI_IMAGE_LOAD_EVENT, DevicePath), = FilePathSize, EventSize);=0D + if (EFI_ERROR (Status)) {=0D + DEBUG ((DEBUG_ERROR, "EventSize would overflow!\n"));=0D + return EFI_BAD_BUFFER_SIZE;=0D + }=0D +=0D + // Replacing logic:=0D + // EventSize + sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event)=0D + Status =3D SafeUint32Add (*EventSize, OFFSET_OF (EFI_TCG2_EVENT, Event),= EventSize);=0D + if (EFI_ERROR (Status)) {=0D + DEBUG ((DEBUG_ERROR, "EventSize would overflow!\n"));=0D + return EFI_BAD_BUFFER_SIZE;=0D + }=0D +=0D + return EFI_SUCCESS;=0D +}=0D diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/Dxe= Tpm2MeasureBootLibSanitizationTest.c b/SecurityPkg/Library/DxeTpm2MeasureBo= otLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c index 3eb9763e3c91..820e99aeb9b4 100644 --- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2Mea= sureBootLibSanitizationTest.c +++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2Mea= sureBootLibSanitizationTest.c @@ -72,10 +72,10 @@ TestSanitizeEfiPartitionTableHeader ( PrimaryHeader.Header.Revision =3D DEFAULT_PRIMARY_TABLE_HEADER_= REVISION;=0D PrimaryHeader.Header.HeaderSize =3D sizeof (EFI_PARTITION_TABLE_H= EADER);=0D PrimaryHeader.MyLBA =3D 1;=0D - PrimaryHeader.AlternateLBA =3D 2;=0D - PrimaryHeader.FirstUsableLBA =3D 3;=0D - PrimaryHeader.LastUsableLBA =3D 4;=0D - PrimaryHeader.PartitionEntryLBA =3D 5;=0D + PrimaryHeader.PartitionEntryLBA =3D 2;=0D + PrimaryHeader.AlternateLBA =3D 3;=0D + PrimaryHeader.FirstUsableLBA =3D 4;=0D + PrimaryHeader.LastUsableLBA =3D 5;=0D PrimaryHeader.NumberOfPartitionEntries =3D DEFAULT_PRIMARY_TABLE_HEADER_= NUMBER_OF_PARTITION_ENTRIES;=0D PrimaryHeader.SizeOfPartitionEntry =3D DEFAULT_PRIMARY_TABLE_HEADER_= SIZE_OF_PARTITION_ENTRY;=0D PrimaryHeader.PartitionEntryArrayCRC32 =3D 0; // Purposely invalid=0D @@ -187,11 +187,6 @@ TestSanitizePrimaryHeaderGptEventSize ( EFI_STATUS Status;=0D EFI_PARTITION_TABLE_HEADER PrimaryHeader;=0D UINTN NumberOfPartition;=0D - EFI_GPT_DATA *GptData;=0D - EFI_TCG2_EVENT *Tcg2Event;=0D -=0D - Tcg2Event =3D NULL;=0D - GptData =3D NULL;=0D =0D // Test that a normal PrimaryHeader passes validation=0D PrimaryHeader.NumberOfPartitionEntries =3D 5;=0D @@ -225,6 +220,52 @@ TestSanitizePrimaryHeaderGptEventSize ( return UNIT_TEST_PASSED;=0D }=0D =0D +/**=0D + This function tests the SanitizePeImageEventSize function.=0D + It's intent is to test that the untrusted input from a file path when ge= nerating a=0D + EFI_IMAGE_LOAD_EVENT structure will not cause an overflow when calculati= ng=0D + the event size when allocating space=0D +=0D + @param[in] Context The unit test context.=0D +=0D + @retval UNIT_TEST_PASSED The test passed.=0D + @retval UNIT_TEST_ERROR_TEST_FAILED The test failed.=0D +**/=0D +UNIT_TEST_STATUS=0D +EFIAPI=0D +TestSanitizePeImageEventSize (=0D + IN UNIT_TEST_CONTEXT Context=0D + )=0D +{=0D + UINT32 EventSize;=0D + UINTN ExistingLogicEventSize;=0D + UINT32 FilePathSize;=0D + EFI_STATUS Status;=0D +=0D + FilePathSize =3D 255;=0D +=0D + // Test that a normal PE image passes validation=0D + Status =3D SanitizePeImageEventSize (FilePathSize, &EventSize);=0D + UT_ASSERT_EQUAL (Status, EFI_SUCCESS);=0D +=0D + // Test that the event size is correct compared to the existing logic=0D + ExistingLogicEventSize =3D OFFSET_OF (EFI_IMAGE_LOAD_EVENT, DevicePath)= + FilePathSize;=0D + ExistingLogicEventSize +=3D OFFSET_OF (EFI_TCG2_EVENT, Event);=0D +=0D + if (EventSize !=3D ExistingLogicEventSize) {=0D + UT_LOG_ERROR ("SanitizePeImageEventSize returned an incorrect event si= ze. Expected %u, got %u\n", ExistingLogicEventSize, EventSize);=0D + return UNIT_TEST_ERROR_TEST_FAILED;=0D + }=0D +=0D + // Test that the event size may not overflow=0D + Status =3D SanitizePeImageEventSize (MAX_UINT32, &EventSize);=0D + UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);=0D +=0D + DEBUG ((DEBUG_INFO, "%a: Test passed\n", __func__));=0D +=0D + return UNIT_TEST_PASSED;=0D +}=0D +=0D // *--------------------------------------------------------------------*= =0D // * Unit Test Code Main Function=0D // *--------------------------------------------------------------------*= =0D @@ -267,6 +308,7 @@ UefiTestMain ( AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Validating EF= I Partition Table", "Common.Tcg2MeasureBootLibValidation", TestSanitizeEfiP= artitionTableHeader, NULL, NULL, NULL);=0D AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Primary heade= r gpt event checks for overflow", "Common.Tcg2MeasureBootLibValidation", Te= stSanitizePrimaryHeaderAllocationSize, NULL, NULL, NULL);=0D AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Primary heade= r allocation size checks for overflow", "Common.Tcg2MeasureBootLibValidatio= n", TestSanitizePrimaryHeaderGptEventSize, NULL, NULL, NULL);=0D + AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests PE Image and = FileSize checks for overflow", "Common.Tcg2MeasureBootLibValidation", TestS= anitizePeImageEventSize, NULL, NULL, NULL);=0D =0D Status =3D RunAllTestSuites (Framework);=0D =0D --=20 2.43.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113760): https://edk2.groups.io/g/devel/message/113760 Mute This Topic: https://groups.io/mt/103689723/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-