From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from huawei.com (huawei.com [45.249.212.35]) by mx.groups.io with SMTP id smtpd.web10.9866.1597216151477171945 for ; Wed, 12 Aug 2020 00:09:12 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: huawei.com, ip: 45.249.212.35, mailfrom: xiewenyi2@huawei.com) Received: from DGGEMS406-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id F2B8A3DBFB5AF171021E for ; Wed, 12 Aug 2020 15:09:05 +0800 (CST) Received: from HGH1000039998.huawei.com (10.184.68.188) by DGGEMS406-HUB.china.huawei.com (10.3.19.206) with Microsoft SMTP Server id 14.3.487.0; Wed, 12 Aug 2020 15:08:56 +0800 From: "wenyi,xie" To: , , , CC: , Subject: [PATCH EDK2 v1 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset(CVE-2019-14562) Date: Wed, 12 Aug 2020 15:04:46 +0800 Message-ID: <1597215886-48713-2-git-send-email-xiewenyi2@huawei.com> X-Mailer: git-send-email 2.8.1 In-Reply-To: <1597215886-48713-1-git-send-email-xiewenyi2@huawei.com> References: <1597215886-48713-1-git-send-email-xiewenyi2@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.184.68.188] X-CFilter-Loop: Reflected Content-Type: text/plain REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2215 There is an integer overflow vulnerability in DxeImageVerificationHandler function when parsing the PE files attribute certificate table. In cases where WinCertificate->dwLength is sufficiently large, it's possible to overflow Offset back to 0 causing an endless loop. Check offset inbetween VirtualAddress and VirtualAddress + Size. Using SafeintLib to do offset addition with result check. Cc: Jiewen Yao Cc: Jian J Wang Cc: Chao Zhang Signed-off-by: Wenyi Xie --- SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf | 1 + SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h | 1 + SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 21 +++++++++++++++----- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf index 1e1a639857e0..a7ac4830b3d4 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf @@ -53,6 +53,7 @@ [LibraryClasses] SecurityManagementLib PeCoffLib TpmMeasurementLib + SafeIntLib [Protocols] gEfiFirmwareVolume2ProtocolGuid ## SOMETIMES_CONSUMES diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h index 17955ff9774c..060273917d5d 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include #include #include +#include #include #include #include diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c index 36b87e16d53d..2b42d4595f2c 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1658,6 +1658,9 @@ DxeImageVerificationHandler ( EFI_STATUS HashStatus; EFI_STATUS DbStatus; BOOLEAN IsFound; + UINT32 AlignedLength; + UINT32 Result; + EFI_STATUS AddStatus; SignatureList = NULL; SignatureListSize = 0; @@ -1667,6 +1670,7 @@ DxeImageVerificationHandler ( Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED; IsVerified = FALSE; IsFound = FALSE; + Result = 0; // // Check the image type and get policy setting. @@ -1850,9 +1854,9 @@ DxeImageVerificationHandler ( // The first certificate starts at offset (SecDataDir->VirtualAddress) from the start of the file. // for (OffSet = SecDataDir->VirtualAddress; - OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size); - OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength))) { + (OffSet >= SecDataDir->VirtualAddress) && (OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size));) { WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet); + AlignedLength = WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength); if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof (WIN_CERTIFICATE) || (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < WinCertificate->dwLength) { break; @@ -1881,7 +1885,7 @@ DxeImageVerificationHandler ( break; } if (!CompareGuid (&WinCertUefiGuid->CertType, &gEfiCertPkcs7Guid)) { - continue; + goto NEXT_LOOP; } AuthData = WinCertUefiGuid->CertData; AuthDataSize = WinCertUefiGuid->Hdr.dwLength - OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData); @@ -1889,12 +1893,12 @@ DxeImageVerificationHandler ( if (WinCertificate->dwLength < sizeof (WIN_CERTIFICATE)) { break; } - continue; + goto NEXT_LOOP; } HashStatus = HashPeImageByType (AuthData, AuthDataSize); if (EFI_ERROR (HashStatus)) { - continue; + goto NEXT_LOOP; } // @@ -1946,6 +1950,13 @@ DxeImageVerificationHandler ( DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but signature is not allowed by DB and %s hash of image is not found in DB/DBX.\n", mHashTypeStr)); } } + +NEXT_LOOP: + AddStatus = SafeUint32Add (OffSet, AlignedLength, &Result); + if (EFI_ERROR (AddStatus)) { + break; + } + OffSet = Result; } if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size)) { -- 2.20.1.windows.1