tyler.durden
/
audk
mirror of https://github.com/acidanthera/audk.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

NetworkPkg/IScsiDxe: fix potential integer overflow in IScsiBinToHex() 

Considering IScsiBinToHex():

>   if (((*HexLength) - 3) < BinLength * 2) {
>     *HexLength = 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 == BinLength))

Because the patch makes

  BinLength * 2 + 3

safe to calculate in UINT32, using UINT32 for

  Index * 2 + 2                                (with (Index == BinLength))

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 <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210608121259.32451-6-lersek@redhat.com>
This commit is contained in:
Laszlo Ersek 2021-06-08 14:12:54 +02:00 committed by mergify[bot]
parent e8f28b09e6
commit cf01b2dc8f
4 changed files with 18 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
NetworkPkg/IScsiDxe/IScsiDxe.inf
View File

@ -74,6 +74,7 @@
MemoryAllocationLib
NetLib
PrintLib
SafeIntLib
TcpIoLib
UefiBootServicesTableLib
UefiDriverEntryPoint

1
NetworkPkg/IScsiDxe/IScsiImpl.h
View File

@ -44,6 +44,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/MemoryAllocationLib.h>
#include <Library/NetLib.h>
#include <Library/PrintLib.h>
#include <Library/SafeIntLib.h>
#include <Library/TcpIoLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiHiiServicesLib.h>

19
NetworkPkg/IScsiDxe/IScsiMisc.c
View File

@ -316,6 +316,7 @@ IScsiMacAddrToStr (
@retval EFI_SUCCESS The binary data is converted to the hexadecimal 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.
**/
@ -327,18 +328,28 @@ IScsiBinToHex (
IN OUT UINT32 *HexLength
)
{
UINTN Index;
UINT32 HexLengthMin;
UINT32 HexLengthProvided;
UINT32 Index;
if ((HexStr == NULL) || (BinBuffer == NULL) || (BinLength == 0)) {
return EFI_INVALID_PARAMETER;
}
if (((*HexLength) - 3) < BinLength * 2) {
*HexLength = BinLength * 2 + 3;
//
// Safely calculate: HexLengthMin := BinLength * 2 + 3.
//
if (RETURN_ERROR (SafeUint32Mult (BinLength, 2, &HexLengthMin)) ||
RETURN_ERROR (SafeUint32Add (HexLengthMin, 3, &HexLengthMin))) {
return EFI_BAD_BUFFER_SIZE;
}
HexLengthProvided = *HexLength;
*HexLength = HexLengthMin;
if (HexLengthProvided < HexLengthMin) {
return EFI_BUFFER_TOO_SMALL;
}
*HexLength = BinLength * 2 + 3;
//
// Prefix for Hex String.
//

1
NetworkPkg/IScsiDxe/IScsiMisc.h
View File

@ -150,6 +150,7 @@ IScsiAsciiStrToIp (
@retval EFI_SUCCESS The binary data is converted to the hexadecimal 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.
**/