From nobody Sat Mar 14 23:06:06 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+114265+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+114265+1787277+3901457@groups.io ARC-Seal: i=1; a=rsa-sha256; t=1706073637; cv=none; d=zohomail.com; s=zohoarc; b=SXH4Z4Lqb4u5jvce77FN0Ks6IzBLiwS7fyVBqQOoJvhtRL61HuE43WZkgjfY67LQ/Ix7Ig+R23J8qROQHSzmMSfpED1qUDCne/DqVS1NCEOObFCwYXHa4jzE55IseaIrbgQ+Nx0GpFs9162iGl1+5kMpn0Dfd/bb/D4r/CaO1d0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1706073637; h=Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Reply-To:References:Sender:Subject:Subject:To:To:Message-Id; bh=BNtd5DQSNdOiTAD3O1dYfH1qZq8c9Hh+B1yu2aVSDQk=; b=N+6XZYeeSh4/4DwLg9cE90zI+fs+OgqlYIaljrn5sZvoZGG6uSllMvw6DiotytbI6+pKi6/TsgtNTTalG3YVEdfibRR3zvQNlvVmkqq4etmnAoPjiuM9XrMeilecJctxc+i3piqCAIFlUCsItALW09Gymfa5yB5Ud7jIDckAr/E= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+114265+1787277+3901457@groups.io Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1706073637973625.7332663062233; Tue, 23 Jan 2024 21:20:37 -0800 (PST) Return-Path: DKIM-Signature: a=rsa-sha256; bh=IM/SMbI1DpesVDzSzvRBUL9AH9B+bplNovmz7ZyxdnI=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1706073637; v=1; b=SQLzeh17oqGwzcJsCpELjRBy3skKqVTqLOYzmTuav5ervkm0qQ3fKccc6fBeL88fQaYDYp+v GuRWtQpQpFv3V8N5we+kGqTsEksveS8DzI+u18ckPSVH4WqgWy7UNXb8zzY+HyOEANE7sha8iq2 GIof1I2pI/PloY3MhKPfcFvk= X-Received: by 127.0.0.2 with SMTP id I6A0YY1788612xj9DTnf5YJJ; Tue, 23 Jan 2024 21:20:37 -0800 X-Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by mx.groups.io with SMTP id smtpd.web11.16113.1706073637049071586 for ; Tue, 23 Jan 2024 21:20:37 -0800 X-Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-1d51ba18e1bso45715585ad.0 for ; Tue, 23 Jan 2024 21:20:37 -0800 (PST) X-Gm-Message-State: HzKUN97N1oDO0kjcdaCkPqrMx1787277AA= X-Google-Smtp-Source: AGHT+IG/7lhZo3RdEoCjMZzImfnTQRfNwSk9i0BZ1JQe/tiRwNiwyN2xYgWxx8gocZYv4sjuPDg+ng== X-Received: by 2002:a17:902:e802:b0:1d3:eb37:57bf with SMTP id u2-20020a170902e80200b001d3eb3757bfmr478601plg.12.1706073636214; Tue, 23 Jan 2024 21:20:36 -0800 (PST) X-Received: from localhost.localdomain ([24.17.138.83]) by smtp.gmail.com with ESMTPSA id w2-20020a170902c78200b001d71f10aa42sm7831709pla.11.2024.01.23.21.20.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Jan 2024 21:20:35 -0800 (PST) From: "Doug Flick via groups.io" To: devel@edk2.groups.io Cc: Doug Flick , Saloni Kasbekar , Zachary Clark-williams , "Doug Flick [MSFT]" Subject: [edk2-devel] [PATCH 13/14] NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45235 Unit Tests Date: Tue, 23 Jan 2024 19:33:36 -0800 Message-ID: In-Reply-To: References: MIME-Version: 1.0 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,dougflick@microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1706073638346100044 Content-Type: text/plain; charset="utf-8" From: Doug Flick REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D4540 SECURITY PATCH - Unit Tests TCBZ4540 CVE-2023-45235 CVSS 8.3 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:L/A:H CWE-119 Improper Restriction of Operations within the Bounds of a Memory Buffer Cc: Saloni Kasbekar Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] --- NetworkPkg/Test/NetworkPkgHostTest.dsc | 5 +- .../GoogleTest/PxeBcDhcp6GoogleTest.h | 18 ++ .../GoogleTest/PxeBcDhcp6GoogleTest.cpp | 278 +++++++++++++++++- 3 files changed, 298 insertions(+), 3 deletions(-) diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/Netwo= rkPkgHostTest.dsc index a0273c431025..fa301a7a52ab 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -27,7 +27,10 @@ [Components] # NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf - NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf + NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf { + + UefiRuntimeServicesTableLib|MdePkg/Test/Mock/Library/GoogleTest/Mock= UefiRuntimeServicesTableLib/MockUefiRuntimeServicesTableLib.inf + } =20 # Despite these library classes being listed in [LibraryClasses] below, th= ey are not needed for the host-based unit tests. [LibraryClasses] diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h b/Ne= tworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h index b17c314791c8..0d825e44250a 100644 --- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h @@ -47,4 +47,22 @@ PxeBcCacheDnsServerAddresses ( IN PXEBC_DHCP6_PACKET_CACHE *Cache6 ); =20 +/** + Build and send out the request packet for the bootfile, and parse the re= ply. + + @param[in] Private The pointer to PxeBc private data. + @param[in] Index PxeBc option boot item type. + + @retval EFI_SUCCESS Successfully discovered the boot file. + @retval EFI_OUT_OF_RESOURCES Failed to allocate resources. + @retval EFI_NOT_FOUND Can't get the PXE reply packet. + @retval Others Failed to discover the boot file. + +**/ +EFI_STATUS +PxeBcRequestBootService ( + IN PXEBC_PRIVATE_DATA *Private, + IN UINT32 Index + ); + #endif // PXE_BC_DHCP6_GOOGLE_TEST_H_ diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp b/= NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp index 8260eeee50dc..bd423ebadfce 100644 --- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp @@ -4,7 +4,9 @@ Copyright (c) Microsoft Corporation SPDX-License-Identifier: BSD-2-Clause-Patent **/ -#include +#include +#include +#include =20 extern "C" { #include @@ -19,7 +21,8 @@ extern "C" { // Definitions //////////////////////////////////////////////////////////////////////////= ///// =20 -#define PACKET_SIZE (1500) +#define PACKET_SIZE (1500) +#define REQUEST_OPTION_LENGTH (120) =20 typedef struct { UINT16 OptionCode; // The option code for DHCP6_OPT_SERVER_ID (e.g.= , 0x03) @@ -76,6 +79,26 @@ MockConfigure ( } =20 // Needed by PxeBcSupport +EFI_STATUS +PxeBcDns6 ( + IN PXEBC_PRIVATE_DATA *Private, + IN CHAR16 *HostName, + OUT EFI_IPv6_ADDRESS *IpAddress + ) +{ + return EFI_SUCCESS; +} + +UINT32 +PxeBcBuildDhcp6Options ( + IN PXEBC_PRIVATE_DATA *Private, + OUT EFI_DHCP6_PACKET_OPTION **OptList, + IN UINT8 *Buffer + ) +{ + return EFI_SUCCESS; +} + EFI_STATUS EFIAPI QueueDpc ( @@ -159,6 +182,10 @@ TEST_F (PxeBcHandleDhcp6OfferTest, BasicUsageTest) { ASSERT_EQ (PxeBcHandleDhcp6Offer (&(PxeBcHandleDhcp6OfferTest::Private))= , EFI_DEVICE_ERROR); } =20 +//////////////////////////////////////////////////////////////////////////= ///// +// PxeBcCacheDnsServerAddresses Tests +//////////////////////////////////////////////////////////////////////////= ///// + class PxeBcCacheDnsServerAddressesTest : public ::testing::Test { public: PXEBC_PRIVATE_DATA Private =3D { 0 }; @@ -298,3 +325,250 @@ TEST_F (PxeBcCacheDnsServerAddressesTest, MultipleDns= Entries) { FreePool (Private.DnsServer); } } + +//////////////////////////////////////////////////////////////////////////= ///// +// PxeBcRequestBootServiceTest Test Cases +//////////////////////////////////////////////////////////////////////////= ///// + +class PxeBcRequestBootServiceTest : public ::testing::Test { +public: + PXEBC_PRIVATE_DATA Private =3D { 0 }; + EFI_UDP6_PROTOCOL Udp6Read; + +protected: + // Add any setup code if needed + virtual void + SetUp ( + ) + { + Private.Dhcp6Request =3D (EFI_DHCP6_PACKET *)AllocateZeroPool (PACKET_= SIZE); + + // Need to setup the EFI_PXE_BASE_CODE_PROTOCOL + // The function under test really only needs the following: + // UdpWrite + // UdpRead + + Private.PxeBc.UdpWrite =3D (EFI_PXE_BASE_CODE_UDP_WRITE)MockUdpWrite; + Private.PxeBc.UdpRead =3D (EFI_PXE_BASE_CODE_UDP_READ)MockUdpRead; + + // Need to setup EFI_UDP6_PROTOCOL + // The function under test really only needs the following: + // Configure + + Udp6Read.Configure =3D (EFI_UDP6_CONFIGURE)MockConfigure; + Private.Udp6Read =3D &Udp6Read; + } + + // Add any cleanup code if needed + virtual void + TearDown ( + ) + { + if (Private.Dhcp6Request !=3D NULL) { + FreePool (Private.Dhcp6Request); + } + + // Clean up any resources or variables + } +}; + +TEST_F (PxeBcRequestBootServiceTest, ServerDiscoverBasicUsageTest) { + PxeBcRequestBootServiceTest::Private.OfferBuffer[0].Dhcp6.OfferType =3D = PxeOfferTypeProxyBinl; + + DHCP6_OPTION_SERVER_ID Server =3D { 0 }; + + Server.OptionCode =3D HTONS (DHCP6_OPT_SERVER_ID); + Server.OptionLen =3D HTONS (16); // valid length + UINT8 Index =3D 0; + + EFI_DHCP6_PACKET *Packet =3D (EFI_DHCP6_PACKET *)&Private.OfferBuffer[I= ndex].Dhcp6.Packet.Offer; + + UINT8 *Cursor =3D (UINT8 *)(Packet->Dhcp6.Option); + + CopyMem (Cursor, &Server, sizeof (Server)); + Cursor +=3D sizeof (Server); + + // Update the packet length + Packet->Length =3D (UINT16)(Cursor - (UINT8 *)Packet); + Packet->Size =3D PACKET_SIZE; + + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Priva= te), Index), EFI_SUCCESS); +} + +TEST_F (PxeBcRequestBootServiceTest, AttemptDiscoverOverFlowExpectFailure)= { + PxeBcRequestBootServiceTest::Private.OfferBuffer[0].Dhcp6.OfferType =3D = PxeOfferTypeProxyBinl; + + DHCP6_OPTION_SERVER_ID Server =3D { 0 }; + + Server.OptionCode =3D HTONS (DHCP6_OPT_SERVER_ID); + Server.OptionLen =3D HTONS (1500); // This length would overflow withou= t a check + UINT8 Index =3D 0; + + EFI_DHCP6_PACKET *Packet =3D (EFI_DHCP6_PACKET *)&Private.OfferBuffer[I= ndex].Dhcp6.Packet.Offer; + + UINT8 *Cursor =3D (UINT8 *)(Packet->Dhcp6.Option); + + CopyMem (Cursor, &Server, sizeof (Server)); + Cursor +=3D sizeof (Server); + + // Update the packet length + Packet->Length =3D (UINT16)(Cursor - (UINT8 *)Packet); + Packet->Size =3D PACKET_SIZE; + + // This is going to be stopped by the duid overflow check + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Priva= te), Index), EFI_INVALID_PARAMETER); +} + +TEST_F (PxeBcRequestBootServiceTest, RequestBasicUsageTest) { + EFI_DHCP6_PACKET_OPTION RequestOpt =3D { 0 }; // the data section doesn= 't really matter + + RequestOpt.OpCode =3D HTONS (0x1337); + RequestOpt.OpLen =3D 0; // valid length + + UINT8 Index =3D 0; + + EFI_DHCP6_PACKET *Packet =3D (EFI_DHCP6_PACKET *)&Private.Dhcp6Request[= Index]; + + UINT8 *Cursor =3D (UINT8 *)(Packet->Dhcp6.Option); + + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt)); + Cursor +=3D sizeof (RequestOpt); + + // Update the packet length + Packet->Length =3D (UINT16)(Cursor - (UINT8 *)Packet); + Packet->Size =3D PACKET_SIZE; + + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Priva= te), Index), EFI_SUCCESS); +} + +TEST_F (PxeBcRequestBootServiceTest, AttemptRequestOverFlowExpectFailure) { + EFI_DHCP6_PACKET_OPTION RequestOpt =3D { 0 }; // the data section doesn= 't really matter + + RequestOpt.OpCode =3D HTONS (0x1337); + RequestOpt.OpLen =3D 1500; // this length would overflow without a check + + UINT8 Index =3D 0; + + EFI_DHCP6_PACKET *Packet =3D (EFI_DHCP6_PACKET *)&Private.Dhcp6Request[= Index]; + + UINT8 *Cursor =3D (UINT8 *)(Packet->Dhcp6.Option); + + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt)); + Cursor +=3D sizeof (RequestOpt); + + // Update the packet length + Packet->Length =3D (UINT16)(Cursor - (UINT8 *)Packet); + Packet->Size =3D PACKET_SIZE; + + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Priva= te), Index), EFI_OUT_OF_RESOURCES); +} + +//////////////////////////////////////////////////////////////////////////= ///// +// PxeBcDhcp6Discover Test +//////////////////////////////////////////////////////////////////////////= ///// + +class PxeBcDhcp6DiscoverTest : public ::testing::Test { +public: + PXEBC_PRIVATE_DATA Private =3D { 0 }; + EFI_UDP6_PROTOCOL Udp6Read; + +protected: + MockUefiRuntimeServicesTableLib RtServicesMock; + + // Add any setup code if needed + virtual void + SetUp ( + ) + { + Private.Dhcp6Request =3D (EFI_DHCP6_PACKET *)AllocateZeroPool (PACKET_= SIZE); + + // Need to setup the EFI_PXE_BASE_CODE_PROTOCOL + // The function under test really only needs the following: + // UdpWrite + // UdpRead + + Private.PxeBc.UdpWrite =3D (EFI_PXE_BASE_CODE_UDP_WRITE)MockUdpWrite; + Private.PxeBc.UdpRead =3D (EFI_PXE_BASE_CODE_UDP_READ)MockUdpRead; + + // Need to setup EFI_UDP6_PROTOCOL + // The function under test really only needs the following: + // Configure + + Udp6Read.Configure =3D (EFI_UDP6_CONFIGURE)MockConfigure; + Private.Udp6Read =3D &Udp6Read; + } + + // Add any cleanup code if needed + virtual void + TearDown ( + ) + { + if (Private.Dhcp6Request !=3D NULL) { + FreePool (Private.Dhcp6Request); + } + + // Clean up any resources or variables + } +}; + +// Test Description +// This will cause an overflow by an untrusted packet during the option pa= rsing +TEST_F (PxeBcDhcp6DiscoverTest, BasicOverflowTest) { + EFI_IPv6_ADDRESS DestIp =3D { 0 }; + EFI_DHCP6_PACKET_OPTION RequestOpt =3D { 0 }; // the data section doesn= 't really matter + + RequestOpt.OpCode =3D HTONS (0x1337); + RequestOpt.OpLen =3D HTONS (0xFFFF); // overflow + + UINT8 *Cursor =3D (UINT8 *)(Private.Dhcp6Request->Dhcp6.Option); + + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt)); + Cursor +=3D sizeof (RequestOpt); + + Private.Dhcp6Request->Length =3D (UINT16)(Cursor - (UINT8 *)Private.Dhcp= 6Request); + + EXPECT_CALL (RtServicesMock, gRT_GetTime) + .WillOnce (::testing::Return (0)); + + ASSERT_EQ ( + PxeBcDhcp6Discover ( + &(PxeBcDhcp6DiscoverTest::Private), + 0, + NULL, + FALSE, + (EFI_IP_ADDRESS *)&DestIp + ), + EFI_OUT_OF_RESOURCES + ); +} + +// Test Description +// This will test that we can handle a packet with a valid option length +TEST_F (PxeBcDhcp6DiscoverTest, BasicUsageTest) { + EFI_IPv6_ADDRESS DestIp =3D { 0 }; + EFI_DHCP6_PACKET_OPTION RequestOpt =3D { 0 }; // the data section doesn= 't really matter + + RequestOpt.OpCode =3D HTONS (0x1337); + RequestOpt.OpLen =3D HTONS (0x30); + + UINT8 *Cursor =3D (UINT8 *)(Private.Dhcp6Request->Dhcp6.Option); + + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt)); + Cursor +=3D sizeof (RequestOpt); + + Private.Dhcp6Request->Length =3D (UINT16)(Cursor - (UINT8 *)Private.Dhcp= 6Request); + + EXPECT_CALL (RtServicesMock, gRT_GetTime) + .WillOnce (::testing::Return (0)); + + ASSERT_EQ ( + PxeBcDhcp6Discover ( + &(PxeBcDhcp6DiscoverTest::Private), + 0, + NULL, + FALSE, + (EFI_IP_ADDRESS *)&DestIp + ), + EFI_SUCCESS + ); +} --=20 2.43.0 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114265): https://edk2.groups.io/g/devel/message/114265 Mute This Topic: https://groups.io/mt/103926744/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-