From 54e90edaed0d7c15230902ac4d74f4304bad2ebd Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Tue, 8 Jun 2021 14:12:58 +0200
Subject: [PATCH 26/27] NetworkPkg/IScsiDxe: fix IScsiHexToBin() buffer
 overflow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The IScsiHexToBin() function documents the EFI_BUFFER_TOO_SMALL return
condition, but never actually checks whether the decoded buffer fits into
the caller-provided room (i.e., the input value of "BinLength"), and
EFI_BUFFER_TOO_SMALL is never returned. The decoding of "HexStr" can
overflow "BinBuffer".

This is remotely exploitable, as shown in a subsequent patch, which adds
error checking to the IScsiHexToBin() call sites. This issue allows the
target to compromise the initiator.

Introduce EFI_BAD_BUFFER_SIZE, in addition to the existent
EFI_BUFFER_TOO_SMALL, for reporting a special case of the buffer overflow,
plus actually catch the buffer overflow.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210608121259.32451-10-lersek@redhat.com>
---
 NetworkPkg/IScsiDxe/IScsiMisc.c | 20 +++++++++++++++++---
 NetworkPkg/IScsiDxe/IScsiMisc.h |  3 +++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index f0f4992b07..4069547867 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -377,6 +377,9 @@ IScsiBinToHex (
   @retval EFI_SUCCESS           The hexadecimal string is converted into a
                                 binary encoded buffer.
   @retval EFI_INVALID_PARAMETER Invalid hex encoding found in HexStr.
+  @retval EFI_BAD_BUFFER_SIZE   The length of HexStr is too large for decoding:
+                                the decoded size cannot be expressed in
+                                BinLength on output.
   @retval EFI_BUFFER_TOO_SMALL  The binary buffer is too small to hold the
                                 converted data.
 **/
@@ -387,6 +390,8 @@ IScsiHexToBin (
   IN     CHAR8  *HexStr
   )
 {
+  UINTN   BinLengthMin;
+  UINT32  BinLengthProvided;
   UINTN   Index;
   UINTN   Length;
   UINT8   Digit;
@@ -409,6 +414,18 @@ IScsiHexToBin (
   if (Length == 0 || Length % 2 != 0) {
     return EFI_INVALID_PARAMETER;
   }
+  //
+  // Check if the caller provides enough room for the decoded blob.
+  //
+  BinLengthMin = Length / 2;
+  if (BinLengthMin > MAX_UINT32) {
+    return EFI_BAD_BUFFER_SIZE;
+  }
+  BinLengthProvided = *BinLength;
+  *BinLength = (UINT32)BinLengthMin;
+  if (BinLengthProvided < BinLengthMin) {
+    return EFI_BUFFER_TOO_SMALL;
+  }
 
   for (Index = 0; Index < Length; Index ++) {
     TemStr[0] = HexStr[Index];
@@ -425,9 +442,6 @@ IScsiHexToBin (
       BinBuffer [Index/2] = (UINT8) ((BinBuffer [Index/2] << 4) + Digit);
     }
   }
-
-  *BinLength = (UINT32) ((Index + 1)/2);
-
   return EFI_SUCCESS;
 }
 
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h
index 404a482e57..fddef4f466 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.h
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.h
@@ -172,6 +172,9 @@ IScsiBinToHex (
   @retval EFI_SUCCESS           The hexadecimal string is converted into a
                                 binary encoded buffer.
   @retval EFI_INVALID_PARAMETER Invalid hex encoding found in HexStr.
+  @retval EFI_BAD_BUFFER_SIZE   The length of HexStr is too large for decoding:
+                                the decoded size cannot be expressed in
+                                BinLength on output.
   @retval EFI_BUFFER_TOO_SMALL  The binary buffer is too small to hold the
                                 converted data.
 **/
-- 
2.27.0