[edk2-devel] [PUBLIC edk2 PATCH v2 09/10] NetworkPkg/IScsiDxe: fix IScsiHexToBin() buffer overflow

[Laszlo Ersek]

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

diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h
index 404a482e57f3..fddef4f466dc 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.h
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.h
@@ -156,38 +156,41 @@ IScsiAsciiStrToIp (
 **/
 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 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.
 **/
 EFI_STATUS
 IScsiHexToBin (
   IN OUT UINT8  *BinBuffer,
   IN OUT UINT32 *BinLength,
   IN     CHAR8  *HexStr
   );
 
 
 /**
   Convert the decimal-constant string or hex-constant string into a numerical value.
 
   @param[in] Str                    String in decimal or hex.
 
   @return The numerical value.
 
 **/
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index f0f4992b07c7..406954786751 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -361,89 +361,103 @@ IScsiBinToHex (
     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.
 
   @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 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.
 **/
 EFI_STATUS
 IScsiHexToBin (
   IN OUT UINT8  *BinBuffer,
   IN OUT UINT32 *BinLength,
   IN     CHAR8  *HexStr
   )
 {
+  UINTN   BinLengthMin;
+  UINT32  BinLengthProvided;
   UINTN   Index;
   UINTN   Length;
   UINT8   Digit;
   CHAR8   TemStr[2];
 
   ZeroMem (TemStr, sizeof (TemStr));
 
   //
   // Find out how many hex characters the string has.
   //
   if ((HexStr[0] == '0') && ((HexStr[1] == 'x') || (HexStr[1] == 'X'))) {
     HexStr += 2;
   }
 
   Length = AsciiStrLen (HexStr);
 
   //
   // Reject an empty hex string; reject a stray nibble.
   //
   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];
     Digit = (UINT8) AsciiStrHexToUint64 (TemStr);
     if (Digit == 0 && TemStr[0] != '0') {
       //
       // Invalid Hex Char.
       //
       return EFI_INVALID_PARAMETER;
     }
     if ((Index & 1) == 0) {
       BinBuffer [Index/2] = Digit;
     } else {
       BinBuffer [Index/2] = (UINT8) ((BinBuffer [Index/2] << 4) + Digit);
     }
   }
-
-  *BinLength = (UINT32) ((Index + 1)/2);
-
   return EFI_SUCCESS;
 }
 
 
 /**
   Convert the decimal-constant string or hex-constant string into a numerical value.
 
   @param[in] Str                    String in decimal or hex.
 
   @return The numerical value.
 
 **/
 UINTN
 IScsiNetNtoi (
   IN     CHAR8  *Str
   )
 {
   if ((Str[0] == '0') && ((Str[1] == 'x') || (Str[1] == 'X'))) {
     Str += 2;
-- 
2.19.1.3.g30247aa5d201




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76207): https://edk2.groups.io/g/devel/message/76207
Mute This Topic: https://groups.io/mt/83394119/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-