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 <jiaxin...@intel.com>
Cc: Maciej Rabeda <maciej.rab...@linux.intel.com>
Cc: Philippe Mathieu-Daudé <phi...@redhat.com>
Cc: Siyuan Fu <siyuan...@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rab...@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
---
NetworkPkg/IScsiDxe/IScsiMisc.h | 1 +
NetworkPkg/IScsiDxe/IScsiMisc.c | 12 ++++++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h
index 28cf408cd5c5..404a482e57f3 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.h
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.h
@@ -155,38 +155,39 @@ IScsiAsciiStrToIp (
**/
EFI_STATUS
IScsiBinToHex (
IN UINT8 *BinBuffer,
IN UINT32 BinLength,
IN OUT CHAR8 *HexStr,
IN OUT UINT32 *HexLength
);
/**
Convert the hexadecimal string into a binary encoded buffer.
@param[in, out] BinBuffer The binary buffer.
@param[in, out] BinLength Length of the binary buffer.
@param[in] HexStr The hexadecimal string.
@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.
**/
EFI_STATUS
IScsiHexToBin (
IN OUT UINT8 *BinBuffer,
IN OUT UINT32 *BinLength,
IN CHAR8 *HexStr
);
/**
Convert the decimal-constant string or hex-constant string into a numerical
value.
@param[in] Str String in decimal or hex.
@return The numerical value.
**/
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index 014700e87a5f..f0f4992b07c7 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -360,72 +360,80 @@ IScsiBinToHex (
HexStr[Index * 2 + 2] = IScsiHexString[BinBuffer[Index] >> 4];
HexStr[Index * 2 + 3] = IScsiHexString[BinBuffer[Index] & 0xf];
}
HexStr[Index * 2 + 2] = '\0';
return EFI_SUCCESS;
}
/**
Convert the hexadecimal string into a binary encoded buffer.
@param[in, out] BinBuffer The binary buffer.
@param[in, out] BinLength Length of the binary buffer.
@param[in] HexStr The hexadecimal string.
@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.
**/
EFI_STATUS
IScsiHexToBin (
IN OUT UINT8 *BinBuffer,
IN OUT UINT32 *BinLength,
IN CHAR8 *HexStr
)
{
UINTN Index;
UINTN Length;
UINT8 Digit;
CHAR8 TemStr[2];
ZeroMem (TemStr, sizeof (TemStr));
//
// Find out how many hex characters the string has.
//
if ((HexStr[0] == '0') && ((HexStr[1] == 'x') || (HexStr[1] == 'X'))) {
HexStr += 2;
}
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;
} else {
BinBuffer [Index/2] = (UINT8) ((BinBuffer [Index/2] << 4) + Digit);
}
}
*BinLength = (UINT32) ((Index + 1)/2);
return EFI_SUCCESS;
}
/**
Convert the decimal-constant string or hex-constant string into a numerical
value.
@param[in] Str String in decimal or hex.
--
2.19.1.3.g30247aa5d201
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76206): https://edk2.groups.io/g/devel/message/76206
Mute This Topic: https://groups.io/mt/83394118/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
