From 02266fef2b5320a41f79cebd4b6c5a83f1036427 Mon Sep 17 00:00:00 2001 From: Jon Maloy Date: Fri, 16 Feb 2024 10:48:05 -0500 Subject: [PATCH 2/3] NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45234 Patch RH-Author: Jon Maloy RH-MergeRequest: 60: NetworkPkg: Apply uncrustify changes RH-Jira: RHEL-22002 RH-Acked-by: Gerd Hoffmann RH-Acked-by: Oliver Steffen RH-Commit: [2/3] 3d71d4ca4e6cc0deb9af78b3b7901a93120de210 (jmaloy/jons_fork) JIRA: https://issues.redhat.com/browse/RHEL-22002 CVE: CVE-2022-45234 Upstream: Merged commit 1b53515d53d303166b2bbd31e2cc7f16fd0aecd7 Author: Doug Flick Date: Fri Jan 26 05:54:52 2024 +0800 NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45234 Patch REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4539 Bug Details: PixieFail Bug #6 CVE-2023-45234 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 processing DNS Servers option in a DHCPv6 Advertise message Change Overview: Introduces a function to cache the Dns Server and perform sanitizing on the incoming DnsServerLen to ensure that the length is valid > + EFI_STATUS > + PxeBcCacheDnsServerAddresses ( > + IN PXEBC_PRIVATE_DATA *Private, > + IN PXEBC_DHCP6_PACKET_CACHE *Cache6 > + ) Additional code cleanup Cc: Saloni Kasbekar Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] Reviewed-by: Saloni Kasbekar Signed-off-by: Jon Maloy --- NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 71 +++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 6 deletions(-) diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c index 425e0cf806..2b2d372889 100644 --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c @@ -3,6 +3,7 @@ (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) Microsoft Corporation SPDX-License-Identifier: BSD-2-Clause-Patent @@ -1312,6 +1313,65 @@ PxeBcSelectDhcp6Offer ( } } +/** + Cache the DHCPv6 DNS Server addresses + + @param[in] Private The pointer to PXEBC_PRIVATE_DATA. + @param[in] Cache6 The pointer to PXEBC_DHCP6_PACKET_CACHE. + + @retval EFI_SUCCESS Cache the DHCPv6 DNS Server address successfully. + @retval EFI_OUT_OF_RESOURCES Failed to allocate resources. + @retval EFI_DEVICE_ERROR The DNS Server Address Length provided by a untrusted + option is not a multiple of 16 bytes (sizeof (EFI_IPv6_ADDRESS)). +**/ +EFI_STATUS +PxeBcCacheDnsServerAddresses ( + IN PXEBC_PRIVATE_DATA *Private, + IN PXEBC_DHCP6_PACKET_CACHE *Cache6 + ) +{ + UINT16 DnsServerLen; + + DnsServerLen = NTOHS (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen); + // + // Make sure that the number is nonzero + // + if (DnsServerLen == 0) { + return EFI_DEVICE_ERROR; + } + + // + // Make sure the DnsServerlen is a multiple of EFI_IPv6_ADDRESS (16) + // + if (DnsServerLen % sizeof (EFI_IPv6_ADDRESS) != 0) { + return EFI_DEVICE_ERROR; + } + + // + // This code is currently written to only support a single DNS Server instead + // of multiple such as is spec defined (RFC3646, Section 3). The proper behavior + // would be to allocate the full space requested, CopyMem all of the data, + // and then add a DnsServerCount field to Private and update additional code + // that depends on this. + // + // To support multiple DNS servers the `AllocationSize` would need to be changed to DnsServerLen + // + // This is tracked in https://bugzilla.tianocore.org/show_bug.cgi?id=1886 + // + Private->DnsServer = AllocateZeroPool (sizeof (EFI_IPv6_ADDRESS)); + if (Private->DnsServer == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + // + // Intentionally only copy over the first server address. + // To support multiple DNS servers, the `Length` would need to be changed to DnsServerLen + // + CopyMem (Private->DnsServer, Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, sizeof (EFI_IPv6_ADDRESS)); + + return EFI_SUCCESS; +} + /** Handle the DHCPv6 offer packet. @@ -1335,6 +1395,7 @@ PxeBcHandleDhcp6Offer ( UINT32 SelectIndex; UINT32 Index; + ASSERT (Private != NULL); ASSERT (Private->SelectIndex > 0); SelectIndex = (UINT32)(Private->SelectIndex - 1); ASSERT (SelectIndex < PXEBC_OFFER_MAX_NUM); @@ -1342,15 +1403,13 @@ PxeBcHandleDhcp6Offer ( Status = EFI_SUCCESS; // - // First try to cache DNS server address if DHCP6 offer provides. + // First try to cache DNS server addresses if DHCP6 offer provides. // if (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] != NULL) { - Private->DnsServer = AllocateZeroPool (NTOHS (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen)); - if (Private->DnsServer == NULL) { - return EFI_OUT_OF_RESOURCES; + Status = PxeBcCacheDnsServerAddresses (Private, Cache6); + if (EFI_ERROR (Status)) { + return Status; } - - CopyMem (Private->DnsServer, Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, sizeof (EFI_IPv6_ADDRESS)); } if (Cache6->OfferType == PxeOfferTypeDhcpBinl) { -- 2.41.0