From 54e90edaed0d7c15230902ac4d74f4304bad2ebd Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Tue, 8 Jun 2021 14:12:58 +0200 Subject: NetworkPkg/IScsiDxe: fix IScsiHexToBin() buffer overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Cc: Maciej Rabeda Cc: Philippe Mathieu-Daudé Cc: Siyuan Fu Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 Signed-off-by: Laszlo Ersek Reviewed-by: Maciej Rabeda Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210608121259.32451-10-lersek@redhat.com> --- NetworkPkg/IScsiDxe/IScsiMisc.c | 20 +++++++++++++++++--- NetworkPkg/IScsiDxe/IScsiMisc.h | 3 +++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c index f0f4992b07..4069547867 100644 --- a/NetworkPkg/IScsiDxe/IScsiMisc.c +++ b/NetworkPkg/IScsiDxe/IScsiMisc.c @@ -377,6 +377,9 @@ IScsiBinToHex ( @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. **/ @@ -387,6 +390,8 @@ IScsiHexToBin ( IN CHAR8 *HexStr ) { + UINTN BinLengthMin; + UINT32 BinLengthProvided; UINTN Index; UINTN Length; UINT8 Digit; @@ -409,6 +414,18 @@ IScsiHexToBin ( 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]; @@ -425,9 +442,6 @@ IScsiHexToBin ( BinBuffer [Index/2] = (UINT8) ((BinBuffer [Index/2] << 4) + Digit); } } - - *BinLength = (UINT32) ((Index + 1)/2); - return EFI_SUCCESS; } diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h index 404a482e57..fddef4f466 100644 --- a/NetworkPkg/IScsiDxe/IScsiMisc.h +++ b/NetworkPkg/IScsiDxe/IScsiMisc.h @@ -172,6 +172,9 @@ IScsiBinToHex ( @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. **/ -- cgit v1.2.3