REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4534
SECURITY PATCH - Unit Tests
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 <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/Dhcp6DxeGoogleTest.inf | 1 +
.../Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h | 58 +++
NetworkPkg/Dhcp6Dxe/Dhcp6Io.c | 2 +-
.../Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp | 365 +++++++++++++++++-
5 files changed, 424 insertions(+), 3 deletions(-)
create mode 100644 NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h
diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc
index 20bc90b1728d..24dee654df2e 100644
--- a/NetworkPkg/Test/NetworkPkgHostTest.dsc
+++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc
@@ -16,6 +16,7 @@ [Defines]
SKUID_IDENTIFIER = DEFAULT
!include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
+
[Packages]
MdePkg/MdePkg.dec
UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf
index 8e9119a37158..12532ed30cb3 100644
--- a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf
+++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf
@@ -18,6 +18,7 @@ [Defines]
[Sources]
Dhcp6DxeGoogleTest.cpp
Dhcp6IoGoogleTest.cpp
+ Dhcp6IoGoogleTest.h
../Dhcp6Io.c
../Dhcp6Utility.c
diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h
new file mode 100644
index 000000000000..aed3b890827b
--- /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 <Uefi.h>
+
+/**
+ 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/Dhcp6Dxe/Dhcp6Io.c b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c
index 89d16484a568..3b8feb4a2032 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/Dhcp6IoGoogleTest.cpp b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp
index 7ee40e4af480..7db253a7b87f 100644
--- a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp
+++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp
@@ -13,6 +13,7 @@ extern "C" {
#include <Library/BaseMemoryLib.h>
#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);
+}
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114256): https://edk2.groups.io/g/devel/message/114256
Mute This Topic: https://groups.io/mt/103926734/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.