From fd1bc6ff10a45123b0ec7f9ae3354ad3713bc532 Mon Sep 17 00:00:00 2001 From: Jon Maloy Date: Fri, 16 Feb 2024 10:48:05 -0500 Subject: [PATCH 08/15] NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45234 Unit Tests RH-Author: Jon Maloy RH-MergeRequest: 56: Pixiefail issues in NetworkPkg package RH-Jira: RHEL-21840 RHEL-21844 RHEL-21846 RHEL-21848 RHEL-21850 RHEL-21852 RH-Acked-by: Gerd Hoffmann RH-Acked-by: Oliver Steffen RH-Commit: [8/15] f88ebc7fa79ce4fe615dd79c42fedee0a0da7a0b JIRA: https://issues.redhat.com/browse/RHEL-21850 CVE: CVE-2022-45234 Upstream: Merged commit 458c582685fc0e8057d2511c5a0394078d988c17 Author: Doug Flick Date: Fri Jan 26 05:54:53 2024 +0800 NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45234 Unit Tests REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4539 Unit tests to that the bug.. Buffer overflow when processing DNS Servers option in a DHCPv6 Advertise message ..has been patched This contains tests for the following functions: PxeBcHandleDhcp6Offer PxeBcCacheDnsServerAddresses Cc: Saloni Kasbekar Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] Reviewed-by: Saloni Kasbekar Signed-off-by: Jon Maloy --- NetworkPkg/Test/NetworkPkgHostTest.dsc | 1 + .../GoogleTest/PxeBcDhcp6GoogleTest.cpp | 300 ++++++++++++++++++ .../GoogleTest/PxeBcDhcp6GoogleTest.h | 50 +++ .../GoogleTest/UefiPxeBcDxeGoogleTest.cpp | 19 ++ .../GoogleTest/UefiPxeBcDxeGoogleTest.inf | 48 +++ 5 files changed, 418 insertions(+) create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc index ab7c2857b6..c8a991e5c1 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -26,6 +26,7 @@ # 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/PxeBcDhcp6GoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp new file mode 100644 index 0000000000..8260eeee50 --- /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 + +extern "C" { + #include + #include + #include + #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/PxeBcDhcp6GoogleTest.h b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h new file mode 100644 index 0000000000..b17c314791 --- /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.
+ 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 +#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/UefiPxeBcDxeGoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp new file mode 100644 index 0000000000..cc4fdf525b --- /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 + +//////////////////////////////////////////////////////////////////////////////// +// Run the tests +//////////////////////////////////////////////////////////////////////////////// +int +main ( + int argc, + char *argv[] + ) +{ + testing::InitGoogleTest (&argc, argv); + return RUN_ALL_TESTS (); +} diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf new file mode 100644 index 0000000000..301dcdf611 --- /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.
+# 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 -- 2.39.3