From 07362769ab7a7d74dbea1c7a7a3662c7b5d1f097 Mon Sep 17 00:00:00 2001 From: "Doug Flick via groups.io" Date: Fri, 26 Jan 2024 05:54:47 +0800 Subject: NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Unit Tests REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4534 These tests confirm that the report bug... "Out-of-bounds read when processing IA_NA/IA_TA options in a DHCPv6 Advertise message" ..has been patched. The following functions are tested to confirm an out of bounds read is patched and that the correct statuses are returned: Dhcp6SeekInnerOptionSafe Dhcp6SeekStsOption TCBZ4534 CVE-2023-45229 CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N CWE-125 Out-of-bounds Read Cc: Saloni Kasbekar Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] Reviewed-by: Saloni Kasbekar --- NetworkPkg/Dhcp6Dxe/Dhcp6Io.c | 2 +- .../Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf | 1 + .../Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp | 365 ++++++++++++++++++++- NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h | 58 ++++ NetworkPkg/Test/NetworkPkgHostTest.dsc | 1 + 5 files changed, 424 insertions(+), 3 deletions(-) create mode 100644 NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c index 89d1648..3b8feb4 100644 --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c @@ -816,7 +816,7 @@ Dhcp6SeekStsOption ( // IA option to the end of the DHCP6 option area, thus subtract the space // up until this option // - OptionLen = OptionLen - (*Option - Packet->Dhcp6.Option); + OptionLen = OptionLen - (UINT32)(*Option - Packet->Dhcp6.Option); // // Seek the inner option diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf index 8e9119a..12532ed 100644 --- a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf @@ -18,6 +18,7 @@ [Sources] Dhcp6DxeGoogleTest.cpp Dhcp6IoGoogleTest.cpp + Dhcp6IoGoogleTest.h ../Dhcp6Io.c ../Dhcp6Utility.c diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp index 7ee40e4..7db253a 100644 --- a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp @@ -13,6 +13,7 @@ extern "C" { #include #include "../Dhcp6Impl.h" #include "../Dhcp6Utility.h" + #include "Dhcp6IoGoogleTest.h" } //////////////////////////////////////////////////////////////////////// @@ -21,7 +22,35 @@ extern "C" { #define DHCP6_PACKET_MAX_LEN 1500 +// This definition is used by this test but is also required to compile +// by Dhcp6Io.c +#define DHCPV6_OPTION_IA_NA 3 +#define DHCPV6_OPTION_IA_TA 4 + +#define SEARCH_PATTERN 0xDEADC0DE +#define SEARCH_PATTERN_LEN sizeof(SEARCH_PATTERN) + //////////////////////////////////////////////////////////////////////// +// Test structures for IA_NA and IA_TA options +//////////////////////////////////////////////////////////////////////// +typedef struct { + UINT16 Code; + UINT16 Len; + UINT32 IAID; +} DHCPv6_OPTION; + +typedef struct { + DHCPv6_OPTION Header; + UINT32 T1; + UINT32 T2; + UINT8 InnerOptions[0]; +} DHCPv6_OPTION_IA_NA; + +typedef struct { + DHCPv6_OPTION Header; + UINT8 InnerOptions[0]; +} DHCPv6_OPTION_IA_TA; + //////////////////////////////////////////////////////////////////////// // Symbol Definitions // These functions are not directly under test - but required to compile @@ -210,7 +239,7 @@ TEST_F (Dhcp6AppendETOptionTest, InvalidDataExpectBufferTooSmall) { Status = Dhcp6AppendETOption ( Dhcp6AppendETOptionTest::Packet, &Cursor, - &Instance, // Instance is not used in this function + &Instance, // Instance is not used in this function &ElapsedTime ); @@ -240,7 +269,7 @@ TEST_F (Dhcp6AppendETOptionTest, ValidDataExpectSuccess) { Status = Dhcp6AppendETOption ( Dhcp6AppendETOptionTest::Packet, &Cursor, - &Instance, // Instance is not used in this function + &Instance, // Instance is not used in this function &ElapsedTime ); @@ -476,3 +505,335 @@ TEST_F (Dhcp6AppendIaOptionTest, IaTaValidDataExpectSuccess) { // verify that the status is EFI_SUCCESS ASSERT_EQ (Status, EFI_SUCCESS); } + +//////////////////////////////////////////////////////////////////////// +// Dhcp6SeekInnerOptionSafe Tests +//////////////////////////////////////////////////////////////////////// + +// Define a fixture for your tests if needed +class Dhcp6SeekInnerOptionSafeTest : public ::testing::Test { +protected: + // Add any setup code if needed + virtual void + SetUp ( + ) + { + // Initialize any resources or variables + } + + // Add any cleanup code if needed + virtual void + TearDown ( + ) + { + // Clean up any resources or variables + } +}; + +// Test Description: +// This test verifies that Dhcp6SeekInnerOptionSafe returns EFI_SUCCESS when the IANA option is found. +TEST_F (Dhcp6SeekInnerOptionSafeTest, IANAValidOptionExpectSuccess) { + EFI_STATUS Result; + UINT8 Option[sizeof (DHCPv6_OPTION_IA_NA) + SEARCH_PATTERN_LEN] = { 0 }; + UINT32 OptionLength = sizeof (Option); + DHCPv6_OPTION_IA_NA *OptionPtr = (DHCPv6_OPTION_IA_NA *)Option; + UINT32 SearchPattern = SEARCH_PATTERN; + + UINTN SearchPatternLength = SEARCH_PATTERN_LEN; + UINT8 *InnerOptionPtr = NULL; + UINT16 InnerOptionLength = 0; + + OptionPtr->Header.Code = Dhcp6OptIana; + OptionPtr->Header.Len = HTONS (4 + 12); // Valid length has to be more than 12 + OptionPtr->Header.IAID = 0x12345678; + OptionPtr->T1 = 0x11111111; + OptionPtr->T2 = 0x22222222; + CopyMem (OptionPtr->InnerOptions, &SearchPattern, SearchPatternLength); + + Result = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIana, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Result, EFI_SUCCESS); + ASSERT_EQ (InnerOptionLength, 4); + ASSERT_EQ (CompareMem (InnerOptionPtr, &SearchPattern, SearchPatternLength), 0); +} + +// Test Description: +// This test verifies that Dhcp6SeekInnerOptionSafe returns EFI_DEIVCE_ERROR when the IANA option size is invalid. +TEST_F (Dhcp6SeekInnerOptionSafeTest, IANAInvalidSizeExpectFail) { + // Lets add an inner option of bytes we expect to find + EFI_STATUS Status; + UINT8 Option[sizeof (DHCPv6_OPTION_IA_NA) + SEARCH_PATTERN_LEN] = { 0 }; + UINT32 OptionLength = sizeof (Option); + DHCPv6_OPTION_IA_NA *OptionPtr = (DHCPv6_OPTION_IA_NA *)Option; + UINT32 SearchPattern = SEARCH_PATTERN; + + UINTN SearchPatternLength = SEARCH_PATTERN_LEN; + UINT8 *InnerOptionPtr = NULL; + UINT16 InnerOptionLength = 0; + + OptionPtr->Header.Code = Dhcp6OptIana; + OptionPtr->Header.Len = HTONS (4); // Set the length to lower than expected (12) + OptionPtr->Header.IAID = 0x12345678; + OptionPtr->T1 = 0x11111111; + OptionPtr->T2 = 0x22222222; + CopyMem (OptionPtr->InnerOptions, &SearchPattern, SearchPatternLength); + + // Set the InnerOptionLength to be less than the size of the option + Status = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIana, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Status, EFI_DEVICE_ERROR); + + // Now set the OptionLength to be less than the size of the option + OptionLength = sizeof (DHCPv6_OPTION_IA_NA) - 1; + Status = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIana, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Status, EFI_DEVICE_ERROR); +} + +// Test Description: +// This test verifies that Dhcp6SeekInnerOptionSafe returns EFI_SUCCESS when the IATA option is found +TEST_F (Dhcp6SeekInnerOptionSafeTest, IATAValidOptionExpectSuccess) { + // Lets add an inner option of bytes we expect to find + EFI_STATUS Status; + UINT8 Option[sizeof (DHCPv6_OPTION_IA_TA) + SEARCH_PATTERN_LEN] = { 0 }; + UINT32 OptionLength = sizeof (Option); + DHCPv6_OPTION_IA_TA *OptionPtr = (DHCPv6_OPTION_IA_TA *)Option; + UINT32 SearchPattern = SEARCH_PATTERN; + + UINTN SearchPatternLength = SEARCH_PATTERN_LEN; + UINT8 *InnerOptionPtr = NULL; + UINT16 InnerOptionLength = 0; + + OptionPtr->Header.Code = Dhcp6OptIata; + OptionPtr->Header.Len = HTONS (4 + 4); // Valid length has to be more than 4 + OptionPtr->Header.IAID = 0x12345678; + CopyMem (OptionPtr->InnerOptions, &SearchPattern, SearchPatternLength); + + Status = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIata, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Status, EFI_SUCCESS); + ASSERT_EQ (InnerOptionLength, 4); + ASSERT_EQ (CompareMem (InnerOptionPtr, &SearchPattern, SearchPatternLength), 0); +} + +// Test Description: +// This test verifies that Dhcp6SeekInnerOptionSafe returns EFI_SUCCESS when the IATA option size is invalid. +TEST_F (Dhcp6SeekInnerOptionSafeTest, IATAInvalidSizeExpectFail) { + // Lets add an inner option of bytes we expect to find + EFI_STATUS Status; + UINT8 Option[sizeof (DHCPv6_OPTION_IA_TA) + SEARCH_PATTERN_LEN] = { 0 }; + UINT32 OptionLength = sizeof (Option); + DHCPv6_OPTION_IA_TA *OptionPtr = (DHCPv6_OPTION_IA_TA *)Option; + UINT32 SearchPattern = SEARCH_PATTERN; + + UINTN SearchPatternLength = SEARCH_PATTERN_LEN; + UINT8 *InnerOptionPtr = NULL; + UINT16 InnerOptionLength = 0; + + OptionPtr->Header.Code = Dhcp6OptIata; + OptionPtr->Header.Len = HTONS (2); // Set the length to lower than expected (4) + OptionPtr->Header.IAID = 0x12345678; + CopyMem (OptionPtr->InnerOptions, &SearchPattern, SearchPatternLength); + + Status = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIata, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Status, EFI_DEVICE_ERROR); + + // Now lets try modifying the OptionLength to be less than the size of the option + OptionLength = sizeof (DHCPv6_OPTION_IA_TA) - 1; + Status = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIata, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Status, EFI_DEVICE_ERROR); +} + +// Test Description: +// This test verifies that any other Option Type fails +TEST_F (Dhcp6SeekInnerOptionSafeTest, InvalidOption) { + // Lets add an inner option of bytes we expect to find + EFI_STATUS Result; + UINT8 Option[sizeof (DHCPv6_OPTION_IA_TA) + SEARCH_PATTERN_LEN] = { 0 }; + UINT32 OptionLength = sizeof (Option); + DHCPv6_OPTION_IA_TA *OptionPtr = (DHCPv6_OPTION_IA_TA *)Option; + UINT32 SearchPattern = SEARCH_PATTERN; + + UINTN SearchPatternLength = SEARCH_PATTERN_LEN; + UINT8 *InnerOptionPtr = NULL; + UINT16 InnerOptionLength = 0; + + OptionPtr->Header.Code = 0xC0DE; + OptionPtr->Header.Len = HTONS (2); // Set the length to lower than expected (4) + OptionPtr->Header.IAID = 0x12345678; + CopyMem (OptionPtr->InnerOptions, &SearchPattern, SearchPatternLength); + + Result = Dhcp6SeekInnerOptionSafe (0xC0DE, Option, OptionLength, &InnerOptionPtr, &InnerOptionLength); + ASSERT_EQ (Result, EFI_DEVICE_ERROR); +} + +//////////////////////////////////////////////////////////////////////// +// Dhcp6SeekStsOption Tests +//////////////////////////////////////////////////////////////////////// + +#define PACKET_SIZE (1500) + +class Dhcp6SeekStsOptionTest : public ::testing::Test { +public: + DHCP6_INSTANCE Instance = { 0 }; + EFI_DHCP6_PACKET *Packet = NULL; + EFI_DHCP6_CONFIG_DATA Config = { 0 }; + +protected: + // Add any setup code if needed + virtual void + SetUp ( + ) + { + // Allocate a packet + Packet = (EFI_DHCP6_PACKET *)AllocateZeroPool (PACKET_SIZE); + ASSERT_NE (Packet, nullptr); + + // Initialize the packet + Packet->Size = PACKET_SIZE; + + Instance.Config = &Config; + } + + // Add any cleanup code if needed + virtual void + TearDown ( + ) + { + // Clean up any resources or variables + FreePool (Packet); + } +}; + +// Test Description: +// This test verifies that Dhcp6SeekStsOption returns EFI_DEVICE_ERROR when the option is invalid +// This verifies that the calling function is working as expected +TEST_F (Dhcp6SeekStsOptionTest, SeekIATAOptionExpectFail) { + EFI_STATUS Status; + UINT8 *Option = NULL; + UINT32 SearchPattern = SEARCH_PATTERN; + UINT16 SearchPatternLength = SEARCH_PATTERN_LEN; + UINT16 *Len = NULL; + EFI_DHCP6_IA Ia = { 0 }; + + Ia.Descriptor.Type = DHCPV6_OPTION_IA_TA; + Ia.IaAddressCount = 1; + Ia.IaAddress[0].PreferredLifetime = 0xDEADBEEF; + Ia.IaAddress[0].ValidLifetime = 0xDEADAAAA; + Ia.IaAddress[0].IpAddress = mAllDhcpRelayAndServersAddress; + + Packet->Length = sizeof (EFI_DHCP6_HEADER); + + Option = Dhcp6SeekStsOptionTest::Packet->Dhcp6.Option; + + // Let's append the option to the packet + Status = Dhcp6AppendOption ( + Dhcp6SeekStsOptionTest::Packet, + &Option, + Dhcp6OptStatusCode, + SearchPatternLength, + (UINT8 *)&SearchPattern + ); + ASSERT_EQ (Status, EFI_SUCCESS); + + // Inner option length - this will be overwritten later + Len = (UINT16 *)(Option + 2); + + // Fill in the inner IA option + Status = Dhcp6AppendIaOption ( + Dhcp6SeekStsOptionTest::Packet, + &Option, + &Ia, + 0x12345678, + 0x11111111, + 0x22222222 + ); + ASSERT_EQ (Status, EFI_SUCCESS); + + // overwrite the len of inner Ia option + *Len = HTONS (3); + + Dhcp6SeekStsOptionTest::Instance.Config->IaDescriptor.Type = DHCPV6_OPTION_IA_TA; + + Option = NULL; + Status = Dhcp6SeekStsOption (&(Dhcp6SeekStsOptionTest::Instance), Dhcp6SeekStsOptionTest::Packet, &Option); + + ASSERT_EQ (Status, EFI_DEVICE_ERROR); +} + +// Test Description: +// This test verifies that Dhcp6SeekInnerOptionSafe returns EFI_SUCCESS when the IATA option size is invalid. +TEST_F (Dhcp6SeekStsOptionTest, SeekIANAOptionExpectSuccess) { + EFI_STATUS Status = EFI_NOT_FOUND; + UINT8 *Option = NULL; + UINT32 SearchPattern = SEARCH_PATTERN; + UINT16 SearchPatternLength = SEARCH_PATTERN_LEN; + EFI_DHCP6_IA Ia = { 0 }; + + Ia.Descriptor.Type = DHCPV6_OPTION_IA_NA; + Ia.IaAddressCount = 1; + Ia.IaAddress[0].PreferredLifetime = 0x11111111; + Ia.IaAddress[0].ValidLifetime = 0x22222222; + Ia.IaAddress[0].IpAddress = mAllDhcpRelayAndServersAddress; + Packet->Length = sizeof (EFI_DHCP6_HEADER); + + Option = Dhcp6SeekStsOptionTest::Packet->Dhcp6.Option; + + Status = Dhcp6AppendOption ( + Dhcp6SeekStsOptionTest::Packet, + &Option, + Dhcp6OptStatusCode, + SearchPatternLength, + (UINT8 *)&SearchPattern + ); + ASSERT_EQ (Status, EFI_SUCCESS); + + Status = Dhcp6AppendIaOption ( + Dhcp6SeekStsOptionTest::Packet, + &Option, + &Ia, + 0x12345678, + 0x11111111, + 0x22222222 + ); + ASSERT_EQ (Status, EFI_SUCCESS); + + Dhcp6SeekStsOptionTest::Instance.Config->IaDescriptor.Type = DHCPV6_OPTION_IA_NA; + + Option = NULL; + Status = Dhcp6SeekStsOption (&(Dhcp6SeekStsOptionTest::Instance), Dhcp6SeekStsOptionTest::Packet, &Option); + + ASSERT_EQ (Status, EFI_SUCCESS); +} diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h new file mode 100644 index 0000000..aed3b89 --- /dev/null +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h @@ -0,0 +1,58 @@ +/** @file + Acts as header for private functions under test in Dhcp6Io.c + + Copyright (c) Microsoft Corporation + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ + +#ifndef DHCP6_IO_GOOGLE_TEST_H_ +#define DHCP6_IO_GOOGLE_TEST_H_ + +//////////////////////////////////////////////////////////////////////////////// +// These are the functions that are being unit tested +//////////////////////////////////////////////////////////////////////////////// + +#include + +/** + Seeks the Inner Options from a DHCP6 Option + + @param[in] IaType The type of the IA option. + @param[in] Option The pointer to the DHCP6 Option. + @param[in] OptionLen The length of the DHCP6 Option. + @param[out] IaInnerOpt The pointer to the IA inner option. + @param[out] IaInnerLen The length of the IA inner option. + + @retval EFI_SUCCESS Seek the inner option successfully. + @retval EFI_DEVICE_ERROR The OptionLen is invalid. +*/ +EFI_STATUS +Dhcp6SeekInnerOptionSafe ( + UINT16 IaType, + UINT8 *Option, + UINT32 OptionLen, + UINT8 **IaInnerOpt, + UINT16 *IaInnerLen + ); + +/** + Seek StatusCode Option in package. A Status Code option may appear in the + options field of a DHCP message and/or in the options field of another option. + See details in section 22.13, RFC3315. + + @param[in] Instance The pointer to the Dhcp6 instance. + @param[in] Packet The pointer to reply messages. + @param[out] Option The pointer to status code option. + + @retval EFI_SUCCESS Seek status code option successfully. + @retval EFI_DEVICE_ERROR An unexpected error. + +**/ +EFI_STATUS +Dhcp6SeekStsOption ( + IN DHCP6_INSTANCE *Instance, + IN EFI_DHCP6_PACKET *Packet, + OUT UINT8 **Option + ); + +#endif // DHCP6_IO_GOOGLE_TEST_H diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc index 20bc90b..24dee65 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -16,6 +16,7 @@ SKUID_IDENTIFIER = DEFAULT !include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc + [Packages] MdePkg/MdePkg.dec UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec -- cgit v1.1