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>

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
index 425e0cf8061de1392178a03ef7de211d7457f913..2b2d372889a3541a9632b26e83d625038e0d14e6 100644 (file)
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
@@ -3,6 +3,7 @@
 \r
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>\r
   Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>\r
+  Copyright (c) Microsoft Corporation\r
 \r
   SPDX-License-Identifier: BSD-2-Clause-Patent\r
 \r
@@ -1312,6 +1313,65 @@ PxeBcSelectDhcp6Offer (
   }\r
 }\r
 \r
+/**\r
+  Cache the DHCPv6 DNS Server addresses\r
+\r
+  @param[in] Private               The pointer to PXEBC_PRIVATE_DATA.\r
+  @param[in] Cache6                The pointer to PXEBC_DHCP6_PACKET_CACHE.\r
+\r
+  @retval    EFI_SUCCESS           Cache the DHCPv6 DNS Server address successfully.\r
+  @retval    EFI_OUT_OF_RESOURCES  Failed to allocate resources.\r
+  @retval    EFI_DEVICE_ERROR      The DNS Server Address Length provided by a untrusted\r
+                                   option is not a multiple of 16 bytes (sizeof (EFI_IPv6_ADDRESS)).\r
+**/\r
+EFI_STATUS\r
+PxeBcCacheDnsServerAddresses (\r
+  IN PXEBC_PRIVATE_DATA        *Private,\r
+  IN PXEBC_DHCP6_PACKET_CACHE  *Cache6\r
+  )\r
+{\r
+  UINT16  DnsServerLen;\r
+\r
+  DnsServerLen = NTOHS (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen);\r
+  //\r
+  // Make sure that the number is nonzero\r
+  //\r
+  if (DnsServerLen == 0) {\r
+    return EFI_DEVICE_ERROR;\r
+  }\r
+\r
+  //\r
+  // Make sure the DnsServerlen is a multiple of EFI_IPv6_ADDRESS (16)\r
+  //\r
+  if (DnsServerLen % sizeof (EFI_IPv6_ADDRESS) != 0) {\r
+    return EFI_DEVICE_ERROR;\r
+  }\r
+\r
+  //\r
+  // This code is currently written to only support a single DNS Server instead\r
+  // of multiple such as is spec defined (RFC3646, Section 3). The proper behavior\r
+  // would be to allocate the full space requested, CopyMem all of the data,\r
+  // and then add a DnsServerCount field to Private and update additional code\r
+  // that depends on this.\r
+  //\r
+  // To support multiple DNS servers the `AllocationSize` would need to be changed to DnsServerLen\r
+  //\r
+  // This is tracked in https://bugzilla.tianocore.org/show_bug.cgi?id=1886\r
+  //\r
+  Private->DnsServer = AllocateZeroPool (sizeof (EFI_IPv6_ADDRESS));\r
+  if (Private->DnsServer == NULL) {\r
+    return EFI_OUT_OF_RESOURCES;\r
+  }\r
+\r
+  //\r
+  // Intentionally only copy over the first server address.\r
+  // To support multiple DNS servers, the `Length` would need to be changed to DnsServerLen\r
+  //\r
+  CopyMem (Private->DnsServer, Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, sizeof (EFI_IPv6_ADDRESS));\r
+\r
+  return EFI_SUCCESS;\r
+}\r
+\r
 /**\r
   Handle the DHCPv6 offer packet.\r
 \r
@@ -1335,6 +1395,7 @@ PxeBcHandleDhcp6Offer (
   UINT32                    SelectIndex;\r
   UINT32                    Index;\r
 \r
+  ASSERT (Private != NULL);\r
   ASSERT (Private->SelectIndex > 0);\r
   SelectIndex = (UINT32)(Private->SelectIndex - 1);\r
   ASSERT (SelectIndex < PXEBC_OFFER_MAX_NUM);\r
@@ -1342,15 +1403,13 @@ PxeBcHandleDhcp6Offer (
   Status = EFI_SUCCESS;\r
 \r
   //\r
-  // First try to cache DNS server address if DHCP6 offer provides.\r
+  // First try to cache DNS server addresses if DHCP6 offer provides.\r
   //\r
   if (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] != NULL) {\r
-    Private->DnsServer = AllocateZeroPool (NTOHS (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen));\r
-    if (Private->DnsServer == NULL) {\r
-      return EFI_OUT_OF_RESOURCES;\r
+    Status = PxeBcCacheDnsServerAddresses (Private, Cache6);\r
+    if (EFI_ERROR (Status)) {\r
+      return Status;\r
     }\r
-\r
-    CopyMem (Private->DnsServer, Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, sizeof (EFI_IPv6_ADDRESS));\r
   }\r
 \r
   if (Cache6->OfferType == PxeOfferTypeDhcpBinl) {\r