|
 |
12bdf0 |
From 2f0e51dcfea6d9101c4694636a948eb4b6e6d4d4 Mon Sep 17 00:00:00 2001
|
|
|
12bdf0 |
From: Laszlo Ersek <lersek@redhat.com>
|
|
|
12bdf0 |
Date: Tue, 8 Jun 2021 14:12:57 +0200
|
|
|
12bdf0 |
Subject: [PATCH 08/10] NetworkPkg/IScsiDxe: fix IScsiHexToBin() hex parsing
|
|
|
12bdf0 |
MIME-Version: 1.0
|
|
|
12bdf0 |
Content-Type: text/plain; charset=UTF-8
|
|
|
12bdf0 |
Content-Transfer-Encoding: 8bit
|
|
|
12bdf0 |
|
|
|
12bdf0 |
RH-Author: Laszlo Ersek <lersek@redhat.com>
|
|
|
12bdf0 |
RH-MergeRequest: 5: NetworkPkg/IScsiDxe: fix IScsiHexToBin() security and functionality bugs [rhel-8.5.0, post-rebase]
|
|
|
12bdf0 |
RH-Commit: [8/10] febb96c07dbd0e4a191e855742cb47fc6e39dfba
|
|
|
12bdf0 |
RH-Bugzilla: 1956408
|
|
|
12bdf0 |
RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
|
|
|
12bdf0 |
|
|
|
12bdf0 |
The IScsiHexToBin() function has the following parser issues:
|
|
|
12bdf0 |
|
|
|
12bdf0 |
(1) If the *subject sequence* in "HexStr" is empty, the function returns
|
|
|
12bdf0 |
EFI_SUCCESS (with "BinLength" set to 0 on output). Such inputs should
|
|
|
12bdf0 |
be rejected.
|
|
|
12bdf0 |
|
|
|
12bdf0 |
(2) The function mis-handles a "HexStr" that ends with a stray nibble. For
|
|
|
12bdf0 |
example, if "HexStr" is "0xABC", the function decodes it to the bytes
|
|
|
12bdf0 |
{0xAB, 0x0C}, sets "BinLength" to 2 on output, and returns
|
|
|
12bdf0 |
EFI_SUCCESS. Such inputs should be rejected.
|
|
|
12bdf0 |
|
|
|
12bdf0 |
(3) If an invalid hex char is found in "HexStr", the function treats it as
|
|
|
12bdf0 |
end-of-hex-string, and returns EFI_SUCCESS. Such inputs should be
|
|
|
12bdf0 |
rejected.
|
|
|
12bdf0 |
|
|
|
12bdf0 |
All of the above cases are remotely triggerable, as shown in a subsequent
|
|
|
12bdf0 |
patch, which adds error checking to the IScsiHexToBin() call sites. While
|
|
|
12bdf0 |
the initiator is not immediately compromised, incorrectly parsing CHAP_R
|
|
|
12bdf0 |
from the target, in case of mutual authentication, is not great.
|
|
|
12bdf0 |
|
|
|
12bdf0 |
Extend the interface contract of IScsiHexToBin() with
|
|
|
12bdf0 |
EFI_INVALID_PARAMETER, for reporting issues (1) through (3), and implement
|
|
|
12bdf0 |
the new checks.
|
|
|
12bdf0 |
|
|
|
12bdf0 |
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
|
|
|
12bdf0 |
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
|
|
|
12bdf0 |
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
|
|
|
12bdf0 |
Cc: Siyuan Fu <siyuan.fu@intel.com>
|
|
|
12bdf0 |
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
|
|
|
12bdf0 |
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
|
|
|
12bdf0 |
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
|
|
|
12bdf0 |
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
|
|
|
12bdf0 |
Message-Id: <20210608121259.32451-9-lersek@redhat.com>
|
|
|
12bdf0 |
(cherry picked from commit 47b76780b487dbfde4efb6843b16064c4a97e94d)
|
|
|
12bdf0 |
---
|
|
|
12bdf0 |
NetworkPkg/IScsiDxe/IScsiMisc.c | 12 ++++++++++--
|
|
|
12bdf0 |
NetworkPkg/IScsiDxe/IScsiMisc.h | 1 +
|
|
|
12bdf0 |
2 files changed, 11 insertions(+), 2 deletions(-)
|
|
|
12bdf0 |
|
|
|
12bdf0 |
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
|
|
|
12bdf0 |
index 014700e87a..f0f4992b07 100644
|
|
|
12bdf0 |
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
|
|
|
12bdf0 |
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
|
|
|
12bdf0 |
@@ -376,6 +376,7 @@ IScsiBinToHex (
|
|
|
12bdf0 |
|
|
|
12bdf0 |
@retval EFI_SUCCESS The hexadecimal string is converted into a
|
|
|
12bdf0 |
binary encoded buffer.
|
|
|
12bdf0 |
+ @retval EFI_INVALID_PARAMETER Invalid hex encoding found in HexStr.
|
|
|
12bdf0 |
@retval EFI_BUFFER_TOO_SMALL The binary buffer is too small to hold the
|
|
|
12bdf0 |
converted data.
|
|
|
12bdf0 |
**/
|
|
|
12bdf0 |
@@ -402,14 +403,21 @@ IScsiHexToBin (
|
|
|
12bdf0 |
|
|
|
12bdf0 |
Length = AsciiStrLen (HexStr);
|
|
|
12bdf0 |
|
|
|
12bdf0 |
+ //
|
|
|
12bdf0 |
+ // Reject an empty hex string; reject a stray nibble.
|
|
|
12bdf0 |
+ //
|
|
|
12bdf0 |
+ if (Length == 0 || Length % 2 != 0) {
|
|
|
12bdf0 |
+ return EFI_INVALID_PARAMETER;
|
|
|
12bdf0 |
+ }
|
|
|
12bdf0 |
+
|
|
|
12bdf0 |
for (Index = 0; Index < Length; Index ++) {
|
|
|
12bdf0 |
TemStr[0] = HexStr[Index];
|
|
|
12bdf0 |
Digit = (UINT8) AsciiStrHexToUint64 (TemStr);
|
|
|
12bdf0 |
if (Digit == 0 && TemStr[0] != '0') {
|
|
|
12bdf0 |
//
|
|
|
12bdf0 |
- // Invalid Lun Char.
|
|
|
12bdf0 |
+ // Invalid Hex Char.
|
|
|
12bdf0 |
//
|
|
|
12bdf0 |
- break;
|
|
|
12bdf0 |
+ return EFI_INVALID_PARAMETER;
|
|
|
12bdf0 |
}
|
|
|
12bdf0 |
if ((Index & 1) == 0) {
|
|
|
12bdf0 |
BinBuffer [Index/2] = Digit;
|
|
|
12bdf0 |
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h
|
|
|
12bdf0 |
index 28cf408cd5..404a482e57 100644
|
|
|
12bdf0 |
--- a/NetworkPkg/IScsiDxe/IScsiMisc.h
|
|
|
12bdf0 |
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.h
|
|
|
12bdf0 |
@@ -171,6 +171,7 @@ IScsiBinToHex (
|
|
|
12bdf0 |
|
|
|
12bdf0 |
@retval EFI_SUCCESS The hexadecimal string is converted into a
|
|
|
12bdf0 |
binary encoded buffer.
|
|
|
12bdf0 |
+ @retval EFI_INVALID_PARAMETER Invalid hex encoding found in HexStr.
|
|
|
12bdf0 |
@retval EFI_BUFFER_TOO_SMALL The binary buffer is too small to hold the
|
|
|
12bdf0 |
converted data.
|
|
|
12bdf0 |
**/
|
|
|
12bdf0 |
--
|
|
|
12bdf0 |
2.27.0
|
|
|
12bdf0 |
|