Re: [edk2-devel] [PATCH v2 09/15] NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45232 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 09/15] NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45232 Unit Tests

From: Doug Flick <dougflick@microsoft.com>

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

Unit tests to confirm that..

Infinite loop when parsing unknown options in the Destination Options header

and

Infinite loop when parsing a PadN option in the Destination Options header

... have been patched

This patch tests the following functions:
Ip6IsOptionValid

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>
---
 .../Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf    |  10 +-
 .../Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.h   |  40 +++
 .../Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp | 278 ++++++++++++++++++
 3 files changed, 324 insertions(+), 4 deletions(-)  create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.h

diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf
index 6e4de0745fb5..ba29dbabadb9 100644
--- a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf
+++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf
@@ -1,13 +1,13 @@
 ## @file-# Unit test suite for the Ip6Dxe using Google Test+# Unit test suite for the Ip6DxeGoogleTest 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+  BASE_NAME           = Ip6DxeGoogleTest+  FILE_GUID           = AE39981C-B7FE-41A8-A9C2-F41910477CA3   VERSION_STRING      = 1.0   MODULE_TYPE         = HOST_APPLICATION #@@ -16,9 +16,11 @@ [Defines]
 #  VALID_ARCHITECTURES           = IA32 X64 AARCH64 # [Sources]+  ../Ip6Option.c+  Ip6OptionGoogleTest.h   Ip6DxeGoogleTest.cpp   Ip6OptionGoogleTest.cpp-  ../Ip6Option.c+  Ip6OptionGoogleTest.h  [Packages]   MdePkg/MdePkg.decdiff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.h b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.h
new file mode 100644
index 000000000000..0509b6ae30d2
--- /dev/null
+++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.h
@@ -0,0 +1,40 @@
+/** @file+  Exposes the functions needed to test the Ip6Option module.++  Copyright (c) Microsoft Corporation+  SPDX-License-Identifier: BSD-2-Clause-Patent+**/++#ifndef IP6_OPTION_HEADER_GOOGLE_TEST_H_+#define IP6_OPTION_HEADER_GOOGLE_TEST_H_++#include <Uefi.h>+#include "../Ip6Impl.h"++/**+  Validate the IP6 option format for both the packets we received+  and that we will transmit. It will compute the ICMPv6 error message fields+  if the option is malformatted.++  @param[in]  IpSb              The IP6 service data.+  @param[in]  Packet            The to be validated packet.+  @param[in]  Option            The first byte of the option.+  @param[in]  OptionLen         The length of the whole option.+  @param[in]  Pointer           Identifies the octet offset within+                                the invoking packet where the error was detected.+++  @retval TRUE     The option is properly formatted.+  @retval FALSE    The option is malformatted.++**/+BOOLEAN+Ip6IsOptionValid (+  IN IP6_SERVICE  *IpSb,+  IN NET_BUF      *Packet,+  IN UINT8        *Option,+  IN UINT16       OptionLen,+  IN UINT32       Pointer+  );++#endif // __IP6_OPTION_HEADER_GOOGLE_TEST_H__diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp
index f2cd90e1a952..29f8a4a96e4c 100644
--- a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp
+++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp
@@ -12,6 +12,7 @@ extern "C" {
   #include <Library/DebugLib.h>   #include "../Ip6Impl.h"   #include "../Ip6Option.h"+  #include "Ip6OptionGoogleTest.h" }  /////////////////////////////////////////////////////////////////////////@@ -127,3 +128,280 @@ TEST_F (Ip6OptionValidationTest, InvalidPrefixInfoOptionLengthShouldReturnFalse)
    EXPECT_FALSE (Ip6IsNDOptionValid (option, optionLen)); }++////////////////////////////////////////////////////////////////////////+// Ip6IsOptionValid Tests+////////////////////////////////////////////////////////////////////////++// Define a fixture for your tests if needed+class Ip6IsOptionValidTest : 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+// Verify that a NULL option is Invalid+TEST_F (Ip6IsOptionValidTest, NullOptionShouldReturnTrue) {+  NET_BUF  Packet = { 0 };+  // we need to define enough of the packet to make the function work+  // The function being tested will pass IpSb to Ip6SendIcmpError which is defined above+  IP6_SERVICE  *IpSb = NULL;++  EFI_IPv6_ADDRESS  SourceAddress      = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IPv6_ADDRESS  DestinationAddress = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IP6_HEADER    Ip6Header          = { 0 };++  Ip6Header.SourceAddress      = SourceAddress;+  Ip6Header.DestinationAddress = DestinationAddress;+  Packet.Ip.Ip6                = &Ip6Header;++  EXPECT_FALSE (Ip6IsOptionValid (IpSb, &Packet, NULL, 0, 0));+}++// Test Description+// Verify that an unknown option with a length of 0 and type of <unknown> does not cause an infinite loop+TEST_F (Ip6IsOptionValidTest, VerifyNoInfiniteLoopOnUnknownOptionLength0) {+  NET_BUF  Packet = { 0 };+  // we need to define enough of the packet to make the function work+  // The function being tested will pass IpSb to Ip6SendIcmpError which is defined above+  UINT32  DeadCode = 0xDeadC0de;+  // Don't actually use this pointer, just pass it to the function, nothing will be done with it+  IP6_SERVICE  *IpSb = (IP6_SERVICE *)&DeadCode;++  EFI_IPv6_ADDRESS  SourceAddress      = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IPv6_ADDRESS  DestinationAddress = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IP6_HEADER    Ip6Header          = { 0 };++  Ip6Header.SourceAddress      = SourceAddress;+  Ip6Header.DestinationAddress = DestinationAddress;+  Packet.Ip.Ip6                = &Ip6Header;++  IP6_OPTION_HEADER  optionHeader;++  optionHeader.Type   = 23;   // Unknown Option+  optionHeader.Length = 0;    // This will cause an infinite loop if the function is not working correctly++  // This should be a valid option even though the length is 0+  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));+}++// Test Description+// Verify that an unknown option with a length of 1 and type of <unknown> does not cause an infinite loop+TEST_F (Ip6IsOptionValidTest, VerifyNoInfiniteLoopOnUnknownOptionLength1) {+  NET_BUF  Packet = { 0 };+  // we need to define enough of the packet to make the function work+  // The function being tested will pass IpSb to Ip6SendIcmpError which is defined above+  UINT32  DeadCode = 0xDeadC0de;+  // Don't actually use this pointer, just pass it to the function, nothing will be done with it+  IP6_SERVICE  *IpSb = (IP6_SERVICE *)&DeadCode;++  EFI_IPv6_ADDRESS  SourceAddress      = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IPv6_ADDRESS  DestinationAddress = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IP6_HEADER    Ip6Header          = { 0 };++  Ip6Header.SourceAddress      = SourceAddress;+  Ip6Header.DestinationAddress = DestinationAddress;+  Packet.Ip.Ip6                = &Ip6Header;++  IP6_OPTION_HEADER  optionHeader;++  optionHeader.Type   = 23;   // Unknown Option+  optionHeader.Length = 1;    // This will cause an infinite loop if the function is not working correctly++  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));+}++// Test Description+// Verify that an unknown option with a length of 2 and type of <unknown> does not cause an infinite loop+TEST_F (Ip6IsOptionValidTest, VerifyIpSkipUnknownOption) {+  NET_BUF  Packet = { 0 };+  // we need to define enough of the packet to make the function work+  // The function being tested will pass IpSb to Ip6SendIcmpError which is defined above+  UINT32  DeadCode = 0xDeadC0de;+  // Don't actually use this pointer, just pass it to the function, nothing will be done with it+  IP6_SERVICE  *IpSb = (IP6_SERVICE *)&DeadCode;++  EFI_IPv6_ADDRESS  SourceAddress      = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IPv6_ADDRESS  DestinationAddress = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IP6_HEADER    Ip6Header          = { 0 };++  Ip6Header.SourceAddress      = SourceAddress;+  Ip6Header.DestinationAddress = DestinationAddress;+  Packet.Ip.Ip6                = &Ip6Header;++  IP6_OPTION_HEADER  optionHeader;++  optionHeader.Type   = 23;   // Unknown Option+  optionHeader.Length = 2;    // Valid length for an unknown option++  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));+}++// Test Description+// Verify that Ip6OptionPad1 is valid with a length of 0+TEST_F (Ip6IsOptionValidTest, VerifyIp6OptionPad1) {+  NET_BUF  Packet = { 0 };+  // we need to define enough of the packet to make the function work+  // The function being tested will pass IpSb to Ip6SendIcmpError which is defined above+  UINT32  DeadCode = 0xDeadC0de;+  // Don't actually use this pointer, just pass it to the function, nothing will be done with it+  IP6_SERVICE  *IpSb = (IP6_SERVICE *)&DeadCode;++  EFI_IPv6_ADDRESS  SourceAddress      = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IPv6_ADDRESS  DestinationAddress = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IP6_HEADER    Ip6Header          = { 0 };++  Ip6Header.SourceAddress      = SourceAddress;+  Ip6Header.DestinationAddress = DestinationAddress;+  Packet.Ip.Ip6                = &Ip6Header;++  IP6_OPTION_HEADER  optionHeader;++  optionHeader.Type   = Ip6OptionPad1;+  optionHeader.Length = 0;++  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));+}++// Test Description+// Verify that Ip6OptionPadN doesn't overflow with various lengths+TEST_F (Ip6IsOptionValidTest, VerifyIp6OptionPadN) {+  NET_BUF  Packet = { 0 };+  // we need to define enough of the packet to make the function work+  // The function being tested will pass IpSb to Ip6SendIcmpError which is defined above+  UINT32  DeadCode = 0xDeadC0de;+  // Don't actually use this pointer, just pass it to the function, nothing will be done with it+  IP6_SERVICE  *IpSb = (IP6_SERVICE *)&DeadCode;++  EFI_IPv6_ADDRESS  SourceAddress      = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IPv6_ADDRESS  DestinationAddress = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IP6_HEADER    Ip6Header          = { 0 };++  Ip6Header.SourceAddress      = SourceAddress;+  Ip6Header.DestinationAddress = DestinationAddress;+  Packet.Ip.Ip6                = &Ip6Header;++  IP6_OPTION_HEADER  optionHeader;++  optionHeader.Type   = Ip6OptionPadN;+  optionHeader.Length = 0xFF;+  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));++  optionHeader.Length = 0xFE;+  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));++  optionHeader.Length = 0xFD;+  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));++  optionHeader.Length = 0xFC;+  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));+}++// Test Description+// Verify an unknown option doesn't cause an infinite loop with various lengths+TEST_F (Ip6IsOptionValidTest, VerifyNoInfiniteLoopOnUnknownOptionLengthAttemptOverflow) {+  NET_BUF  Packet = { 0 };+  // we need to define enough of the packet to make the function work+  // The function being tested will pass IpSb to Ip6SendIcmpError which is defined above+  UINT32  DeadCode = 0xDeadC0de;+  // Don't actually use this pointer, just pass it to the function, nothing will be done with it+  IP6_SERVICE  *IpSb = (IP6_SERVICE *)&DeadCode;++  EFI_IPv6_ADDRESS  SourceAddress      = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IPv6_ADDRESS  DestinationAddress = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IP6_HEADER    Ip6Header          = { 0 };++  Ip6Header.SourceAddress      = SourceAddress;+  Ip6Header.DestinationAddress = DestinationAddress;+  Packet.Ip.Ip6                = &Ip6Header;++  IP6_OPTION_HEADER  optionHeader;++  optionHeader.Type   = 23;   // Unknown Option+  optionHeader.Length = 0xFF;+  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));++  optionHeader.Length = 0xFE;+  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));++  optionHeader.Length = 0xFD;+  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));++  optionHeader.Length = 0xFC;+  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, sizeof (optionHeader), 0));+}++// Test Description+// Verify that the function supports multiple options+TEST_F (Ip6IsOptionValidTest, MultiOptionSupport) {+  UINT16   HdrLen;+  NET_BUF  Packet = { 0 };+  // we need to define enough of the packet to make the function work+  // The function being tested will pass IpSb to Ip6SendIcmpError which is defined above+  UINT32  DeadCode = 0xDeadC0de;+  // Don't actually use this pointer, just pass it to the function, nothing will be done with it+  IP6_SERVICE  *IpSb = (IP6_SERVICE *)&DeadCode;++  EFI_IPv6_ADDRESS  SourceAddress      = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IPv6_ADDRESS  DestinationAddress = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x00, 0x00, 0x42, 0x83, 0x29 };+  EFI_IP6_HEADER    Ip6Header          = { 0 };++  Ip6Header.SourceAddress      = SourceAddress;+  Ip6Header.DestinationAddress = DestinationAddress;+  Packet.Ip.Ip6                = &Ip6Header;++  UINT8              ExtHdr[1024] = { 0 };+  UINT8              *Cursor      = ExtHdr;+  IP6_OPTION_HEADER  *Option      = (IP6_OPTION_HEADER *)ExtHdr;++  // Let's start chaining options++  Option->Type   = 23;   // Unknown Option+  Option->Length = 0xFC;++  Cursor += sizeof (IP6_OPTION_HEADER) + 0xFC;++  Option       = (IP6_OPTION_HEADER *)Cursor;+  Option->Type = Ip6OptionPad1;++  Cursor += sizeof (1);++  // Type and length aren't processed, instead it just moves the pointer forward by 4 bytes+  Option         = (IP6_OPTION_HEADER *)Cursor;+  Option->Type   = Ip6OptionRouterAlert;+  Option->Length = 4;++  Cursor += sizeof (IP6_OPTION_HEADER) + 4;++  Option         = (IP6_OPTION_HEADER *)Cursor;+  Option->Type   = Ip6OptionPadN;+  Option->Length = 0xFC;++  Cursor += sizeof (IP6_OPTION_HEADER) + 0xFC;++  Option         = (IP6_OPTION_HEADER *)Cursor;+  Option->Type   = Ip6OptionRouterAlert;+  Option->Length = 4;++  Cursor += sizeof (IP6_OPTION_HEADER) + 4;++  // Total 524++  HdrLen = (UINT16)(Cursor - ExtHdr);++  EXPECT_TRUE (Ip6IsOptionValid (IpSb, &Packet, ExtHdr, HdrLen, 0));+}-- 
2.43.0



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