[PUBLIC edk2 PATCH v2 05/10] NetworkPkg/IScsiDxe: fix potential integer overflow in IScsiBinToHex()

["Laszlo Ersek" <lersek@redhat.com>]

Considering IScsiBinToHex():

>   if (((*HexLength) - 3) < BinLength * 2) {
>     *HexLength = BinLength * 2 + 3;
>   }

the following subexpressions are problematic:

  (*HexLength) - 3
  BinLength * 2
  BinLength * 2 + 3

The first one may wrap under zero, the latter two may wrap over
MAX_UINT32.

Rewrite the calculation using SafeIntLib.

While at it, change the type of the "Index" variable from UINTN to UINT32.
The largest "Index"-based value that we calculate is

  Index * 2 + 2                                (with (Index == BinLength))

Because the patch makes

  BinLength * 2 + 3

safe to calculate in UINT32, using UINT32 for

  Index * 2 + 2                                (with (Index == BinLength))

is safe too. Consistently using UINT32 improves readability.

This patch is best reviewed with "git show -W".

The integer overflows that this patch fixes are theoretical; a subsequent
patch in the series will audit the IScsiBinToHex() call sites, and show
that none of them can fail.

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>
---
 NetworkPkg/IScsiDxe/IScsiDxe.inf |  1 +
 NetworkPkg/IScsiDxe/IScsiImpl.h  |  1 +
 NetworkPkg/IScsiDxe/IScsiMisc.h  |  1 +
 NetworkPkg/IScsiDxe/IScsiMisc.c  | 19 +++++++++++++++----
 4 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiDxe.inf b/NetworkPkg/IScsiDxe/IScsiDxe.inf
index 543c4083026a..1dde56d00ca2 100644
--- a/NetworkPkg/IScsiDxe/IScsiDxe.inf
+++ b/NetworkPkg/IScsiDxe/IScsiDxe.inf
@@ -58,38 +58,39 @@ [Sources]
   IScsiProto.c
   IScsiProto.h
 
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   CryptoPkg/CryptoPkg.dec
   NetworkPkg/NetworkPkg.dec
 
 [LibraryClasses]
   BaseCryptLib
   BaseLib
   BaseMemoryLib
   DebugLib
   DevicePathLib
   HiiLib
   MemoryAllocationLib
   NetLib
   PrintLib
+  SafeIntLib
   TcpIoLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
   UefiHiiServicesLib
   UefiLib
   UefiRuntimeServicesTableLib
 
 [Protocols]
   gEfiAcpiTableProtocolGuid                     ## SOMETIMES_CONSUMES ## SystemTable
   gEfiDriverBindingProtocolGuid                 ## SOMETIMES_PRODUCES
   gEfiPciIoProtocolGuid                         ## SOMETIMES_CONSUMES
   gEfiDhcp4ProtocolGuid                         ## SOMETIMES_CONSUMES
   gEfiDhcp6ProtocolGuid                         ## SOMETIMES_CONSUMES
   gEfiDhcp4ServiceBindingProtocolGuid           ## SOMETIMES_CONSUMES
   gEfiDhcp6ServiceBindingProtocolGuid           ## SOMETIMES_CONSUMES
   gEfiDns4ServiceBindingProtocolGuid            ## SOMETIMES_CONSUMES
   gEfiDns4ProtocolGuid                          ## SOMETIMES_CONSUMES
   gEfiDns6ServiceBindingProtocolGuid            ## SOMETIMES_CONSUMES
   gEfiDns6ProtocolGuid                          ## SOMETIMES_CONSUMES
diff --git a/NetworkPkg/IScsiDxe/IScsiImpl.h b/NetworkPkg/IScsiDxe/IScsiImpl.h
index d895c7feb947..ac3a25730efb 100644
--- a/NetworkPkg/IScsiDxe/IScsiImpl.h
+++ b/NetworkPkg/IScsiDxe/IScsiImpl.h
@@ -28,38 +28,39 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Protocol/Tcp6.h>
 #include <Protocol/Ip4Config2.h>
 #include <Protocol/Ip6Config.h>
 
 #include <Protocol/AuthenticationInfo.h>
 #include <Protocol/IScsiInitiatorName.h>
 #include <Protocol/ScsiPassThruExt.h>
 #include <Protocol/AdapterInformation.h>
 #include <Protocol/NetworkInterfaceIdentifier.h>
 
 #include <Library/BaseCryptLib.h>
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/DevicePathLib.h>
 #include <Library/HiiLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/NetLib.h>
 #include <Library/PrintLib.h>
+#include <Library/SafeIntLib.h>
 #include <Library/TcpIoLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiHiiServicesLib.h>
 #include <Library/UefiLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
 
 #include <Guid/MdeModuleHii.h>
 #include <Guid/EventGroup.h>
 #include <Guid/Acpi.h>
 
 #include "IScsiConfigNVDataStruc.h"
 #include "IScsiDriver.h"
 #include "IScsiProto.h"
 #include "IScsiCHAP.h"
 #include "IScsiDhcp.h"
 #include "IScsiDhcp6.h"
 
 #include "IScsiIbft.h"
 #include "IScsiMisc.h"
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h
index 46c725aab3a4..231413993b08 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.h
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.h
@@ -134,38 +134,39 @@ IScsiMacAddrToStr (
 **/
 EFI_STATUS
 IScsiAsciiStrToIp (
   IN  CHAR8             *Str,
   IN  UINT8             IpMode,
   OUT EFI_IP_ADDRESS    *Ip
   );
 
 /**
   Convert the binary encoded buffer into a hexadecimal encoded string.
 
   @param[in]       BinBuffer   The buffer containing the binary data.
   @param[in]       BinLength   Length of the binary buffer.
   @param[in, out]  HexStr      Pointer to the string.
   @param[in, out]  HexLength   The length of the string.
 
   @retval EFI_SUCCESS          The binary data is converted to the hexadecimal string
                                and the length of the string is updated.
   @retval EFI_BUFFER_TOO_SMALL The string is too small.
+  @retval EFI_BAD_BUFFER_SIZE  BinLength is too large for hex encoding.
   @retval EFI_INVALID_PARAMETER The IP string is malformatted.
 
 **/
 EFI_STATUS
 IScsiBinToHex (
   IN     UINT8  *BinBuffer,
   IN     UINT32 BinLength,
   IN OUT CHAR8  *HexStr,
   IN OUT UINT32 *HexLength
   );
 
 /**
   Convert the hexadecimal string into a binary encoded buffer.
 
   @param[in, out]  BinBuffer   The binary buffer.
   @param[in, out]  BinLength   Length of the binary buffer.
   @param[in]       HexStr      The hexadecimal string.
 
   @retval EFI_SUCCESS          The hexadecimal string is converted into a binary
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index b8fef3ff6f5a..42988e15cb06 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -300,61 +300,72 @@ IScsiMacAddrToStr (
   String = &Str[3 * Index - 1] ;
   if (VlanId != 0) {
     String += UnicodeSPrint (String, 6 * sizeof (CHAR16), L"\\%04x", (UINTN) VlanId);
   }
 
   *String = L'\0';
 }
 
 /**
   Convert the binary encoded buffer into a hexadecimal encoded string.
 
   @param[in]       BinBuffer   The buffer containing the binary data.
   @param[in]       BinLength   Length of the binary buffer.
   @param[in, out]  HexStr      Pointer to the string.
   @param[in, out]  HexLength   The length of the string.
 
   @retval EFI_SUCCESS          The binary data is converted to the hexadecimal string
                                and the length of the string is updated.
   @retval EFI_BUFFER_TOO_SMALL The string is too small.
+  @retval EFI_BAD_BUFFER_SIZE  BinLength is too large for hex encoding.
   @retval EFI_INVALID_PARAMETER The IP string is malformatted.
 
 **/
 EFI_STATUS
 IScsiBinToHex (
   IN     UINT8  *BinBuffer,
   IN     UINT32 BinLength,
   IN OUT CHAR8  *HexStr,
   IN OUT UINT32 *HexLength
   )
 {
-  UINTN Index;
+  UINT32 HexLengthMin;
+  UINT32 HexLengthProvided;
+  UINT32 Index;
 
   if ((HexStr == NULL) || (BinBuffer == NULL) || (BinLength == 0)) {
     return EFI_INVALID_PARAMETER;
   }
 
-  if (((*HexLength) - 3) < BinLength * 2) {
-    *HexLength = BinLength * 2 + 3;
+  //
+  // Safely calculate: HexLengthMin := BinLength * 2 + 3.
+  //
+  if (RETURN_ERROR (SafeUint32Mult (BinLength, 2, &HexLengthMin)) ||
+      RETURN_ERROR (SafeUint32Add (HexLengthMin, 3, &HexLengthMin))) {
+    return EFI_BAD_BUFFER_SIZE;
+  }
+
+  HexLengthProvided = *HexLength;
+  *HexLength = HexLengthMin;
+  if (HexLengthProvided < HexLengthMin) {
     return EFI_BUFFER_TOO_SMALL;
   }
 
-  *HexLength = BinLength * 2 + 3;
   //
   // Prefix for Hex String.
   //
   HexStr[0] = '0';
   HexStr[1] = 'x';
 
   for (Index = 0; Index < BinLength; Index++) {
     HexStr[Index * 2 + 2] = IScsiHexString[BinBuffer[Index] >> 4];
     HexStr[Index * 2 + 3] = IScsiHexString[BinBuffer[Index] & 0xf];
   }
 
   HexStr[Index * 2 + 2] = '\0';
 
   return EFI_SUCCESS;
 }
 
 
 /**
   Convert the hexadecimal string into a binary encoded buffer.
-- 
2.19.1.3.g30247aa5d201