154 lines
5.1 KiB
Diff
154 lines
5.1 KiB
Diff
From ae16157ee5c96e36e5d1ec558f875e6b89188770 Mon Sep 17 00:00:00 2001
|
|
From: Laszlo Ersek <lersek@redhat.com>
|
|
Date: Tue, 27 Apr 2021 10:10:42 +0200
|
|
Subject: [PATCH 05/10] 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: 3: NetworkPkg/IScsiDxe: fix IScsiHexToBin() security and functionality bugs [rhel-8.4.0.z]
|
|
RH-Commit: [5/10] 96bbb794ca2355c2d9e83d79d385582daf8e4aa4
|
|
RH-Bugzilla: 1956676
|
|
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>
|
|
Upstream: https://bugzilla.tianocore.org/show_bug.cgi?id=3356, c#17...c#22
|
|
---
|
|
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
|
|
|