[edk2-devel] [PATCH 10/14] NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45234 Unit Tests

["Doug Flick via groups.io" <dougflick=microsoft.com@groups.io>]

From: Doug Flick <dougflick@microsoft.com>

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

SECURITY PATCH - Unit Tests

TCBZ4539
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

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>
---
 NetworkPkg/Test/NetworkPkgHostTest.dsc        |   1 +
 .../GoogleTest/UefiPxeBcDxeGoogleTest.inf     |  48 +++
 .../GoogleTest/PxeBcDhcp6GoogleTest.h         |  50 +++
 .../GoogleTest/PxeBcDhcp6GoogleTest.cpp       | 300 ++++++++++++++++++
 .../GoogleTest/UefiPxeBcDxeGoogleTest.cpp     |  19 ++
 5 files changed, 418 insertions(+)
 create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf
 create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h
 create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp
 create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp

diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc
index 7fa7b0f9d5be..a0273c431025 100644
--- a/NetworkPkg/Test/NetworkPkgHostTest.dsc
+++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc
@@ -27,6 +27,7 @@ [Components]
   #
   NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf
   NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf
+  NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf
 
 # Despite these library classes being listed in [LibraryClasses] below, they are not needed for the host-based unit tests.
 [LibraryClasses]
diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf
new file mode 100644
index 000000000000..301dcdf61109
--- /dev/null
+++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf
@@ -0,0 +1,48 @@
+## @file
+# Unit test suite for the UefiPxeBcDxe using Google Test
+#
+# Copyright (c) Microsoft Corporation.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+[Defines]
+INF_VERSION    = 0x00010005
+BASE_NAME      = UefiPxeBcDxeGoogleTest
+FILE_GUID      = 77D45C64-EC1E-4174-887B-886E89FD1EDF
+MODULE_TYPE    = HOST_APPLICATION
+VERSION_STRING = 1.0
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Sources]
+  UefiPxeBcDxeGoogleTest.cpp
+  PxeBcDhcp6GoogleTest.cpp
+  PxeBcDhcp6GoogleTest.h
+  ../PxeBcDhcp6.c
+  ../PxeBcSupport.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
+  NetworkPkg/NetworkPkg.dec
+
+[LibraryClasses]
+  GoogleTestLib
+  DebugLib
+  NetLib
+  PcdLib
+
+[Protocols]
+  gEfiDhcp6ServiceBindingProtocolGuid
+  gEfiDns6ServiceBindingProtocolGuid
+  gEfiDns6ProtocolGuid
+
+[Pcd]
+  gEfiNetworkPkgTokenSpaceGuid.PcdDhcp6UidType
+
+[Guids]
+  gZeroGuid
diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h
new file mode 100644
index 000000000000..b17c314791c8
--- /dev/null
+++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h
@@ -0,0 +1,50 @@
+/** @file
+  This file exposes the internal interfaces which may be unit tested
+  for the PxeBcDhcp6Dxe driver.
+
+  Copyright (c) Microsoft Corporation.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef PXE_BC_DHCP6_GOOGLE_TEST_H_
+#define PXE_BC_DHCP6_GOOGLE_TEST_H_
+
+//
+// Minimal includes needed to compile
+//
+#include <Uefi.h>
+#include "../PxeBcImpl.h"
+
+/**
+  Handle the DHCPv6 offer packet.
+
+  @param[in]  Private             The pointer to PXEBC_PRIVATE_DATA.
+
+  @retval     EFI_SUCCESS           Handled the DHCPv6 offer packet successfully.
+  @retval     EFI_NO_RESPONSE       No response to the following request packet.
+  @retval     EFI_OUT_OF_RESOURCES  Failed to allocate resources.
+  @retval     EFI_BUFFER_TOO_SMALL  Can't cache the offer pacet.
+
+**/
+EFI_STATUS
+PxeBcHandleDhcp6Offer (
+  IN PXEBC_PRIVATE_DATA  *Private
+  );
+
+/**
+  Cache the DHCPv6 Server address
+
+  @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 Server address successfully.
+  @retval    EFI_OUT_OF_RESOURCES  Failed to allocate resources.
+  @retval    EFI_DEVICE_ERROR      Failed to cache the DHCPv6 Server address.
+**/
+EFI_STATUS
+PxeBcCacheDnsServerAddresses (
+  IN PXEBC_PRIVATE_DATA        *Private,
+  IN PXEBC_DHCP6_PACKET_CACHE  *Cache6
+  );
+
+#endif // PXE_BC_DHCP6_GOOGLE_TEST_H_
diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp
new file mode 100644
index 000000000000..8260eeee50dc
--- /dev/null
+++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp
@@ -0,0 +1,300 @@
+/** @file
+  Host based unit test for PxeBcDhcp6.c.
+
+  Copyright (c) Microsoft Corporation
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include <gtest/gtest.h>
+
+extern "C" {
+  #include <Uefi.h>
+  #include <Library/BaseLib.h>
+  #include <Library/DebugLib.h>
+  #include "../PxeBcImpl.h"
+  #include "../PxeBcDhcp6.h"
+  #include "PxeBcDhcp6GoogleTest.h"
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// Definitions
+///////////////////////////////////////////////////////////////////////////////
+
+#define PACKET_SIZE  (1500)
+
+typedef struct {
+  UINT16    OptionCode;   // The option code for DHCP6_OPT_SERVER_ID (e.g., 0x03)
+  UINT16    OptionLen;    // The length of the option (e.g., 16 bytes)
+  UINT8     ServerId[16]; // The 16-byte DHCPv6 Server Identifier
+} DHCP6_OPTION_SERVER_ID;
+
+///////////////////////////////////////////////////////////////////////////////
+/// Symbol Definitions
+///////////////////////////////////////////////////////////////////////////////
+
+EFI_STATUS
+MockUdpWrite (
+  IN EFI_PXE_BASE_CODE_PROTOCOL      *This,
+  IN UINT16                          OpFlags,
+  IN EFI_IP_ADDRESS                  *DestIp,
+  IN EFI_PXE_BASE_CODE_UDP_PORT      *DestPort,
+  IN EFI_IP_ADDRESS                  *GatewayIp   OPTIONAL,
+  IN EFI_IP_ADDRESS                  *SrcIp       OPTIONAL,
+  IN OUT EFI_PXE_BASE_CODE_UDP_PORT  *SrcPort     OPTIONAL,
+  IN UINTN                           *HeaderSize  OPTIONAL,
+  IN VOID                            *HeaderPtr   OPTIONAL,
+  IN UINTN                           *BufferSize,
+  IN VOID                            *BufferPtr
+  )
+{
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+MockUdpRead (
+  IN EFI_PXE_BASE_CODE_PROTOCOL      *This,
+  IN UINT16                          OpFlags,
+  IN OUT EFI_IP_ADDRESS              *DestIp      OPTIONAL,
+  IN OUT EFI_PXE_BASE_CODE_UDP_PORT  *DestPort    OPTIONAL,
+  IN OUT EFI_IP_ADDRESS              *SrcIp       OPTIONAL,
+  IN OUT EFI_PXE_BASE_CODE_UDP_PORT  *SrcPort     OPTIONAL,
+  IN UINTN                           *HeaderSize  OPTIONAL,
+  IN VOID                            *HeaderPtr   OPTIONAL,
+  IN OUT UINTN                       *BufferSize,
+  IN VOID                            *BufferPtr
+  )
+{
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+MockConfigure (
+  IN EFI_UDP6_PROTOCOL     *This,
+  IN EFI_UDP6_CONFIG_DATA  *UdpConfigData OPTIONAL
+  )
+{
+  return EFI_SUCCESS;
+}
+
+// Needed by PxeBcSupport
+EFI_STATUS
+EFIAPI
+QueueDpc (
+  IN EFI_TPL            DpcTpl,
+  IN EFI_DPC_PROCEDURE  DpcProcedure,
+  IN VOID               *DpcContext    OPTIONAL
+  )
+{
+  return EFI_SUCCESS;
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// PxeBcHandleDhcp6OfferTest Tests
+///////////////////////////////////////////////////////////////////////////////
+
+class PxeBcHandleDhcp6OfferTest : public ::testing::Test {
+public:
+  PXEBC_PRIVATE_DATA Private = { 0 };
+  EFI_UDP6_PROTOCOL Udp6Read;
+  EFI_PXE_BASE_CODE_MODE Mode = { 0 };
+
+protected:
+  // Add any setup code if needed
+  virtual void
+  SetUp (
+    )
+  {
+    Private.Dhcp6Request = (EFI_DHCP6_PACKET *)AllocateZeroPool (PACKET_SIZE);
+
+    // Need to setup the EFI_PXE_BASE_CODE_PROTOCOL
+    // The function under test really only needs the following:
+    //  UdpWrite
+    //  UdpRead
+
+    Private.PxeBc.UdpWrite = (EFI_PXE_BASE_CODE_UDP_WRITE)MockUdpWrite;
+    Private.PxeBc.UdpRead  = (EFI_PXE_BASE_CODE_UDP_READ)MockUdpRead;
+
+    // Need to setup EFI_UDP6_PROTOCOL
+    // The function under test really only needs the following:
+    //  Configure
+
+    Udp6Read.Configure = (EFI_UDP6_CONFIGURE)MockConfigure;
+    Private.Udp6Read   = &Udp6Read;
+
+    // Need to setup the EFI_PXE_BASE_CODE_MODE
+    Private.PxeBc.Mode = &Mode;
+
+    // for this test it doesn't really matter what the Dhcpv6 ack is set to
+  }
+
+  // Add any cleanup code if needed
+  virtual void
+  TearDown (
+    )
+  {
+    if (Private.Dhcp6Request != NULL) {
+      FreePool (Private.Dhcp6Request);
+    }
+
+    // Clean up any resources or variables
+  }
+};
+
+// Note:
+// Testing PxeBcHandleDhcp6Offer() is difficult because it depends on a
+// properly setup Private structure. Attempting to properly test this function
+// without a signficant refactor is a fools errand. Instead, we will test
+// that we can prevent an overflow in the function.
+TEST_F (PxeBcHandleDhcp6OfferTest, BasicUsageTest) {
+  PXEBC_DHCP6_PACKET_CACHE  *Cache6 = NULL;
+  EFI_DHCP6_PACKET_OPTION   Option  = { 0 };
+
+  Private.SelectIndex = 1; // SelectIndex is 1-based
+  Cache6              = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6;
+
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option;
+  // Setup the DHCPv6 offer packet
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID;
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen  = NTOHS (1337);
+
+  ASSERT_EQ (PxeBcHandleDhcp6Offer (&(PxeBcHandleDhcp6OfferTest::Private)), EFI_DEVICE_ERROR);
+}
+
+class PxeBcCacheDnsServerAddressesTest : public ::testing::Test {
+public:
+  PXEBC_PRIVATE_DATA Private = { 0 };
+
+protected:
+  // Add any setup code if needed
+  virtual void
+  SetUp (
+    )
+  {
+  }
+
+  // Add any cleanup code if needed
+  virtual void
+  TearDown (
+    )
+  {
+  }
+};
+
+// Test Description
+// Test that we cache the DNS server address from the DHCPv6 offer packet
+TEST_F (PxeBcCacheDnsServerAddressesTest, BasicUsageTest) {
+  UINT8                     SearchPattern[16] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF };
+  EFI_DHCP6_PACKET_OPTION   *Option;
+  PXEBC_DHCP6_PACKET_CACHE  *Cache6 = NULL;
+
+  Option = (EFI_DHCP6_PACKET_OPTION *)AllocateZeroPool (sizeof (EFI_DHCP6_PACKET_OPTION) + sizeof (SearchPattern));
+  ASSERT_NE (Option, nullptr);
+
+  Option->OpCode = DHCP6_OPT_SERVER_ID;
+  Option->OpLen  = NTOHS (sizeof (SearchPattern));
+  CopyMem (Option->Data, SearchPattern, sizeof (SearchPattern));
+
+  Private.SelectIndex                         = 1; // SelectIndex is 1-based
+  Cache6                                      = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6;
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = Option;
+
+  Private.DnsServer = nullptr;
+
+  ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_SUCCESS);
+  ASSERT_NE (Private.DnsServer, nullptr);
+  ASSERT_EQ (CompareMem (Private.DnsServer, SearchPattern, sizeof (SearchPattern)), 0);
+
+  if (Private.DnsServer) {
+    FreePool (Private.DnsServer);
+  }
+
+  if (Option) {
+    FreePool (Option);
+  }
+}
+// Test Description
+// Test that we can prevent an overflow in the function
+TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptOverflowTest) {
+  EFI_DHCP6_PACKET_OPTION   Option  = { 0 };
+  PXEBC_DHCP6_PACKET_CACHE  *Cache6 = NULL;
+
+  Private.SelectIndex                         = 1; // SelectIndex is 1-based
+  Cache6                                      = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6;
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option;
+  // Setup the DHCPv6 offer packet
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID;
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen  = NTOHS (1337);
+
+  Private.DnsServer = NULL;
+
+  ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_DEVICE_ERROR);
+  ASSERT_EQ (Private.DnsServer, nullptr);
+
+  if (Private.DnsServer) {
+    FreePool (Private.DnsServer);
+  }
+}
+
+// Test Description
+// Test that we can prevent an underflow in the function
+TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptUnderflowTest) {
+  EFI_DHCP6_PACKET_OPTION   Option  = { 0 };
+  PXEBC_DHCP6_PACKET_CACHE  *Cache6 = NULL;
+
+  Private.SelectIndex                         = 1; // SelectIndex is 1-based
+  Cache6                                      = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6;
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option;
+  // Setup the DHCPv6 offer packet
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID;
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen  = NTOHS (2);
+
+  Private.DnsServer = NULL;
+
+  ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_DEVICE_ERROR);
+  ASSERT_EQ (Private.DnsServer, nullptr);
+
+  if (Private.DnsServer) {
+    FreePool (Private.DnsServer);
+  }
+}
+
+// Test Description
+// Test that we can handle recursive dns (multiple dns entries)
+TEST_F (PxeBcCacheDnsServerAddressesTest, MultipleDnsEntries) {
+  EFI_DHCP6_PACKET_OPTION   Option  = { 0 };
+  PXEBC_DHCP6_PACKET_CACHE  *Cache6 = NULL;
+
+  Private.SelectIndex                         = 1; // SelectIndex is 1-based
+  Cache6                                      = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6;
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option;
+  // Setup the DHCPv6 offer packet
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID;
+
+  EFI_IPv6_ADDRESS  addresses[2] = {
+    // 2001:db8:85a3::8a2e:370:7334
+    { 0x20, 0x01, 0x0d, 0xb8, 0x85, 0xa3, 0x00, 0x00, 0x00, 0x00, 0x8a, 0x2e, 0x03, 0x70, 0x73, 0x34 },
+    // fe80::d478:91c3:ecd7:4ff9
+    { 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xd4, 0x78, 0x91, 0xc3, 0xec, 0xd7, 0x4f, 0xf9 }
+  };
+
+  CopyMem (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, &addresses, sizeof (addresses));
+
+  Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (sizeof (addresses));
+
+  Private.DnsServer = NULL;
+
+  ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_SUCCESS);
+
+  ASSERT_NE (Private.DnsServer, nullptr);
+
+  //
+  // This is expected to fail until DnsServer supports multiple DNS servers
+  //
+  // This is tracked in https://bugzilla.tianocore.org/show_bug.cgi?id=1886
+  //
+  // Disabling:
+  // ASSERT_EQ (CompareMem(Private.DnsServer, &addresses, sizeof(addresses)), 0);
+
+  if (Private.DnsServer) {
+    FreePool (Private.DnsServer);
+  }
+}
diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp
new file mode 100644
index 000000000000..cc4fdf525b62
--- /dev/null
+++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp
@@ -0,0 +1,19 @@
+/** @file
+  Acts as the main entry point for the tests for the UefiPxeBcDxe module.
+  Copyright (c) Microsoft Corporation
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include <gtest/gtest.h>
+
+////////////////////////////////////////////////////////////////////////////////
+// Run the tests
+////////////////////////////////////////////////////////////////////////////////
+int
+main (
+  int   argc,
+  char  *argv[]
+  )
+{
+  testing::InitGoogleTest (&argc, argv);
+  return RUN_ALL_TESTS ();
+}
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114262): https://edk2.groups.io/g/devel/message/114262
Mute This Topic: https://groups.io/mt/103926741/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-