From nobody Sun Feb 8 22:59:33 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+76202+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+76202+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1623154399; cv=none; d=zohomail.com; s=zohoarc; b=RSVbEMnRDtmO6A+/SeIx3hobfecwsrYfbK6Lq+zpqcCQPzrxTvdziKZS35HhhWlH3xKnvdxIFJ1WqVBXSRLrXqSIV/UB3+vGY7FkWBah6B7ssW9MReK5OL06mSKSEbjitQXbXyW4mUN//xjQxQhwoCNKggCj1mmtFUDvrM/kdss= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1623154399; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=ZmYwaxy48vOkZH3oVyVDdSN/3ccL+NeY7UUAi/m7D5w=; b=B7XuTHNwXDAM79wMSTqlWwwpuVcDBpUygqEs8ffMvvd8ntGlo6t31TP/4hnrMZjObp4kPqbI0lKbDzfv6urXTNIRph2dUnYAa+vwQuyOtSBXjimMnTEwzArlLzzwmX3jI45nhAqSFDpNfqmD5p8EPU+ppgwtxC4ykMX8/QmHy6Q= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+76202+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1623154399376184.58523209560474; Tue, 8 Jun 2021 05:13:19 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id wJ91YY1788612xypRHwK5xYV; Tue, 08 Jun 2021 05:13:19 -0700 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web11.11512.1623154393335203769 for ; Tue, 08 Jun 2021 05:13:13 -0700 X-Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-421-hSk-1E-TNKyQMdDo6VybIg-1; Tue, 08 Jun 2021 08:13:11 -0400 X-MC-Unique: hSk-1E-TNKyQMdDo6VybIg-1 X-Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E50858030A0; Tue, 8 Jun 2021 12:13:09 +0000 (UTC) X-Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-27.ams2.redhat.com [10.36.113.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id AB197620DE; Tue, 8 Jun 2021 12:13:08 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Jiaxin Wu , Maciej Rabeda , =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= , Siyuan Fu Subject: [edk2-devel] [PUBLIC edk2 PATCH v2 05/10] NetworkPkg/IScsiDxe: fix potential integer overflow in IScsiBinToHex() Date: Tue, 8 Jun 2021 14:12:54 +0200 Message-Id: <20210608121259.32451-6-lersek@redhat.com> In-Reply-To: <20210608121259.32451-1-lersek@redhat.com> References: <20210608121259.32451-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com X-Gm-Message-State: Aq1awCbehaltrDPacBRhVygWx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1623154399; bh=ZmYwaxy48vOkZH3oVyVDdSN/3ccL+NeY7UUAi/m7D5w=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=Lh1P74xDPwg78S7qFx0OEE1JKyJBWQA/pPppd3ki7QQiKqgZ7tT3ymIfgSTeq6g1UG7 VUm2oUNcPftZunsuhJyQlXycHDx2tdaJ7UBk44RXUW7r2YV90cjZEgE3R2Rkm+ucPvpmF Ay7+NEn81SCL1FOmiz6eTTy1hy5RCsT1q8k= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" Considering IScsiBinToHex(): > if (((*HexLength) - 3) < BinLength * 2) { > *HexLength =3D 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 =3D=3D BinLengt= h)) Because the patch makes BinLength * 2 + 3 safe to calculate in UINT32, using UINT32 for Index * 2 + 2 (with (Index =3D=3D BinLengt= h)) 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 Cc: Maciej Rabeda Cc: Philippe Mathieu-Daud=C3=A9 Cc: Siyuan Fu Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3356 Signed-off-by: Laszlo Ersek Reviewed-by: Maciej Rabeda Reviewed-by: Philippe Mathieu-Daud=C3=A9 --- NetworkPkg/IScsiDxe/IScsiDxe.inf | 1 + NetworkPkg/IScsiDxe/IScsiImpl.h | 1 + NetworkPkg/IScsiDxe/IScsiMisc.h | 1 + NetworkPkg/IScsiDxe/IScsiMisc.c | 19 +++++++++++++++---- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/NetworkPkg/IScsiDxe/IScsiDxe.inf b/NetworkPkg/IScsiDxe/IScsiDx= e.inf index 543c4083026a..1dde56d00ca2 100644 --- a/NetworkPkg/IScsiDxe/IScsiDxe.inf +++ b/NetworkPkg/IScsiDxe/IScsiDxe.inf @@ -58,38 +58,39 @@ [Sources] IScsiProto.c IScsiProto.h =20 [Packages] MdePkg/MdePkg.dec MdeModulePkg/MdeModulePkg.dec CryptoPkg/CryptoPkg.dec NetworkPkg/NetworkPkg.dec =20 [LibraryClasses] BaseCryptLib BaseLib BaseMemoryLib DebugLib DevicePathLib HiiLib MemoryAllocationLib NetLib PrintLib + SafeIntLib TcpIoLib UefiBootServicesTableLib UefiDriverEntryPoint UefiHiiServicesLib UefiLib UefiRuntimeServicesTableLib =20 [Protocols] gEfiAcpiTableProtocolGuid ## SOMETIMES_CONSUMES ## S= ystemTable gEfiDriverBindingProtocolGuid ## SOMETIMES_PRODUCES gEfiPciIoProtocolGuid ## SOMETIMES_CONSUMES gEfiDhcp4ProtocolGuid ## SOMETIMES_CONSUMES gEfiDhcp6ProtocolGuid ## SOMETIMES_CONSUMES gEfiDhcp4ServiceBindingProtocolGuid ## SOMETIMES_CONSUMES gEfiDhcp6ServiceBindingProtocolGuid ## SOMETIMES_CONSUMES gEfiDns4ServiceBindingProtocolGuid ## SOMETIMES_CONSUMES gEfiDns4ProtocolGuid ## SOMETIMES_CONSUMES gEfiDns6ServiceBindingProtocolGuid ## SOMETIMES_CONSUMES gEfiDns6ProtocolGuid ## SOMETIMES_CONSUMES diff --git a/NetworkPkg/IScsiDxe/IScsiImpl.h b/NetworkPkg/IScsiDxe/IScsiImp= l.h index d895c7feb947..ac3a25730efb 100644 --- a/NetworkPkg/IScsiDxe/IScsiImpl.h +++ b/NetworkPkg/IScsiDxe/IScsiImpl.h @@ -28,38 +28,39 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include #include #include =20 #include #include #include #include #include =20 #include #include #include #include #include #include #include #include #include +#include #include #include #include #include #include =20 #include #include #include =20 #include "IScsiConfigNVDataStruc.h" #include "IScsiDriver.h" #include "IScsiProto.h" #include "IScsiCHAP.h" #include "IScsiDhcp.h" #include "IScsiDhcp6.h" =20 #include "IScsiIbft.h" #include "IScsiMisc.h" diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMis= c.h index 46c725aab3a4..231413993b08 100644 --- a/NetworkPkg/IScsiDxe/IScsiMisc.h +++ b/NetworkPkg/IScsiDxe/IScsiMisc.h @@ -134,38 +134,39 @@ IScsiMacAddrToStr ( **/ EFI_STATUS IScsiAsciiStrToIp ( IN CHAR8 *Str, IN UINT8 IpMode, OUT EFI_IP_ADDRESS *Ip ); =20 /** Convert the binary encoded buffer into a hexadecimal encoded string. =20 @param[in] BinBuffer The buffer containing the binary data. @param[in] BinLength Length of the binary buffer. @param[in, out] HexStr Pointer to the string. @param[in, out] HexLength The length of the string. =20 @retval EFI_SUCCESS The binary data is converted to the hexadec= imal 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. =20 **/ EFI_STATUS IScsiBinToHex ( IN UINT8 *BinBuffer, IN UINT32 BinLength, IN OUT CHAR8 *HexStr, IN OUT UINT32 *HexLength ); =20 /** Convert the hexadecimal string into a binary encoded buffer. =20 @param[in, out] BinBuffer The binary buffer. @param[in, out] BinLength Length of the binary buffer. @param[in] HexStr The hexadecimal string. =20 @retval EFI_SUCCESS The hexadecimal string is converted into a = binary diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMis= c.c index b8fef3ff6f5a..42988e15cb06 100644 --- a/NetworkPkg/IScsiDxe/IScsiMisc.c +++ b/NetworkPkg/IScsiDxe/IScsiMisc.c @@ -300,61 +300,72 @@ IScsiMacAddrToStr ( String =3D &Str[3 * Index - 1] ; if (VlanId !=3D 0) { String +=3D UnicodeSPrint (String, 6 * sizeof (CHAR16), L"\\%04x", (UI= NTN) VlanId); } =20 *String =3D L'\0'; } =20 /** Convert the binary encoded buffer into a hexadecimal encoded string. =20 @param[in] BinBuffer The buffer containing the binary data. @param[in] BinLength Length of the binary buffer. @param[in, out] HexStr Pointer to the string. @param[in, out] HexLength The length of the string. =20 @retval EFI_SUCCESS The binary data is converted to the hexadec= imal 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. =20 **/ EFI_STATUS IScsiBinToHex ( IN UINT8 *BinBuffer, IN UINT32 BinLength, IN OUT CHAR8 *HexStr, IN OUT UINT32 *HexLength ) { - UINTN Index; + UINT32 HexLengthMin; + UINT32 HexLengthProvided; + UINT32 Index; =20 if ((HexStr =3D=3D NULL) || (BinBuffer =3D=3D NULL) || (BinLength =3D=3D= 0)) { return EFI_INVALID_PARAMETER; } =20 - if (((*HexLength) - 3) < BinLength * 2) { - *HexLength =3D BinLength * 2 + 3; + // + // Safely calculate: HexLengthMin :=3D BinLength * 2 + 3. + // + if (RETURN_ERROR (SafeUint32Mult (BinLength, 2, &HexLengthMin)) || + RETURN_ERROR (SafeUint32Add (HexLengthMin, 3, &HexLengthMin))) { + return EFI_BAD_BUFFER_SIZE; + } + + HexLengthProvided =3D *HexLength; + *HexLength =3D HexLengthMin; + if (HexLengthProvided < HexLengthMin) { return EFI_BUFFER_TOO_SMALL; } =20 - *HexLength =3D BinLength * 2 + 3; // // Prefix for Hex String. // HexStr[0] =3D '0'; HexStr[1] =3D 'x'; =20 for (Index =3D 0; Index < BinLength; Index++) { HexStr[Index * 2 + 2] =3D IScsiHexString[BinBuffer[Index] >> 4]; HexStr[Index * 2 + 3] =3D IScsiHexString[BinBuffer[Index] & 0xf]; } =20 HexStr[Index * 2 + 2] =3D '\0'; =20 return EFI_SUCCESS; } =20 =20 /** Convert the hexadecimal string into a binary encoded buffer. --=20 2.19.1.3.g30247aa5d201 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76202): https://edk2.groups.io/g/devel/message/76202 Mute This Topic: https://groups.io/mt/83394112/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-