edk2/edk2-NetworkPkg-IScsiDxe-fix-potential-integer-overflow-i.patch
Miroslav Rezanina b67d81feb6 * Fri Jul 02 2021 Miroslav Rezanina <mrezanin@redhat.com> - 20210527gite1999b264f1f-2
- edk2-NetworkPkg-IScsiDxe-wrap-IScsiCHAP-source-files-to-8.patch [bz#1961100]
- edk2-NetworkPkg-IScsiDxe-simplify-ISCSI_CHAP_AUTH_DATA.In.patch [bz#1961100]
- edk2-NetworkPkg-IScsiDxe-clean-up-ISCSI_CHAP_AUTH_DATA.Ou.patch [bz#1961100]
- edk2-NetworkPkg-IScsiDxe-clean-up-library-class-dependenc.patch [bz#1961100]
- edk2-NetworkPkg-IScsiDxe-fix-potential-integer-overflow-i.patch [bz#1961100]
- edk2-NetworkPkg-IScsiDxe-assert-that-IScsiBinToHex-always.patch [bz#1961100]
- edk2-NetworkPkg-IScsiDxe-reformat-IScsiHexToBin-leading-c.patch [bz#1961100]
- edk2-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-hex-parsing.patch [bz#1961100]
- edk2-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-buffer-overflo.patch [bz#1961100]
- edk2-NetworkPkg-IScsiDxe-check-IScsiHexToBin-return-value.patch [bz#1961100]
- edk2-redhat-build-UefiShell.iso-with-xorriso-rather-than-.patch [bz#1971840]
- Resolves: bz#1961100
  (edk2: remote buffer overflow in IScsiHexToBin function in NetworkPkg/IScsiDxe [rhel-9.0])
- Resolves: bz#1971840
  (Please replace genisoimage with xorriso)
2021-07-02 02:56:49 -04:00

156 lines
5.2 KiB
Diff

From 67474c22010ba8c7c240d8e02b2151c7d796171d Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Tue, 8 Jun 2021 14:12:54 +0200
Subject: [PATCH 05/11] NetworkPkg/IScsiDxe: fix potential integer overflow in
IScsiBinToHex()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Laszlo Ersek <lersek@redhat.com>
RH-MergeRequest: 1: NetworkPkg/IScsiDxe: fix IScsiHexToBin() security and functionality bugs [RHEL-9, c9s]
RH-Commit: [5/10] 3d7a886c1f73d811ef47381e4d6a82683ab0900e
RH-Bugzilla: 1961100
RH-Acked-by: Philippe Mathieu-Daudé <philmd@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>
Message-Id: <20210608121259.32451-6-lersek@redhat.com>
(cherry picked from commit cf01b2dc8fc3ff9cf49fb891af5703dc03e3193e)
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
NetworkPkg/IScsiDxe/IScsiDxe.inf | 1 +
NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
NetworkPkg/IScsiDxe/IScsiMisc.c | 19 +++++++++++++++----
NetworkPkg/IScsiDxe/IScsiMisc.h | 1 +
4 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/NetworkPkg/IScsiDxe/IScsiDxe.inf b/NetworkPkg/IScsiDxe/IScsiDxe.inf
index 543c408302..1dde56d00c 100644
--- a/NetworkPkg/IScsiDxe/IScsiDxe.inf
+++ b/NetworkPkg/IScsiDxe/IScsiDxe.inf
@@ -74,6 +74,7 @@
MemoryAllocationLib
NetLib
PrintLib
+ SafeIntLib
TcpIoLib
UefiBootServicesTableLib
UefiDriverEntryPoint
diff --git a/NetworkPkg/IScsiDxe/IScsiImpl.h b/NetworkPkg/IScsiDxe/IScsiImpl.h
index d895c7feb9..ac3a25730e 100644
--- a/NetworkPkg/IScsiDxe/IScsiImpl.h
+++ b/NetworkPkg/IScsiDxe/IScsiImpl.h
@@ -44,6 +44,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#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>
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index b8fef3ff6f..42988e15cb 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -316,6 +316,7 @@ IScsiMacAddrToStr (
@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.
**/
@@ -327,18 +328,28 @@ IScsiBinToHex (
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.
//
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h
index 46c725aab3..231413993b 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.h
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.h
@@ -150,6 +150,7 @@ IScsiAsciiStrToIp (
@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.
**/
--
2.27.0