From 47b76780b487dbfde4efb6843b16064c4a97e94d Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Tue, 8 Jun 2021 14:12:57 +0200 Subject: NetworkPkg/IScsiDxe: fix IScsiHexToBin() hex parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The IScsiHexToBin() function has the following parser issues: (1) If the *subject sequence* in "HexStr" is empty, the function returns EFI_SUCCESS (with "BinLength" set to 0 on output). Such inputs should be rejected. (2) The function mis-handles a "HexStr" that ends with a stray nibble. For example, if "HexStr" is "0xABC", the function decodes it to the bytes {0xAB, 0x0C}, sets "BinLength" to 2 on output, and returns EFI_SUCCESS. Such inputs should be rejected. (3) If an invalid hex char is found in "HexStr", the function treats it as end-of-hex-string, and returns EFI_SUCCESS. Such inputs should be rejected. All of the above cases are remotely triggerable, as shown in a subsequent patch, which adds error checking to the IScsiHexToBin() call sites. While the initiator is not immediately compromised, incorrectly parsing CHAP_R from the target, in case of mutual authentication, is not great. Extend the interface contract of IScsiHexToBin() with EFI_INVALID_PARAMETER, for reporting issues (1) through (3), and implement the new checks. 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-9-lersek@redhat.com> --- NetworkPkg/IScsiDxe/IScsiMisc.c | 12 ++++++++++-- NetworkPkg/IScsiDxe/IScsiMisc.h | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c index 014700e87a..f0f4992b07 100644 --- a/NetworkPkg/IScsiDxe/IScsiMisc.c +++ b/NetworkPkg/IScsiDxe/IScsiMisc.c @@ -376,6 +376,7 @@ 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_BUFFER_TOO_SMALL The binary buffer is too small to hold the converted data. **/ @@ -402,14 +403,21 @@ IScsiHexToBin ( Length = AsciiStrLen (HexStr); + // + // Reject an empty hex string; reject a stray nibble. + // + if (Length == 0 || Length % 2 != 0) { + return EFI_INVALID_PARAMETER; + } + for (Index = 0; Index < Length; Index ++) { TemStr[0] = HexStr[Index]; Digit = (UINT8) AsciiStrHexToUint64 (TemStr); if (Digit == 0 && TemStr[0] != '0') { // - // Invalid Lun Char. + // Invalid Hex Char. // - break; + return EFI_INVALID_PARAMETER; } if ((Index & 1) == 0) { BinBuffer [Index/2] = Digit; diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h index 28cf408cd5..404a482e57 100644 --- a/NetworkPkg/IScsiDxe/IScsiMisc.h +++ b/NetworkPkg/IScsiDxe/IScsiMisc.h @@ -171,6 +171,7 @@ 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_BUFFER_TOO_SMALL The binary buffer is too small to hold the converted data. **/ -- cgit v1.2.3