From 503248ccdf45c14d4040ce44163facdc212e4991 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Tue, 1 Sep 2020 11:12:19 +0200
Subject: [PATCH 2/4] 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>
---
 .../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 b08fe24e85..377feebb20 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1652,6 +1652,8 @@ DxeImageVerificationHandler (
   UINT8                                *AuthData;
   UINTN                                AuthDataSize;
   EFI_IMAGE_DATA_DIRECTORY             *SecDataDir;
+  UINT32                               SecDataDirEnd;
+  UINT32                               SecDataDirLeft;
   UINT32                               OffSet;
   CHAR16                               *NameStr;
   RETURN_STATUS                        PeCoffStatus;
@@ -1849,12 +1851,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;
     }
 
@@ -1948,7 +1952,7 @@ DxeImageVerificationHandler (
     }
   }
 
-  if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size)) {
+  if (OffSet != SecDataDirEnd) {
     //
     // The Size in Certificate Table or the attribute certificate table is corrupted.
     //
-- 
2.27.0

