Blame SOURCES/edk2-NetworkPkg-IScsiDxe-fix-potential-integer-overflow-i.patch

12bdf0
From 4171bd515a2dcfec59513d3a83adce7ed2903d50 Mon Sep 17 00:00:00 2001
12bdf0
From: Laszlo Ersek <lersek@redhat.com>
12bdf0
Date: Tue, 8 Jun 2021 14:12:54 +0200
12bdf0
Subject: [PATCH 05/10] NetworkPkg/IScsiDxe: fix potential integer overflow in
12bdf0
 IScsiBinToHex()
12bdf0
MIME-Version: 1.0
12bdf0
Content-Type: text/plain; charset=UTF-8
12bdf0
Content-Transfer-Encoding: 8bit
12bdf0
12bdf0
RH-Author: Laszlo Ersek <lersek@redhat.com>
12bdf0
RH-MergeRequest: 5: NetworkPkg/IScsiDxe: fix IScsiHexToBin() security and functionality bugs [rhel-8.5.0, post-rebase]
12bdf0
RH-Commit: [5/10] f52aaaa03b15280eb4a821eeb378d8051ea5ec2a
12bdf0
RH-Bugzilla: 1956408
12bdf0
RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
12bdf0
12bdf0
Considering IScsiBinToHex():
12bdf0
12bdf0
>   if (((*HexLength) - 3) < BinLength * 2) {
12bdf0
>     *HexLength = BinLength * 2 + 3;
12bdf0
>   }
12bdf0
12bdf0
the following subexpressions are problematic:
12bdf0
12bdf0
  (*HexLength) - 3
12bdf0
  BinLength * 2
12bdf0
  BinLength * 2 + 3
12bdf0
12bdf0
The first one may wrap under zero, the latter two may wrap over
12bdf0
MAX_UINT32.
12bdf0
12bdf0
Rewrite the calculation using SafeIntLib.
12bdf0
12bdf0
While at it, change the type of the "Index" variable from UINTN to UINT32.
12bdf0
The largest "Index"-based value that we calculate is
12bdf0
12bdf0
  Index * 2 + 2                                (with (Index == BinLength))
12bdf0
12bdf0
Because the patch makes
12bdf0
12bdf0
  BinLength * 2 + 3
12bdf0
12bdf0
safe to calculate in UINT32, using UINT32 for
12bdf0
12bdf0
  Index * 2 + 2                                (with (Index == BinLength))
12bdf0
12bdf0
is safe too. Consistently using UINT32 improves readability.
12bdf0
12bdf0
This patch is best reviewed with "git show -W".
12bdf0
12bdf0
The integer overflows that this patch fixes are theoretical; a subsequent
12bdf0
patch in the series will audit the IScsiBinToHex() call sites, and show
12bdf0
that none of them can fail.
12bdf0
12bdf0
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
12bdf0
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
12bdf0
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
12bdf0
Cc: Siyuan Fu <siyuan.fu@intel.com>
12bdf0
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
12bdf0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
12bdf0
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
12bdf0
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
12bdf0
Message-Id: <20210608121259.32451-6-lersek@redhat.com>
12bdf0
(cherry picked from commit cf01b2dc8fc3ff9cf49fb891af5703dc03e3193e)
12bdf0
---
12bdf0
 NetworkPkg/IScsiDxe/IScsiDxe.inf |  1 +
12bdf0
 NetworkPkg/IScsiDxe/IScsiImpl.h  |  1 +
12bdf0
 NetworkPkg/IScsiDxe/IScsiMisc.c  | 19 +++++++++++++++----
12bdf0
 NetworkPkg/IScsiDxe/IScsiMisc.h  |  1 +
12bdf0
 4 files changed, 18 insertions(+), 4 deletions(-)
12bdf0
12bdf0
diff --git a/NetworkPkg/IScsiDxe/IScsiDxe.inf b/NetworkPkg/IScsiDxe/IScsiDxe.inf
12bdf0
index 543c408302..1dde56d00c 100644
12bdf0
--- a/NetworkPkg/IScsiDxe/IScsiDxe.inf
12bdf0
+++ b/NetworkPkg/IScsiDxe/IScsiDxe.inf
12bdf0
@@ -74,6 +74,7 @@
12bdf0
   MemoryAllocationLib
12bdf0
   NetLib
12bdf0
   PrintLib
12bdf0
+  SafeIntLib
12bdf0
   TcpIoLib
12bdf0
   UefiBootServicesTableLib
12bdf0
   UefiDriverEntryPoint
12bdf0
diff --git a/NetworkPkg/IScsiDxe/IScsiImpl.h b/NetworkPkg/IScsiDxe/IScsiImpl.h
12bdf0
index d895c7feb9..ac3a25730e 100644
12bdf0
--- a/NetworkPkg/IScsiDxe/IScsiImpl.h
12bdf0
+++ b/NetworkPkg/IScsiDxe/IScsiImpl.h
12bdf0
@@ -44,6 +44,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
12bdf0
 #include <Library/MemoryAllocationLib.h>
12bdf0
 #include <Library/NetLib.h>
12bdf0
 #include <Library/PrintLib.h>
12bdf0
+#include <Library/SafeIntLib.h>
12bdf0
 #include <Library/TcpIoLib.h>
12bdf0
 #include <Library/UefiBootServicesTableLib.h>
12bdf0
 #include <Library/UefiHiiServicesLib.h>
12bdf0
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
12bdf0
index b8fef3ff6f..42988e15cb 100644
12bdf0
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
12bdf0
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
12bdf0
@@ -316,6 +316,7 @@ IScsiMacAddrToStr (
12bdf0
   @retval EFI_SUCCESS          The binary data is converted to the hexadecimal string
12bdf0
                                and the length of the string is updated.
12bdf0
   @retval EFI_BUFFER_TOO_SMALL The string is too small.
12bdf0
+  @retval EFI_BAD_BUFFER_SIZE  BinLength is too large for hex encoding.
12bdf0
   @retval EFI_INVALID_PARAMETER The IP string is malformatted.
12bdf0
 
12bdf0
 **/
12bdf0
@@ -327,18 +328,28 @@ IScsiBinToHex (
12bdf0
   IN OUT UINT32 *HexLength
12bdf0
   )
12bdf0
 {
12bdf0
-  UINTN Index;
12bdf0
+  UINT32 HexLengthMin;
12bdf0
+  UINT32 HexLengthProvided;
12bdf0
+  UINT32 Index;
12bdf0
 
12bdf0
   if ((HexStr == NULL) || (BinBuffer == NULL) || (BinLength == 0)) {
12bdf0
     return EFI_INVALID_PARAMETER;
12bdf0
   }
12bdf0
 
12bdf0
-  if (((*HexLength) - 3) < BinLength * 2) {
12bdf0
-    *HexLength = BinLength * 2 + 3;
12bdf0
+  //
12bdf0
+  // Safely calculate: HexLengthMin := BinLength * 2 + 3.
12bdf0
+  //
12bdf0
+  if (RETURN_ERROR (SafeUint32Mult (BinLength, 2, &HexLengthMin)) ||
12bdf0
+      RETURN_ERROR (SafeUint32Add (HexLengthMin, 3, &HexLengthMin))) {
12bdf0
+    return EFI_BAD_BUFFER_SIZE;
12bdf0
+  }
12bdf0
+
12bdf0
+  HexLengthProvided = *HexLength;
12bdf0
+  *HexLength = HexLengthMin;
12bdf0
+  if (HexLengthProvided < HexLengthMin) {
12bdf0
     return EFI_BUFFER_TOO_SMALL;
12bdf0
   }
12bdf0
 
12bdf0
-  *HexLength = BinLength * 2 + 3;
12bdf0
   //
12bdf0
   // Prefix for Hex String.
12bdf0
   //
12bdf0
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h
12bdf0
index 46c725aab3..231413993b 100644
12bdf0
--- a/NetworkPkg/IScsiDxe/IScsiMisc.h
12bdf0
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.h
12bdf0
@@ -150,6 +150,7 @@ IScsiAsciiStrToIp (
12bdf0
   @retval EFI_SUCCESS          The binary data is converted to the hexadecimal string
12bdf0
                                and the length of the string is updated.
12bdf0
   @retval EFI_BUFFER_TOO_SMALL The string is too small.
12bdf0
+  @retval EFI_BAD_BUFFER_SIZE  BinLength is too large for hex encoding.
12bdf0
   @retval EFI_INVALID_PARAMETER The IP string is malformatted.
12bdf0
 
12bdf0
 **/
12bdf0
-- 
12bdf0
2.27.0
12bdf0