410 lines
13 KiB
Diff
410 lines
13 KiB
Diff
|
From 59b9d468ebf6be2a5c53d7979c12040f9b41c2c2 Mon Sep 17 00:00:00 2001
|
||
|
From: Jon Maloy <jmaloy@redhat.com>
|
||
|
Date: Fri, 16 Feb 2024 10:48:05 -0500
|
||
|
Subject: [PATCH 13/18] NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45235
|
||
|
Unit Tests
|
||
|
|
||
|
RH-Author: Jon Maloy <jmaloy@redhat.com>
|
||
|
RH-MergeRequest: 54: NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45230 Patch
|
||
|
RH-Jira: RHEL-21841 RHEL-21843 RHEL-21845 RHEL-21847 RHEL-21849 RHEL-21851 RHEL-21853
|
||
|
RH-Acked-by: Gerd Hoffmann <None>
|
||
|
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
|
||
|
RH-Commit: [13/18] 074410155526b2ee2a74cf161ea46385932da059
|
||
|
|
||
|
JIRA: https://issues.redhat.com/browse/RHEL-21853
|
||
|
CVE: CVE-2022-45235
|
||
|
Upstream: Merged
|
||
|
|
||
|
commit ff2986358f75d8f58ef08a66fe673539c9c48f41
|
||
|
Author: Doug Flick <dougflick@microsoft.com>
|
||
|
Date: Fri Jan 26 05:54:56 2024 +0800
|
||
|
|
||
|
NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45235 Unit Tests
|
||
|
|
||
|
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4540
|
||
|
|
||
|
Unit tests to confirm that the bug..
|
||
|
|
||
|
Buffer overflow when handling Server ID option from a DHCPv6 proxy
|
||
|
Advertise message
|
||
|
|
||
|
..has been patched.
|
||
|
|
||
|
This patch contains unit tests for the following functions:
|
||
|
PxeBcRequestBootService
|
||
|
PxeBcDhcp6Discover
|
||
|
|
||
|
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>
|
||
|
|
||
|
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
|
||
|
---
|
||
|
NetworkPkg/Test/NetworkPkgHostTest.dsc | 5 +-
|
||
|
.../GoogleTest/PxeBcDhcp6GoogleTest.cpp | 278 +++++++++++++++++-
|
||
|
.../GoogleTest/PxeBcDhcp6GoogleTest.h | 18 ++
|
||
|
3 files changed, 298 insertions(+), 3 deletions(-)
|
||
|
|
||
|
diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc
|
||
|
index a0273c4310..fa301a7a52 100644
|
||
|
--- a/NetworkPkg/Test/NetworkPkgHostTest.dsc
|
||
|
+++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc
|
||
|
@@ -27,7 +27,10 @@
|
||
|
#
|
||
|
NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf
|
||
|
NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf
|
||
|
- NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf
|
||
|
+ NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf {
|
||
|
+ <LibraryClasses>
|
||
|
+ UefiRuntimeServicesTableLib|MdePkg/Test/Mock/Library/GoogleTest/MockUefiRuntimeServicesTableLib/MockUefiRuntimeServicesTableLib.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
|
||
|
index 8260eeee50..bd423ebadf 100644
|
||
|
--- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp
|
||
|
+++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp
|
||
|
@@ -4,7 +4,9 @@
|
||
|
Copyright (c) Microsoft Corporation
|
||
|
SPDX-License-Identifier: BSD-2-Clause-Patent
|
||
|
**/
|
||
|
-#include <gtest/gtest.h>
|
||
|
+#include <Library/GoogleTestLib.h>
|
||
|
+#include <GoogleTest/Library/MockUefiLib.h>
|
||
|
+#include <GoogleTest/Library/MockUefiRuntimeServicesTableLib.h>
|
||
|
|
||
|
extern "C" {
|
||
|
#include <Uefi.h>
|
||
|
@@ -19,7 +21,8 @@ extern "C" {
|
||
|
// Definitions
|
||
|
///////////////////////////////////////////////////////////////////////////////
|
||
|
|
||
|
-#define PACKET_SIZE (1500)
|
||
|
+#define PACKET_SIZE (1500)
|
||
|
+#define REQUEST_OPTION_LENGTH (120)
|
||
|
|
||
|
typedef struct {
|
||
|
UINT16 OptionCode; // The option code for DHCP6_OPT_SERVER_ID (e.g., 0x03)
|
||
|
@@ -76,6 +79,26 @@ MockConfigure (
|
||
|
}
|
||
|
|
||
|
// Needed by PxeBcSupport
|
||
|
+EFI_STATUS
|
||
|
+PxeBcDns6 (
|
||
|
+ IN PXEBC_PRIVATE_DATA *Private,
|
||
|
+ IN CHAR16 *HostName,
|
||
|
+ OUT EFI_IPv6_ADDRESS *IpAddress
|
||
|
+ )
|
||
|
+{
|
||
|
+ return EFI_SUCCESS;
|
||
|
+}
|
||
|
+
|
||
|
+UINT32
|
||
|
+PxeBcBuildDhcp6Options (
|
||
|
+ IN PXEBC_PRIVATE_DATA *Private,
|
||
|
+ OUT EFI_DHCP6_PACKET_OPTION **OptList,
|
||
|
+ IN UINT8 *Buffer
|
||
|
+ )
|
||
|
+{
|
||
|
+ return EFI_SUCCESS;
|
||
|
+}
|
||
|
+
|
||
|
EFI_STATUS
|
||
|
EFIAPI
|
||
|
QueueDpc (
|
||
|
@@ -159,6 +182,10 @@ TEST_F (PxeBcHandleDhcp6OfferTest, BasicUsageTest) {
|
||
|
ASSERT_EQ (PxeBcHandleDhcp6Offer (&(PxeBcHandleDhcp6OfferTest::Private)), EFI_DEVICE_ERROR);
|
||
|
}
|
||
|
|
||
|
+///////////////////////////////////////////////////////////////////////////////
|
||
|
+// PxeBcCacheDnsServerAddresses Tests
|
||
|
+///////////////////////////////////////////////////////////////////////////////
|
||
|
+
|
||
|
class PxeBcCacheDnsServerAddressesTest : public ::testing::Test {
|
||
|
public:
|
||
|
PXEBC_PRIVATE_DATA Private = { 0 };
|
||
|
@@ -298,3 +325,250 @@ TEST_F (PxeBcCacheDnsServerAddressesTest, MultipleDnsEntries) {
|
||
|
FreePool (Private.DnsServer);
|
||
|
}
|
||
|
}
|
||
|
+
|
||
|
+///////////////////////////////////////////////////////////////////////////////
|
||
|
+// PxeBcRequestBootServiceTest Test Cases
|
||
|
+///////////////////////////////////////////////////////////////////////////////
|
||
|
+
|
||
|
+class PxeBcRequestBootServiceTest : public ::testing::Test {
|
||
|
+public:
|
||
|
+ PXEBC_PRIVATE_DATA Private = { 0 };
|
||
|
+ EFI_UDP6_PROTOCOL Udp6Read;
|
||
|
+
|
||
|
+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;
|
||
|
+ }
|
||
|
+
|
||
|
+ // Add any cleanup code if needed
|
||
|
+ virtual void
|
||
|
+ TearDown (
|
||
|
+ )
|
||
|
+ {
|
||
|
+ if (Private.Dhcp6Request != NULL) {
|
||
|
+ FreePool (Private.Dhcp6Request);
|
||
|
+ }
|
||
|
+
|
||
|
+ // Clean up any resources or variables
|
||
|
+ }
|
||
|
+};
|
||
|
+
|
||
|
+TEST_F (PxeBcRequestBootServiceTest, ServerDiscoverBasicUsageTest) {
|
||
|
+ PxeBcRequestBootServiceTest::Private.OfferBuffer[0].Dhcp6.OfferType = PxeOfferTypeProxyBinl;
|
||
|
+
|
||
|
+ DHCP6_OPTION_SERVER_ID Server = { 0 };
|
||
|
+
|
||
|
+ Server.OptionCode = HTONS (DHCP6_OPT_SERVER_ID);
|
||
|
+ Server.OptionLen = HTONS (16); // valid length
|
||
|
+ UINT8 Index = 0;
|
||
|
+
|
||
|
+ EFI_DHCP6_PACKET *Packet = (EFI_DHCP6_PACKET *)&Private.OfferBuffer[Index].Dhcp6.Packet.Offer;
|
||
|
+
|
||
|
+ UINT8 *Cursor = (UINT8 *)(Packet->Dhcp6.Option);
|
||
|
+
|
||
|
+ CopyMem (Cursor, &Server, sizeof (Server));
|
||
|
+ Cursor += sizeof (Server);
|
||
|
+
|
||
|
+ // Update the packet length
|
||
|
+ Packet->Length = (UINT16)(Cursor - (UINT8 *)Packet);
|
||
|
+ Packet->Size = PACKET_SIZE;
|
||
|
+
|
||
|
+ ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Private), Index), EFI_SUCCESS);
|
||
|
+}
|
||
|
+
|
||
|
+TEST_F (PxeBcRequestBootServiceTest, AttemptDiscoverOverFlowExpectFailure) {
|
||
|
+ PxeBcRequestBootServiceTest::Private.OfferBuffer[0].Dhcp6.OfferType = PxeOfferTypeProxyBinl;
|
||
|
+
|
||
|
+ DHCP6_OPTION_SERVER_ID Server = { 0 };
|
||
|
+
|
||
|
+ Server.OptionCode = HTONS (DHCP6_OPT_SERVER_ID);
|
||
|
+ Server.OptionLen = HTONS (1500); // This length would overflow without a check
|
||
|
+ UINT8 Index = 0;
|
||
|
+
|
||
|
+ EFI_DHCP6_PACKET *Packet = (EFI_DHCP6_PACKET *)&Private.OfferBuffer[Index].Dhcp6.Packet.Offer;
|
||
|
+
|
||
|
+ UINT8 *Cursor = (UINT8 *)(Packet->Dhcp6.Option);
|
||
|
+
|
||
|
+ CopyMem (Cursor, &Server, sizeof (Server));
|
||
|
+ Cursor += sizeof (Server);
|
||
|
+
|
||
|
+ // Update the packet length
|
||
|
+ Packet->Length = (UINT16)(Cursor - (UINT8 *)Packet);
|
||
|
+ Packet->Size = PACKET_SIZE;
|
||
|
+
|
||
|
+ // This is going to be stopped by the duid overflow check
|
||
|
+ ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Private), Index), EFI_INVALID_PARAMETER);
|
||
|
+}
|
||
|
+
|
||
|
+TEST_F (PxeBcRequestBootServiceTest, RequestBasicUsageTest) {
|
||
|
+ EFI_DHCP6_PACKET_OPTION RequestOpt = { 0 }; // the data section doesn't really matter
|
||
|
+
|
||
|
+ RequestOpt.OpCode = HTONS (0x1337);
|
||
|
+ RequestOpt.OpLen = 0; // valid length
|
||
|
+
|
||
|
+ UINT8 Index = 0;
|
||
|
+
|
||
|
+ EFI_DHCP6_PACKET *Packet = (EFI_DHCP6_PACKET *)&Private.Dhcp6Request[Index];
|
||
|
+
|
||
|
+ UINT8 *Cursor = (UINT8 *)(Packet->Dhcp6.Option);
|
||
|
+
|
||
|
+ CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt));
|
||
|
+ Cursor += sizeof (RequestOpt);
|
||
|
+
|
||
|
+ // Update the packet length
|
||
|
+ Packet->Length = (UINT16)(Cursor - (UINT8 *)Packet);
|
||
|
+ Packet->Size = PACKET_SIZE;
|
||
|
+
|
||
|
+ ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Private), Index), EFI_SUCCESS);
|
||
|
+}
|
||
|
+
|
||
|
+TEST_F (PxeBcRequestBootServiceTest, AttemptRequestOverFlowExpectFailure) {
|
||
|
+ EFI_DHCP6_PACKET_OPTION RequestOpt = { 0 }; // the data section doesn't really matter
|
||
|
+
|
||
|
+ RequestOpt.OpCode = HTONS (0x1337);
|
||
|
+ RequestOpt.OpLen = 1500; // this length would overflow without a check
|
||
|
+
|
||
|
+ UINT8 Index = 0;
|
||
|
+
|
||
|
+ EFI_DHCP6_PACKET *Packet = (EFI_DHCP6_PACKET *)&Private.Dhcp6Request[Index];
|
||
|
+
|
||
|
+ UINT8 *Cursor = (UINT8 *)(Packet->Dhcp6.Option);
|
||
|
+
|
||
|
+ CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt));
|
||
|
+ Cursor += sizeof (RequestOpt);
|
||
|
+
|
||
|
+ // Update the packet length
|
||
|
+ Packet->Length = (UINT16)(Cursor - (UINT8 *)Packet);
|
||
|
+ Packet->Size = PACKET_SIZE;
|
||
|
+
|
||
|
+ ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Private), Index), EFI_OUT_OF_RESOURCES);
|
||
|
+}
|
||
|
+
|
||
|
+///////////////////////////////////////////////////////////////////////////////
|
||
|
+// PxeBcDhcp6Discover Test
|
||
|
+///////////////////////////////////////////////////////////////////////////////
|
||
|
+
|
||
|
+class PxeBcDhcp6DiscoverTest : public ::testing::Test {
|
||
|
+public:
|
||
|
+ PXEBC_PRIVATE_DATA Private = { 0 };
|
||
|
+ EFI_UDP6_PROTOCOL Udp6Read;
|
||
|
+
|
||
|
+protected:
|
||
|
+ MockUefiRuntimeServicesTableLib RtServicesMock;
|
||
|
+
|
||
|
+ // 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;
|
||
|
+ }
|
||
|
+
|
||
|
+ // Add any cleanup code if needed
|
||
|
+ virtual void
|
||
|
+ TearDown (
|
||
|
+ )
|
||
|
+ {
|
||
|
+ if (Private.Dhcp6Request != NULL) {
|
||
|
+ FreePool (Private.Dhcp6Request);
|
||
|
+ }
|
||
|
+
|
||
|
+ // Clean up any resources or variables
|
||
|
+ }
|
||
|
+};
|
||
|
+
|
||
|
+// Test Description
|
||
|
+// This will cause an overflow by an untrusted packet during the option parsing
|
||
|
+TEST_F (PxeBcDhcp6DiscoverTest, BasicOverflowTest) {
|
||
|
+ EFI_IPv6_ADDRESS DestIp = { 0 };
|
||
|
+ EFI_DHCP6_PACKET_OPTION RequestOpt = { 0 }; // the data section doesn't really matter
|
||
|
+
|
||
|
+ RequestOpt.OpCode = HTONS (0x1337);
|
||
|
+ RequestOpt.OpLen = HTONS (0xFFFF); // overflow
|
||
|
+
|
||
|
+ UINT8 *Cursor = (UINT8 *)(Private.Dhcp6Request->Dhcp6.Option);
|
||
|
+
|
||
|
+ CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt));
|
||
|
+ Cursor += sizeof (RequestOpt);
|
||
|
+
|
||
|
+ Private.Dhcp6Request->Length = (UINT16)(Cursor - (UINT8 *)Private.Dhcp6Request);
|
||
|
+
|
||
|
+ EXPECT_CALL (RtServicesMock, gRT_GetTime)
|
||
|
+ .WillOnce (::testing::Return (0));
|
||
|
+
|
||
|
+ ASSERT_EQ (
|
||
|
+ PxeBcDhcp6Discover (
|
||
|
+ &(PxeBcDhcp6DiscoverTest::Private),
|
||
|
+ 0,
|
||
|
+ NULL,
|
||
|
+ FALSE,
|
||
|
+ (EFI_IP_ADDRESS *)&DestIp
|
||
|
+ ),
|
||
|
+ EFI_OUT_OF_RESOURCES
|
||
|
+ );
|
||
|
+}
|
||
|
+
|
||
|
+// Test Description
|
||
|
+// This will test that we can handle a packet with a valid option length
|
||
|
+TEST_F (PxeBcDhcp6DiscoverTest, BasicUsageTest) {
|
||
|
+ EFI_IPv6_ADDRESS DestIp = { 0 };
|
||
|
+ EFI_DHCP6_PACKET_OPTION RequestOpt = { 0 }; // the data section doesn't really matter
|
||
|
+
|
||
|
+ RequestOpt.OpCode = HTONS (0x1337);
|
||
|
+ RequestOpt.OpLen = HTONS (0x30);
|
||
|
+
|
||
|
+ UINT8 *Cursor = (UINT8 *)(Private.Dhcp6Request->Dhcp6.Option);
|
||
|
+
|
||
|
+ CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt));
|
||
|
+ Cursor += sizeof (RequestOpt);
|
||
|
+
|
||
|
+ Private.Dhcp6Request->Length = (UINT16)(Cursor - (UINT8 *)Private.Dhcp6Request);
|
||
|
+
|
||
|
+ EXPECT_CALL (RtServicesMock, gRT_GetTime)
|
||
|
+ .WillOnce (::testing::Return (0));
|
||
|
+
|
||
|
+ ASSERT_EQ (
|
||
|
+ PxeBcDhcp6Discover (
|
||
|
+ &(PxeBcDhcp6DiscoverTest::Private),
|
||
|
+ 0,
|
||
|
+ NULL,
|
||
|
+ FALSE,
|
||
|
+ (EFI_IP_ADDRESS *)&DestIp
|
||
|
+ ),
|
||
|
+ EFI_SUCCESS
|
||
|
+ );
|
||
|
+}
|
||
|
diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h
|
||
|
index b17c314791..0d825e4425 100644
|
||
|
--- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h
|
||
|
+++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h
|
||
|
@@ -47,4 +47,22 @@ PxeBcCacheDnsServerAddresses (
|
||
|
IN PXEBC_DHCP6_PACKET_CACHE *Cache6
|
||
|
);
|
||
|
|
||
|
+/**
|
||
|
+ Build and send out the request packet for the bootfile, and parse the reply.
|
||
|
+
|
||
|
+ @param[in] Private The pointer to PxeBc private data.
|
||
|
+ @param[in] Index PxeBc option boot item type.
|
||
|
+
|
||
|
+ @retval EFI_SUCCESS Successfully discovered the boot file.
|
||
|
+ @retval EFI_OUT_OF_RESOURCES Failed to allocate resources.
|
||
|
+ @retval EFI_NOT_FOUND Can't get the PXE reply packet.
|
||
|
+ @retval Others Failed to discover the boot file.
|
||
|
+
|
||
|
+**/
|
||
|
+EFI_STATUS
|
||
|
+PxeBcRequestBootService (
|
||
|
+ IN PXEBC_PRIVATE_DATA *Private,
|
||
|
+ IN UINT32 Index
|
||
|
+ );
|
||
|
+
|
||
|
#endif // PXE_BC_DHCP6_GOOGLE_TEST_H_
|
||
|
--
|
||
|
2.39.3
|
||
|
|