From nobody Fri Oct 18 05:18:22 2024 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+114476+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+114476+1787277+3901457@groups.io ARC-Seal: i=1; a=rsa-sha256; t=1706224000; cv=none; d=zohomail.com; s=zohoarc; b=EQ1/EHolYsnbNIbgmajMGWcpGgfJ+3pSWdc/uGF7jwXxp8dQICbLzDcnLWcFquSiOCik3NiZc20xCqBT8KNPaeaoy5zvUyDyzQWS88NUoTtaTSQIBLXTC3xdFYkF6bqWTIF5i2ZxyM+uF323KGO9RIl+Yr3Qs8RzufiPkMdW12s= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1706224000; 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=XK3+fIcCTfRRlyQb4e9xHZLbd3OYpXU57C+zhwdvnvY=; b=l4vOZ0GRRt3rKfUGj/EXRp/omxJz0BCT3M45T2HvEKMH5Oh9pA9tBOXCWgXY3sEAm9IwhIKdaPRQd/8KYsC1qmD7mHE1rTs1BOnlaZYKm8PiWanMMJc3M+yLtRpISeXxRebMnAvd0fIx87ukHwpNAJOqvKVznuOmdYZRBGpSYts= 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+114476+1787277+3901457@groups.io Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1706224000489630.8724610995453; Thu, 25 Jan 2024 15:06:40 -0800 (PST) Return-Path: DKIM-Signature: a=rsa-sha256; bh=bRvUW/yThb/8UKWrVA3cAnbfprCxQUcI3X7UroMdvII=; 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=1706224000; v=1; b=bR5PV7ahHuVCXxB4cqi3ESk833hxSfQQliIGoxVpch7P13/rZhi2hYm1my4xKIYZiq0LTsAb NQ9vWmJNJyvILWAIl8MF8P43ZFl3YY7LNL5C7WQe1Zhhk5RLQygat87uq/skyUw/YBRyuoCpeiG RGIEMzKkN3C1JtzVXmO8Z/UU= X-Received: by 127.0.0.2 with SMTP id r6AKYY1788612xkj6NmscqL2; Thu, 25 Jan 2024 15:06:40 -0800 X-Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) by mx.groups.io with SMTP id smtpd.web11.793.1706223999630067004 for ; Thu, 25 Jan 2024 15:06:39 -0800 X-Received: by mail-pl1-f170.google.com with SMTP id d9443c01a7336-1d746ce7d13so42513425ad.0 for ; Thu, 25 Jan 2024 15:06:39 -0800 (PST) X-Gm-Message-State: A9FJy7JlnKFLZ70wW88loYI4x1787277AA= X-Google-Smtp-Source: AGHT+IHutbe8UoR/pcXXVliQg7WwATFGArWLpz18RsqpxZ/GXTGOABX0uhHP1O4DRM/b0VIQgV+s0g== X-Received: by 2002:a17:902:e80d:b0:1d7:6e2d:db62 with SMTP id u13-20020a170902e80d00b001d76e2ddb62mr504566plg.47.1706223998782; Thu, 25 Jan 2024 15:06:38 -0800 (PST) X-Received: from localhost.localdomain ([24.17.138.83]) by smtp.gmail.com with ESMTPSA id jh1-20020a170903328100b001d752c4f180sm16779plb.94.2024.01.25.15.06.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Jan 2024 15:06:38 -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 v2 14/15] NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45235 Unit Tests Date: Thu, 25 Jan 2024 13:54:56 -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: 1706224001896100059 Content-Type: text/plain; charset="utf-8" From: Doug Flick REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D4540 Unit tests to confirm that the bug.. Buffer overflow when handling Server ID option from a DHCPv6 proxy Advertise message ..has been patched. This patch contains unit tests for the following functions: PxeBcRequestBootService PxeBcDhcp6Discover 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 (#114476): https://edk2.groups.io/g/devel/message/114476 Mute This Topic: https://groups.io/mt/103964992/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-