Re: [edk2-devel] [PATCH v2 07/15] NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45231 Unit Tests

["Saloni Kasbekar" <saloni.kasbekar@intel.com>]

Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>

-----Original Message-----
From: Douglas Flick [MSFT] <doug.edk2@gmail.com> 
Sent: Thursday, January 25, 2024 1:55 PM
To: devel@edk2.groups.io
Cc: Doug Flick <dougflick@microsoft.com>; Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary <zachary.clark-williams@intel.com>; Doug Flick [MSFT] <doug.edk2@gmail.com>
Subject: [PATCH v2 07/15] NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45231 Unit Tests

From: Doug Flick <dougflick@microsoft.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4536

Validates that the patch for...

Out-of-bounds read when handling a ND Redirect message with truncated options

.. has been fixed

Tests the following function to ensure that an out of bounds read does not occur Ip6OptionValidation

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 +
 .../Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf    |  42 ++++++
 .../Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp    |  20 +++
 .../Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp | 129 ++++++++++++++++++
 4 files changed, 192 insertions(+)
 create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf
 create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp
 create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp

diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc
index 24dee654df2e..7fa7b0f9d5be 100644
--- a/NetworkPkg/Test/NetworkPkgHostTest.dsc
+++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc
@@ -26,6 +26,7 @@ [Components]
   # Build HOST_APPLICATION that tests NetworkPkg   #   NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf+  NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.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/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf
new file mode 100644
index 000000000000..6e4de0745fb5
--- /dev/null
+++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf
@@ -0,0 +1,42 @@
+## @file+# Unit test suite for the Ip6Dxe using Google Test+#+# Copyright (c) Microsoft Corporation.<BR>+# SPDX-License-Identifier: BSD-2-Clause-Patent+##+[Defines]+  INF_VERSION         = 0x00010017+  BASE_NAME           = Ip6DxeUnitTest+  FILE_GUID           = 4F05D17D-D3E7-4AAE-820C-576D46D2D34A+  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]+  Ip6DxeGoogleTest.cpp+  Ip6OptionGoogleTest.cpp+  ../Ip6Option.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]+  gZeroGuiddiff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp
new file mode 100644
index 000000000000..6ebfd5fdfb70
--- /dev/null
+++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp
@@ -0,0 +1,20 @@
+/** @file+  Acts as the main entry point for the tests for the Ip6Dxe module.++  Copyright (c) Microsoft Corporation+  SPDX-License-Identifier: BSD-2-Clause-Patent+**/+#include <gtest/gtest.h>++////////////////////////////////////////////////////////////////////////////////+// Run the tests+////////////////////////////////////////////////////////////////////////////////+int+main (+  int   argc,+  char  *argv[]+  )+{+  testing::InitGoogleTest (&argc, argv);+  return RUN_ALL_TESTS ();+}diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp
new file mode 100644
index 000000000000..f2cd90e1a952
--- /dev/null
+++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp
@@ -0,0 +1,129 @@
+/** @file+  Tests for Ip6Option.c.++  Copyright (c) Microsoft Corporation+  SPDX-License-Identifier: BSD-2-Clause-Patent+**/+#include <gtest/gtest.h>++extern "C" {+  #include <Uefi.h>+  #include <Library/BaseLib.h>+  #include <Library/DebugLib.h>+  #include "../Ip6Impl.h"+  #include "../Ip6Option.h"+}++/////////////////////////////////////////////////////////////////////////+// Defines+///////////////////////////////////////////////////////////////////////++#define IP6_PREFIX_INFO_OPTION_DATA_LEN    32+#define OPTION_HEADER_IP6_PREFIX_DATA_LEN  (sizeof (IP6_OPTION_HEADER) + IP6_PREFIX_INFO_OPTION_DATA_LEN)++////////////////////////////////////////////////////////////////////////+// Symbol Definitions+// These functions are not directly under test - but required to compile+////////////////////////////////////////////////////////////////////////+UINT32  mIp6Id;++EFI_STATUS+Ip6SendIcmpError (+  IN IP6_SERVICE       *IpSb,+  IN NET_BUF           *Packet,+  IN EFI_IPv6_ADDRESS  *SourceAddress       OPTIONAL,+  IN EFI_IPv6_ADDRESS  *DestinationAddress,+  IN UINT8             Type,+  IN UINT8             Code,+  IN UINT32            *Pointer             OPTIONAL+  )+{+  // ..+  return EFI_SUCCESS;+}++////////////////////////////////////////////////////////////////////////+// Ip6OptionValidation Tests+////////////////////////////////////////////////////////////////////////++// Define a fixture for your tests if needed+class Ip6OptionValidationTest : 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:+// Null option should return false+TEST_F (Ip6OptionValidationTest, NullOptionShouldReturnFalse) {+  UINT8   *option   = nullptr;+  UINT16  optionLen = 10; // Provide a suitable length++  EXPECT_FALSE (Ip6IsNDOptionValid (option, optionLen));+}++// Test Description:+// Truncated option should return false+TEST_F (Ip6OptionValidationTest, TruncatedOptionShouldReturnFalse) {+  UINT8   option[]  = { 0x01 }; // Provide a truncated option+  UINT16  optionLen = 1;++  EXPECT_FALSE (Ip6IsNDOptionValid (option, optionLen));+}++// Test Description:+// Ip6OptionPrefixInfo Option with zero length should return false+TEST_F (Ip6OptionValidationTest, OptionWithZeroLengthShouldReturnFalse) {+  IP6_OPTION_HEADER  optionHeader;++  optionHeader.Type   = Ip6OptionPrefixInfo;+  optionHeader.Length = 0;+  UINT8  option[sizeof (IP6_OPTION_HEADER)];++  CopyMem (option, &optionHeader, sizeof (IP6_OPTION_HEADER));+  UINT16  optionLen = sizeof (IP6_OPTION_HEADER);++  EXPECT_FALSE (Ip6IsNDOptionValid (option, optionLen));+}++// Test Description:+// Ip6OptionPrefixInfo Option with valid length should return true+TEST_F (Ip6OptionValidationTest, ValidPrefixInfoOptionShouldReturnTrue) {+  IP6_OPTION_HEADER  optionHeader;++  optionHeader.Type   = Ip6OptionPrefixInfo;+  optionHeader.Length = 4; // Length 4 * 8 = 32+  UINT8  option[OPTION_HEADER_IP6_PREFIX_DATA_LEN];++  CopyMem (option, &optionHeader, sizeof (IP6_OPTION_HEADER));++  EXPECT_TRUE (Ip6IsNDOptionValid (option, IP6_PREFIX_INFO_OPTION_DATA_LEN));+}++// Test Description:+// Ip6OptionPrefixInfo Option with invalid length should return false+TEST_F (Ip6OptionValidationTest, InvalidPrefixInfoOptionLengthShouldReturnFalse) {+  IP6_OPTION_HEADER  optionHeader;++  optionHeader.Type   = Ip6OptionPrefixInfo;+  optionHeader.Length = 3; // Length 3 * 8 = 24 (Invalid)+  UINT8  option[sizeof (IP6_OPTION_HEADER)];++  CopyMem (option, &optionHeader, sizeof (IP6_OPTION_HEADER));+  UINT16  optionLen = sizeof (IP6_OPTION_HEADER);++  EXPECT_FALSE (Ip6IsNDOptionValid (option, optionLen));+}-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114960): https://edk2.groups.io/g/devel/message/114960
Mute This Topic: https://groups.io/mt/103964982/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-