summaryrefslogtreecommitdiffstats
path: root/NetworkPkg
diff options
context:
space:
mode:
authorDoug Flick <dougflick@microsoft.com>2024-01-26 05:54:52 +0800
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>2024-02-06 19:24:26 +0000
commit1b53515d53d303166b2bbd31e2cc7f16fd0aecd7 (patch)
treed7a225dffe09968143c84235101b0d4112fcc1c1 /NetworkPkg
parentc9c87f08dd6ace36fa843424522c3558a8374cac (diff)
downloadedk2-1b53515d53d303166b2bbd31e2cc7f16fd0aecd7.tar.gz
edk2-1b53515d53d303166b2bbd31e2cc7f16fd0aecd7.tar.bz2
edk2-1b53515d53d303166b2bbd31e2cc7f16fd0aecd7.zip
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 <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>
Diffstat (limited to 'NetworkPkg')
-rw-r--r--NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c71
1 files 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.<BR>
Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) Microsoft Corporation
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -1313,6 +1314,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.
@param[in] Private The pointer to PXEBC_PRIVATE_DATA.
@@ -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) {