Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP5:GA
ovmf.14653
ovmf-bsc1175476-fix-DxeImageVerificationLib-ove...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File ovmf-bsc1175476-fix-DxeImageVerificationLib-overflow.patch of Package ovmf.14653
From 6152573d983f80a11990883254b1151fae51e06c Mon Sep 17 00:00:00 2001 From: Gary Lin <glin@suse.com> Date: Thu, 3 Sep 2020 11:52:36 +0800 Subject: [PATCH 0/3] *** SUBJECT HERE *** MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *** BLURB HERE *** Laszlo Ersek (3): SecurityPkg/DxeImageVerificationLib: extract SecDataDirEnd, SecDataDirLeft SecurityPkg/DxeImageVerificationLib: assign WinCertificate after size check SecurityPkg/DxeImageVerificationLib: catch alignment overflow (CVE-2019-14562) .../DxeImageVerificationLib.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) -- 2.28.0 From 6dd79572668e9fea3d0ee6d4ef27336162a0b5b5 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <lersek@redhat.com> Date: Tue, 1 Sep 2020 11:12:19 +0200 Subject: [PATCH 1/3] SecurityPkg/DxeImageVerificationLib: extract SecDataDirEnd, SecDataDirLeft MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The following two quantities: SecDataDir->VirtualAddress + SecDataDir->Size SecDataDir->VirtualAddress + SecDataDir->Size - OffSet are used multiple times in DxeImageVerificationHandler(). Introduce helper variables for them: "SecDataDirEnd" and "SecDataDirLeft", respectively. This saves us multiple calculations and significantly simplifies the code. Note that all three summands above have type UINT32, therefore the new variables are also of type UINT32. This patch does not change behavior. (Note that the code already handles the case when the SecDataDir->VirtualAddress + SecDataDir->Size UINT32 addition overflows -- namely, in that case, the certificate loop is never entered, and the corruption check right after the loop fires.) Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Wenyi Xie <xiewenyi2@huawei.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200901091221.20948-2-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Wenyi Xie <xiewenyi2@huawei.com> Reviewed-by: Min M Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> (cherry picked from commit 503248ccdf45c14d4040ce44163facdc212e4991) --- .../DxeImageVerificationLib.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c index 23bf7b9ef760..8872319a2e81 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1671,6 +1671,8 @@ DxeImageVerificationHandler ( UINT8 *AuthData; UINTN AuthDataSize; EFI_IMAGE_DATA_DIRECTORY *SecDataDir; + UINT32 SecDataDirEnd; + UINT32 SecDataDirLeft; UINT32 OffSet; CHAR16 *NameStr; RETURN_STATUS PeCoffStatus; @@ -1883,12 +1885,14 @@ DxeImageVerificationHandler ( // "Attribute Certificate Table". // The first certificate starts at offset (SecDataDir->VirtualAddress) from the start of the file. // + SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size; for (OffSet = SecDataDir->VirtualAddress; - OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size); + OffSet < SecDataDirEnd; OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength))) { WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet); - if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof (WIN_CERTIFICATE) || - (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < WinCertificate->dwLength) { + SecDataDirLeft = SecDataDirEnd - OffSet; + if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) || + SecDataDirLeft < WinCertificate->dwLength) { break; } @@ -1982,7 +1986,7 @@ DxeImageVerificationHandler ( } } - if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size)) { + if (OffSet != SecDataDirEnd) { // // The Size in Certificate Table or the attribute certicate table is corrupted. // -- 2.28.0 From 792d134afe18a95f2112533b7aef41e2202ed03a Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <lersek@redhat.com> Date: Tue, 1 Sep 2020 11:12:20 +0200 Subject: [PATCH 2/3] SecurityPkg/DxeImageVerificationLib: assign WinCertificate after size check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) check only guards the de-referencing of the "WinCertificate" pointer. It does not guard the calculation of the pointer itself: WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet); This is wrong; if we don't know for sure that we have enough room for a WIN_CERTIFICATE, then even creating such a pointer, not just de-referencing it, may invoke undefined behavior. Move the pointer calculation after the size check. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Wenyi Xie <xiewenyi2@huawei.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200901091221.20948-3-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Wenyi Xie <xiewenyi2@huawei.com> Reviewed-by: Min M Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> (cherry picked from commit a7632e913c1c106f436aefd5e76c394249c383a8) --- .../DxeImageVerificationLib/DxeImageVerificationLib.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c index 8872319a2e81..d062984ec906 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1889,10 +1889,12 @@ DxeImageVerificationHandler ( for (OffSet = SecDataDir->VirtualAddress; OffSet < SecDataDirEnd; OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength))) { - WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet); SecDataDirLeft = SecDataDirEnd - OffSet; - if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) || - SecDataDirLeft < WinCertificate->dwLength) { + if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) { + break; + } + WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet); + if (SecDataDirLeft < WinCertificate->dwLength) { break; } -- 2.28.0 From 6152573d983f80a11990883254b1151fae51e06c Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <lersek@redhat.com> Date: Tue, 1 Sep 2020 11:12:21 +0200 Subject: [PATCH 3/3] SecurityPkg/DxeImageVerificationLib: catch alignment overflow (CVE-2019-14562) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DxeImageVerificationHandler() function currently checks whether "SecDataDir" has enough room for "WinCertificate->dwLength". However, for advancing "OffSet", "WinCertificate->dwLength" is aligned to the next multiple of 8. If "WinCertificate->dwLength" is large enough, the alignment will return 0, and "OffSet" will be stuck at the same value. Check whether "SecDataDir" has room left for both "WinCertificate->dwLength" and the alignment. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Wenyi Xie <xiewenyi2@huawei.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200901091221.20948-4-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Wenyi Xie <xiewenyi2@huawei.com> Reviewed-by: Min M Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> (cherry picked from commit 0b143fa43e92be15d11e22f80773bcb1b2b0608f) --- .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c index d062984ec906..d818c4305431 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1894,7 +1894,9 @@ DxeImageVerificationHandler ( break; } WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet); - if (SecDataDirLeft < WinCertificate->dwLength) { + if (SecDataDirLeft < WinCertificate->dwLength || + (SecDataDirLeft - WinCertificate->dwLength < + ALIGN_SIZE (WinCertificate->dwLength))) { break; } -- 2.28.0
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor