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

NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45235 Patch 

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4540

Bug Details:
PixieFail Bug #7
CVE-2023-45235
CVSS 8.3 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:L/A:H
CWE-119 Improper Restriction of Operations within the Bounds of
 a Memory Buffer

Buffer overflow when handling Server ID option from a DHCPv6 proxy
Advertise message

Change Overview:

Performs two checks

1. Checks that the length of the duid is accurate
> + //
> + // Check that the minimum and maximum requirements are met
> + //
> + if ((OpLen < PXEBC_MIN_SIZE_OF_DUID) ||
(OpLen > PXEBC_MAX_SIZE_OF_DUID)) {
> +  Status = EFI_INVALID_PARAMETER;
> +  goto ON_ERROR;
> + }

2. Ensures that the amount of data written to the buffer is tracked and
never exceeds that
> + //
> + // Check that the option length is valid.
> + //
> + if ((DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN)
 > DiscoverLenNeeded) {
> +     Status = EFI_OUT_OF_RESOURCES;
> +     goto ON_ERROR;
> + }

Additional code clean up and fix for memory leak in case Option was NULL

Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
Cc: Zachary Clark-williams <zachary.clark-williams@intel.com>

Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
This commit is contained in:
Doug Flick 2024-01-26 05:54:55 +08:00 committed by mergify[bot]
parent 7f04c7a253
commit fac297724e
2 changed files with 78 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
View File

@ -887,6 +887,7 @@ PxeBcRequestBootService (
EFI_STATUS Status;
EFI_DHCP6_PACKET *IndexOffer;
UINT8 *Option;
UINTN DiscoverLenNeeded;
PxeBc = &Private->PxeBc;
Request = Private->Dhcp6Request;
@ -899,7 +900,8 @@ PxeBcRequestBootService (
return EFI_DEVICE_ERROR;
}
Discover = AllocateZeroPool (sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET));
DiscoverLenNeeded = sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET);
Discover = AllocateZeroPool (DiscoverLenNeeded);
if (Discover == NULL) {
return EFI_OUT_OF_RESOURCES;
}
@ -924,16 +926,34 @@ PxeBcRequestBootService (
DHCP6_OPT_SERVER_ID
);
if (Option == NULL) {
return EFI_NOT_FOUND;
Status = EFI_NOT_FOUND;
goto ON_ERROR;
}
//
// Add Server ID Option.
//
OpLen = NTOHS (((EFI_DHCP6_PACKET_OPTION *)Option)->OpLen);
CopyMem (DiscoverOpt, Option, OpLen + 4);
DiscoverOpt += (OpLen + 4);
DiscoverLen += (OpLen + 4);
//
// Check that the minimum and maximum requirements are met
//
if ((OpLen < PXEBC_MIN_SIZE_OF_DUID) || (OpLen > PXEBC_MAX_SIZE_OF_DUID)) {
Status = EFI_INVALID_PARAMETER;
goto ON_ERROR;
}
//
// Check that the option length is valid.
//
if ((DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN) > DiscoverLenNeeded) {
Status = EFI_OUT_OF_RESOURCES;
goto ON_ERROR;
}
CopyMem (DiscoverOpt, Option, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
}
while (RequestLen < Request->Length) {
@ -944,16 +964,24 @@ PxeBcRequestBootService (
(OpCode != DHCP6_OPT_SERVER_ID)
)
{
//
// Check that the option length is valid.
//
if (DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN > DiscoverLenNeeded) {
Status = EFI_OUT_OF_RESOURCES;
goto ON_ERROR;
}
//
// Copy all the options except IA option and Server ID
//
CopyMem (DiscoverOpt, RequestOpt, OpLen + 4);
DiscoverOpt += (OpLen + 4);
DiscoverLen += (OpLen + 4);
CopyMem (DiscoverOpt, RequestOpt, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
}
RequestOpt += (OpLen + 4);
RequestLen += (OpLen + 4);
RequestOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
RequestLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
}
//
@ -2154,6 +2182,7 @@ PxeBcDhcp6Discover (
UINT16 OpLen;
UINT32 Xid;
EFI_STATUS Status;
UINTN DiscoverLenNeeded;
PxeBc = &Private->PxeBc;
Mode = PxeBc->Mode;
@ -2169,7 +2198,8 @@ PxeBcDhcp6Discover (
return EFI_DEVICE_ERROR;
}
Discover = AllocateZeroPool (sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET));
DiscoverLenNeeded = sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET);
Discover = AllocateZeroPool (DiscoverLenNeeded);
if (Discover == NULL) {
return EFI_OUT_OF_RESOURCES;
}
@ -2185,22 +2215,37 @@ PxeBcDhcp6Discover (
DiscoverLen = sizeof (EFI_DHCP6_HEADER);
RequestLen = DiscoverLen;
//
// The request packet is generated by the UEFI network stack. In the DHCP4 DORA and DHCP6 SARR sequence,
// the first (discover in DHCP4 and solicit in DHCP6) and third (request in both DHCP4 and DHCP6) are
// generated by the DHCP client (the UEFI network stack in this case). By the time this function executes,
// the DHCP sequence already has been executed once (see UEFI Specification Figures 24.2 and 24.3), with
// Private->Dhcp6Request being a cached copy of the DHCP6 request packet that UEFI network stack previously
// generated and sent.
//
// Therefore while this code looks like it could overflow, in practice it's not possible.
//
while (RequestLen < Request->Length) {
OpCode = NTOHS (((EFI_DHCP6_PACKET_OPTION *)RequestOpt)->OpCode);
OpLen = NTOHS (((EFI_DHCP6_PACKET_OPTION *)RequestOpt)->OpLen);
if ((OpCode != EFI_DHCP6_IA_TYPE_NA) &&
(OpCode != EFI_DHCP6_IA_TYPE_TA))
{
if (DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN > DiscoverLenNeeded) {
Status = EFI_OUT_OF_RESOURCES;
goto ON_ERROR;
}
//
// Copy all the options except IA option.
//
CopyMem (DiscoverOpt, RequestOpt, OpLen + 4);
DiscoverOpt += (OpLen + 4);
DiscoverLen += (OpLen + 4);
CopyMem (DiscoverOpt, RequestOpt, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
}
RequestOpt += (OpLen + 4);
RequestLen += (OpLen + 4);
RequestOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
RequestLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
}
Status = PxeBc->UdpWrite (

17
NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
View File

@ -34,6 +34,23 @@
#define PXEBC_ADDR_START_DELIMITER '['
#define PXEBC_ADDR_END_DELIMITER ']'
//
// A DUID consists of a 2-octet type code represented in network byte
// order, followed by a variable number of octets that make up the
// actual identifier. The length of the DUID (not including the type
// code) is at least 1 octet and at most 128 octets.
//
#define PXEBC_MIN_SIZE_OF_DUID (sizeof(UINT16) + 1)
#define PXEBC_MAX_SIZE_OF_DUID (sizeof(UINT16) + 128)
//
// This define represents the combineds code and length field from
// https://datatracker.ietf.org/doc/html/rfc3315#section-22.1
//
#define PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN \
(sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpCode) + \
sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpLen))
#define GET_NEXT_DHCP6_OPTION(Opt) \
(EFI_DHCP6_PACKET_OPTION *) ((UINT8 *) (Opt) + \
sizeof (EFI_DHCP6_PACKET_OPTION) + (NTOHS ((Opt)->OpLen)) - 1)