From 5f3658197bf29c83b3349b0ab1d99cdb0c3814bc Mon Sep 17 00:00:00 2001 From: "Doug Flick via groups.io" Date: Fri, 26 Jan 2024 05:54:45 +0800 Subject: [PATCH] NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45230 Unit Tests REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4535 Confirms that reported issue... "Buffer overflow in the DHCPv6 client via a long Server ID option" ..has been corrected by the provided patch. Tests the following functions to ensure they appropriately handle untrusted data (either too long or too small) to prevent a buffer overflow: Dhcp6AppendOption Dhcp6AppendETOption Dhcp6AppendIaOption Cc: Saloni Kasbekar Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] Reviewed-by: Saloni Kasbekar --- .../GoogleTest/Dhcp6DxeGoogleTest.cpp | 20 + .../GoogleTest/Dhcp6DxeGoogleTest.inf | 43 ++ .../Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp | 478 ++++++++++++++++++ NetworkPkg/Test/NetworkPkgHostTest.dsc | 1 + 4 files changed, 542 insertions(+) create mode 100644 NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp create mode 100644 NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf create mode 100644 NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp new file mode 100644 index 000000000000..9aeced2f9156 --- /dev/null +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp @@ -0,0 +1,20 @@ +/** @file + Acts as the main entry point for the tests for the Dhcp6Dxe 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/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf new file mode 100644 index 000000000000..8e9119a37158 --- /dev/null +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf @@ -0,0 +1,43 @@ +## @file +# Unit test suite for the Dhcp6Dxe using Google Test +# +# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent +## +[Defines] + INF_VERSION = 0x00010017 + BASE_NAME = Dhcp6DxeGoogleTest + FILE_GUID = 1D2A4C65-38C8-4C2F-BB60-B5FA49625AA9 + VERSION_STRING = 1.0 + MODULE_TYPE = HOST_APPLICATION +# +# The following information is for reference only and not required by the build tools. +# +# VALID_ARCHITECTURES = IA32 X64 AARCH64 +# +[Sources] + Dhcp6DxeGoogleTest.cpp + Dhcp6IoGoogleTest.cpp + ../Dhcp6Io.c + ../Dhcp6Utility.c + +[Packages] + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec + UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec + NetworkPkg/NetworkPkg.dec + +[LibraryClasses] + GoogleTestLib + DebugLib + NetLib + PcdLib + +[Protocols] + gEfiDhcp6ServiceBindingProtocolGuid + +[Pcd] + gEfiNetworkPkgTokenSpaceGuid.PcdDhcp6UidType + +[Guids] + gZeroGuid diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp new file mode 100644 index 000000000000..7ee40e4af480 --- /dev/null +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp @@ -0,0 +1,478 @@ +/** @file + Tests for Dhcp6Io.c. + + Copyright (c) Microsoft Corporation + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ +#include + +extern "C" { + #include + #include + #include + #include + #include "../Dhcp6Impl.h" + #include "../Dhcp6Utility.h" +} + +//////////////////////////////////////////////////////////////////////// +// Defines +//////////////////////////////////////////////////////////////////////// + +#define DHCP6_PACKET_MAX_LEN 1500 + +//////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////// +// Symbol Definitions +// These functions are not directly under test - but required to compile +//////////////////////////////////////////////////////////////////////// + +// This definition is used by this test but is also required to compile +// by Dhcp6Io.c +EFI_IPv6_ADDRESS mAllDhcpRelayAndServersAddress = { + { 0xFF, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 2 } +}; + +EFI_STATUS +EFIAPI +UdpIoSendDatagram ( + IN UDP_IO *UdpIo, + IN NET_BUF *Packet, + IN UDP_END_POINT *EndPoint OPTIONAL, + IN EFI_IP_ADDRESS *Gateway OPTIONAL, + IN UDP_IO_CALLBACK CallBack, + IN VOID *Context + ) +{ + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI +UdpIoRecvDatagram ( + IN UDP_IO *UdpIo, + IN UDP_IO_CALLBACK CallBack, + IN VOID *Context, + IN UINT32 HeadLen + ) +{ + return EFI_SUCCESS; +} + +//////////////////////////////////////////////////////////////////////// +// Dhcp6AppendOptionTest Tests +//////////////////////////////////////////////////////////////////////// + +class Dhcp6AppendOptionTest : public ::testing::Test { +public: + UINT8 *Buffer = NULL; + EFI_DHCP6_PACKET *Packet; + +protected: + // Add any setup code if needed + virtual void + SetUp ( + ) + { + // Initialize any resources or variables + Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); + ASSERT_NE (Buffer, (UINT8 *)NULL); + + Packet = (EFI_DHCP6_PACKET *)Buffer; + Packet->Size = DHCP6_PACKET_MAX_LEN; + } + + // Add any cleanup code if needed + virtual void + TearDown ( + ) + { + // Clean up any resources or variables + if (Buffer != NULL) { + FreePool (Buffer); + } + } +}; + +// Test Description: +// Attempt to append an option to a packet that is too small by a duid that is too large +TEST_F (Dhcp6AppendOptionTest, InvalidDataExpectBufferTooSmall) { + UINT8 *Cursor; + EFI_DHCP6_DUID *UntrustedDuid; + EFI_STATUS Status; + + UntrustedDuid = (EFI_DHCP6_DUID *)AllocateZeroPool (sizeof (EFI_DHCP6_DUID)); + ASSERT_NE (UntrustedDuid, (EFI_DHCP6_DUID *)NULL); + + UntrustedDuid->Length = NTOHS (0xFFFF); + + Cursor = Dhcp6AppendOptionTest::Packet->Dhcp6.Option; + + Status = Dhcp6AppendOption ( + Dhcp6AppendOptionTest::Packet, + &Cursor, + HTONS (Dhcp6OptServerId), + UntrustedDuid->Length, + UntrustedDuid->Duid + ); + + ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); +} + +// Test Description: +// Attempt to append an option to a packet that is large enough +TEST_F (Dhcp6AppendOptionTest, ValidDataExpectSuccess) { + UINT8 *Cursor; + EFI_DHCP6_DUID *UntrustedDuid; + EFI_STATUS Status; + UINTN OriginalLength; + + UINT8 Duid[6] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 }; + + Packet->Length = sizeof (EFI_DHCP6_HEADER); + OriginalLength = Packet->Length; + + UntrustedDuid = (EFI_DHCP6_DUID *)AllocateZeroPool (sizeof (EFI_DHCP6_DUID)); + ASSERT_NE (UntrustedDuid, (EFI_DHCP6_DUID *)NULL); + + UntrustedDuid->Length = NTOHS (sizeof (Duid)); + CopyMem (UntrustedDuid->Duid, Duid, sizeof (Duid)); + + Cursor = Dhcp6AppendOptionTest::Packet->Dhcp6.Option; + + Status = Dhcp6AppendOption ( + Dhcp6AppendOptionTest::Packet, + &Cursor, + HTONS (Dhcp6OptServerId), + UntrustedDuid->Length, + UntrustedDuid->Duid + ); + + ASSERT_EQ (Status, EFI_SUCCESS); + + // verify that the pointer to cursor moved by the expected amount + ASSERT_EQ (Cursor, (UINT8 *)Dhcp6AppendOptionTest::Packet->Dhcp6.Option + sizeof (Duid) + 4); + + // verify that the length of the packet is now the expected amount + ASSERT_EQ (Dhcp6AppendOptionTest::Packet->Length, OriginalLength + sizeof (Duid) + 4); +} + +//////////////////////////////////////////////////////////////////////// +// Dhcp6AppendETOption Tests +//////////////////////////////////////////////////////////////////////// + +class Dhcp6AppendETOptionTest : public ::testing::Test { +public: + UINT8 *Buffer = NULL; + EFI_DHCP6_PACKET *Packet; + +protected: + // Add any setup code if needed + virtual void + SetUp ( + ) + { + // Initialize any resources or variables + Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); + ASSERT_NE (Buffer, (UINT8 *)NULL); + + Packet = (EFI_DHCP6_PACKET *)Buffer; + Packet->Size = DHCP6_PACKET_MAX_LEN; + Packet->Length = sizeof (EFI_DHCP6_HEADER); + } + + // Add any cleanup code if needed + virtual void + TearDown ( + ) + { + // Clean up any resources or variables + if (Buffer != NULL) { + FreePool (Buffer); + } + } +}; + +// Test Description: +// Attempt to append an option to a packet that is too small by a duid that is too large +TEST_F (Dhcp6AppendETOptionTest, InvalidDataExpectBufferTooSmall) { + UINT8 *Cursor; + EFI_STATUS Status; + DHCP6_INSTANCE Instance; + UINT16 ElapsedTimeVal; + UINT16 *ElapsedTime; + + Cursor = Dhcp6AppendETOptionTest::Packet->Dhcp6.Option; + ElapsedTime = &ElapsedTimeVal; + + Packet->Length = Packet->Size - 2; + + Status = Dhcp6AppendETOption ( + Dhcp6AppendETOptionTest::Packet, + &Cursor, + &Instance, // Instance is not used in this function + &ElapsedTime + ); + + // verify that we error out because the packet is too small for the option header + ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); + + // reset the length + Packet->Length = sizeof (EFI_DHCP6_HEADER); +} + +// Test Description: +// Attempt to append an option to a packet that is large enough +TEST_F (Dhcp6AppendETOptionTest, ValidDataExpectSuccess) { + UINT8 *Cursor; + EFI_STATUS Status; + DHCP6_INSTANCE Instance; + UINT16 ElapsedTimeVal; + UINT16 *ElapsedTime; + UINTN ExpectedSize; + UINTN OriginalLength; + + Cursor = Dhcp6AppendETOptionTest::Packet->Dhcp6.Option; + ElapsedTime = &ElapsedTimeVal; + ExpectedSize = 6; + OriginalLength = Packet->Length; + + Status = Dhcp6AppendETOption ( + Dhcp6AppendETOptionTest::Packet, + &Cursor, + &Instance, // Instance is not used in this function + &ElapsedTime + ); + + // verify that the status is EFI_SUCCESS + ASSERT_EQ (Status, EFI_SUCCESS); + + // verify that the pointer to cursor moved by the expected amount + ASSERT_EQ (Cursor, (UINT8 *)Dhcp6AppendETOptionTest::Packet->Dhcp6.Option + ExpectedSize); + + // verify that the length of the packet is now the expected amount + ASSERT_EQ (Dhcp6AppendETOptionTest::Packet->Length, OriginalLength + ExpectedSize); +} + +//////////////////////////////////////////////////////////////////////// +// Dhcp6AppendIaOption Tests +//////////////////////////////////////////////////////////////////////// + +class Dhcp6AppendIaOptionTest : public ::testing::Test { +public: + UINT8 *Buffer = NULL; + EFI_DHCP6_PACKET *Packet; + EFI_DHCP6_IA *Ia; + +protected: + // Add any setup code if needed + virtual void + SetUp ( + ) + { + // Initialize any resources or variables + Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); + ASSERT_NE (Buffer, (UINT8 *)NULL); + + Packet = (EFI_DHCP6_PACKET *)Buffer; + Packet->Size = DHCP6_PACKET_MAX_LEN; + + Ia = (EFI_DHCP6_IA *)AllocateZeroPool (sizeof (EFI_DHCP6_IA) + sizeof (EFI_DHCP6_IA_ADDRESS) * 2); + ASSERT_NE (Ia, (EFI_DHCP6_IA *)NULL); + + CopyMem (Ia->IaAddress, mAllDhcpRelayAndServersAddress.Addr, sizeof (EFI_IPv6_ADDRESS)); + CopyMem (Ia->IaAddress + 1, mAllDhcpRelayAndServersAddress.Addr, sizeof (EFI_IPv6_ADDRESS)); + + Ia->IaAddressCount = 2; + } + + // Add any cleanup code if needed + virtual void + TearDown ( + ) + { + // Clean up any resources or variables + if (Buffer != NULL) { + FreePool (Buffer); + } + + if (Ia != NULL) { + FreePool (Ia); + } + } +}; + +// Test Description: +// Attempt to append an option to a packet that doesn't have enough space +// for the option header +TEST_F (Dhcp6AppendIaOptionTest, IaNaInvalidDataExpectBufferTooSmall) { + UINT8 *Cursor; + EFI_STATUS Status; + + Packet->Length = Packet->Size - 2; + + Ia->Descriptor.Type = Dhcp6OptIana; + Ia->Descriptor.IaId = 0x12345678; + + Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; + + Status = Dhcp6AppendIaOption ( + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0x12345678, + 0x11111111, + Dhcp6OptIana + ); + + // verify that we error out because the packet is too small for the option header + ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); + + // reset the length + Packet->Length = sizeof (EFI_DHCP6_HEADER); +} + +// Test Description: +// Attempt to append an option to a packet that doesn't have enough space +// for the option header +TEST_F (Dhcp6AppendIaOptionTest, IaTaInvalidDataExpectBufferTooSmall) { + UINT8 *Cursor; + EFI_STATUS Status; + + // Use up nearly all the space in the packet + Packet->Length = Packet->Size - 2; + + Ia->Descriptor.Type = Dhcp6OptIata; + Ia->Descriptor.IaId = 0x12345678; + + Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; + + Status = Dhcp6AppendIaOption ( + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0, + 0, + Dhcp6OptIata + ); + + // verify that we error out because the packet is too small for the option header + ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); + + // reset the length + Packet->Length = sizeof (EFI_DHCP6_HEADER); +} + +TEST_F (Dhcp6AppendIaOptionTest, IaNaValidDataExpectSuccess) { + UINT8 *Cursor; + EFI_STATUS Status; + UINTN ExpectedSize; + UINTN OriginalLength; + + // + // 2 bytes for the option header type + // + ExpectedSize = 2; + // + // 2 bytes for the option header length + // + ExpectedSize += 2; + // + // 4 bytes for the IAID + // + ExpectedSize += 4; + // + // + 4 bytes for the T1 + // + ExpectedSize += 4; + // + // + 4 bytes for the T2 + // + ExpectedSize += 4; + // + // + (4 + sizeof (EFI_DHCP6_IA_ADDRESS)) * 2; + // + 2 bytes for the option header type + // + 2 bytes for the option header length + // + sizeof (EFI_DHCP6_IA_ADDRESS) for the IA Address + // + ExpectedSize += (4 + sizeof (EFI_DHCP6_IA_ADDRESS)) * 2; + + Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; + + Packet->Length = sizeof (EFI_DHCP6_HEADER); + OriginalLength = Packet->Length; + + Ia->Descriptor.Type = Dhcp6OptIana; + Ia->Descriptor.IaId = 0x12345678; + + Status = Dhcp6AppendIaOption ( + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0x12345678, + 0x12345678, + Dhcp6OptIana + ); + + // verify that the pointer to cursor moved by the expected amount + ASSERT_EQ (Cursor, (UINT8 *)Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option + ExpectedSize); + + // verify that the length of the packet is now the expected amount + ASSERT_EQ (Dhcp6AppendIaOptionTest::Packet->Length, OriginalLength + ExpectedSize); + + // verify that the status is EFI_SUCCESS + ASSERT_EQ (Status, EFI_SUCCESS); +} + +TEST_F (Dhcp6AppendIaOptionTest, IaTaValidDataExpectSuccess) { + UINT8 *Cursor; + EFI_STATUS Status; + UINTN ExpectedSize; + UINTN OriginalLength; + + // + // 2 bytes for the option header type + // + ExpectedSize = 2; + // + // 2 bytes for the option header length + // + ExpectedSize += 2; + // + // 4 bytes for the IAID + // + ExpectedSize += 4; + // + // + (4 + sizeof (EFI_DHCP6_IA_ADDRESS)) * 2; + // + 2 bytes for the option header type + // + 2 bytes for the option header length + // + sizeof (EFI_DHCP6_IA_ADDRESS) for the IA Address + // + ExpectedSize += (4 + sizeof (EFI_DHCP6_IA_ADDRESS)) * 2; + + Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; + + Packet->Length = sizeof (EFI_DHCP6_HEADER); + OriginalLength = Packet->Length; + + Ia->Descriptor.Type = Dhcp6OptIata; + Ia->Descriptor.IaId = 0x12345678; + + Status = Dhcp6AppendIaOption ( + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0, + 0, + Dhcp6OptIata + ); + + // verify that the pointer to cursor moved by the expected amount + ASSERT_EQ (Cursor, (UINT8 *)Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option + ExpectedSize); + + // verify that the length of the packet is now the expected amount + ASSERT_EQ (Dhcp6AppendIaOptionTest::Packet->Length, OriginalLength + ExpectedSize); + + // verify that the status is EFI_SUCCESS + ASSERT_EQ (Status, EFI_SUCCESS); +} diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc index 1aeca5c5b353..20bc90b1728d 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -24,6 +24,7 @@ # # Build HOST_APPLICATION that tests NetworkPkg # + NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf # Despite these library classes being listed in [LibraryClasses] below, they are not needed for the host-based unit tests. [LibraryClasses]