From a7632e913c1c106f436aefd5e76c394249c383a8 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Tue, 1 Sep 2020 11:12:20 +0200 Subject: 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 Cc: Jiewen Yao Cc: Min Xu Cc: Wenyi Xie Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215 Signed-off-by: Laszlo Ersek Message-Id: <20200901091221.20948-3-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Tested-by: Wenyi Xie Reviewed-by: Min M Xu Reviewed-by: Jiewen Yao --- .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'SecurityPkg') diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c index 377feebb20..100739eb3e 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1855,10 +1855,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; } -- cgit v1.2.3