From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 6443D780091 for ; Wed, 24 Jan 2024 05:20:42 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=uOmP7ajn6PZ0OkxkFVbJf18utY1+iwFV9ihuiaeZUVc=; 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=1706073641; v=1; b=XodxkSxPXL9YJeUQuSCkkIeIAskBiiS2lLRrlHkb9PJYjj0YRme7wTYzsOK37G5JwIWl0qhW KxKTBlT0w3NoHOMTObDrGyDB0SHImJyraZNtOmCcKL8dNftbiuQoJasvFSD8eZ54Z6GBPJhQ3tQ sDfje2efpnyPqxOWN/2VVjOM= X-Received: by 127.0.0.2 with SMTP id k1jIYY7687511x1t9AtyOcou; Tue, 23 Jan 2024 21:20:41 -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: eCv1QXMJDWVvqYHDguxHFiRFx7686176AA= 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-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=XodxkSxP; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io 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] #=0D NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf=0D NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf=0D - NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf=0D + NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf {=0D + =0D + UefiRuntimeServicesTableLib|MdePkg/Test/Mock/Library/GoogleTest/Mock= UefiRuntimeServicesTableLib/MockUefiRuntimeServicesTableLib.inf=0D + }=0D =0D # Despite these library classes being listed in [LibraryClasses] below, th= ey are not needed for the host-based unit tests.=0D [LibraryClasses]=0D 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=0D );=0D =0D +/**=0D + Build and send out the request packet for the bootfile, and parse the re= ply.=0D +=0D + @param[in] Private The pointer to PxeBc private data.=0D + @param[in] Index PxeBc option boot item type.=0D +=0D + @retval EFI_SUCCESS Successfully discovered the boot file.= =0D + @retval EFI_OUT_OF_RESOURCES Failed to allocate resources.=0D + @retval EFI_NOT_FOUND Can't get the PXE reply packet.=0D + @retval Others Failed to discover the boot file.=0D +=0D +**/=0D +EFI_STATUS=0D +PxeBcRequestBootService (=0D + IN PXEBC_PRIVATE_DATA *Private,=0D + IN UINT32 Index=0D + );=0D +=0D #endif // PXE_BC_DHCP6_GOOGLE_TEST_H_=0D 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=0D SPDX-License-Identifier: BSD-2-Clause-Patent=0D **/=0D -#include =0D +#include =0D +#include =0D +#include =0D =0D extern "C" {=0D #include =0D @@ -19,7 +21,8 @@ extern "C" { // Definitions=0D //////////////////////////////////////////////////////////////////////////= /////=0D =0D -#define PACKET_SIZE (1500)=0D +#define PACKET_SIZE (1500)=0D +#define REQUEST_OPTION_LENGTH (120)=0D =0D typedef struct {=0D UINT16 OptionCode; // The option code for DHCP6_OPT_SERVER_ID (e.g.= , 0x03)=0D @@ -76,6 +79,26 @@ MockConfigure ( }=0D =0D // Needed by PxeBcSupport=0D +EFI_STATUS=0D +PxeBcDns6 (=0D + IN PXEBC_PRIVATE_DATA *Private,=0D + IN CHAR16 *HostName,=0D + OUT EFI_IPv6_ADDRESS *IpAddress=0D + )=0D +{=0D + return EFI_SUCCESS;=0D +}=0D +=0D +UINT32=0D +PxeBcBuildDhcp6Options (=0D + IN PXEBC_PRIVATE_DATA *Private,=0D + OUT EFI_DHCP6_PACKET_OPTION **OptList,=0D + IN UINT8 *Buffer=0D + )=0D +{=0D + return EFI_SUCCESS;=0D +}=0D +=0D EFI_STATUS=0D EFIAPI=0D QueueDpc (=0D @@ -159,6 +182,10 @@ TEST_F (PxeBcHandleDhcp6OfferTest, BasicUsageTest) { ASSERT_EQ (PxeBcHandleDhcp6Offer (&(PxeBcHandleDhcp6OfferTest::Private))= , EFI_DEVICE_ERROR);=0D }=0D =0D +//////////////////////////////////////////////////////////////////////////= /////=0D +// PxeBcCacheDnsServerAddresses Tests=0D +//////////////////////////////////////////////////////////////////////////= /////=0D +=0D class PxeBcCacheDnsServerAddressesTest : public ::testing::Test {=0D public:=0D PXEBC_PRIVATE_DATA Private =3D { 0 };=0D @@ -298,3 +325,250 @@ TEST_F (PxeBcCacheDnsServerAddressesTest, MultipleDns= Entries) { FreePool (Private.DnsServer);=0D }=0D }=0D +=0D +//////////////////////////////////////////////////////////////////////////= /////=0D +// PxeBcRequestBootServiceTest Test Cases=0D +//////////////////////////////////////////////////////////////////////////= /////=0D +=0D +class PxeBcRequestBootServiceTest : public ::testing::Test {=0D +public:=0D + PXEBC_PRIVATE_DATA Private =3D { 0 };=0D + EFI_UDP6_PROTOCOL Udp6Read;=0D +=0D +protected:=0D + // Add any setup code if needed=0D + virtual void=0D + SetUp (=0D + )=0D + {=0D + Private.Dhcp6Request =3D (EFI_DHCP6_PACKET *)AllocateZeroPool (PACKET_= SIZE);=0D +=0D + // Need to setup the EFI_PXE_BASE_CODE_PROTOCOL=0D + // The function under test really only needs the following:=0D + // UdpWrite=0D + // UdpRead=0D +=0D + Private.PxeBc.UdpWrite =3D (EFI_PXE_BASE_CODE_UDP_WRITE)MockUdpWrite;= =0D + Private.PxeBc.UdpRead =3D (EFI_PXE_BASE_CODE_UDP_READ)MockUdpRead;=0D +=0D + // Need to setup EFI_UDP6_PROTOCOL=0D + // The function under test really only needs the following:=0D + // Configure=0D +=0D + Udp6Read.Configure =3D (EFI_UDP6_CONFIGURE)MockConfigure;=0D + Private.Udp6Read =3D &Udp6Read;=0D + }=0D +=0D + // Add any cleanup code if needed=0D + virtual void=0D + TearDown (=0D + )=0D + {=0D + if (Private.Dhcp6Request !=3D NULL) {=0D + FreePool (Private.Dhcp6Request);=0D + }=0D +=0D + // Clean up any resources or variables=0D + }=0D +};=0D +=0D +TEST_F (PxeBcRequestBootServiceTest, ServerDiscoverBasicUsageTest) {=0D + PxeBcRequestBootServiceTest::Private.OfferBuffer[0].Dhcp6.OfferType =3D = PxeOfferTypeProxyBinl;=0D +=0D + DHCP6_OPTION_SERVER_ID Server =3D { 0 };=0D +=0D + Server.OptionCode =3D HTONS (DHCP6_OPT_SERVER_ID);=0D + Server.OptionLen =3D HTONS (16); // valid length=0D + UINT8 Index =3D 0;=0D +=0D + EFI_DHCP6_PACKET *Packet =3D (EFI_DHCP6_PACKET *)&Private.OfferBuffer[I= ndex].Dhcp6.Packet.Offer;=0D +=0D + UINT8 *Cursor =3D (UINT8 *)(Packet->Dhcp6.Option);=0D +=0D + CopyMem (Cursor, &Server, sizeof (Server));=0D + Cursor +=3D sizeof (Server);=0D +=0D + // Update the packet length=0D + Packet->Length =3D (UINT16)(Cursor - (UINT8 *)Packet);=0D + Packet->Size =3D PACKET_SIZE;=0D +=0D + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Priva= te), Index), EFI_SUCCESS);=0D +}=0D +=0D +TEST_F (PxeBcRequestBootServiceTest, AttemptDiscoverOverFlowExpectFailure)= {=0D + PxeBcRequestBootServiceTest::Private.OfferBuffer[0].Dhcp6.OfferType =3D = PxeOfferTypeProxyBinl;=0D +=0D + DHCP6_OPTION_SERVER_ID Server =3D { 0 };=0D +=0D + Server.OptionCode =3D HTONS (DHCP6_OPT_SERVER_ID);=0D + Server.OptionLen =3D HTONS (1500); // This length would overflow withou= t a check=0D + UINT8 Index =3D 0;=0D +=0D + EFI_DHCP6_PACKET *Packet =3D (EFI_DHCP6_PACKET *)&Private.OfferBuffer[I= ndex].Dhcp6.Packet.Offer;=0D +=0D + UINT8 *Cursor =3D (UINT8 *)(Packet->Dhcp6.Option);=0D +=0D + CopyMem (Cursor, &Server, sizeof (Server));=0D + Cursor +=3D sizeof (Server);=0D +=0D + // Update the packet length=0D + Packet->Length =3D (UINT16)(Cursor - (UINT8 *)Packet);=0D + Packet->Size =3D PACKET_SIZE;=0D +=0D + // This is going to be stopped by the duid overflow check=0D + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Priva= te), Index), EFI_INVALID_PARAMETER);=0D +}=0D +=0D +TEST_F (PxeBcRequestBootServiceTest, RequestBasicUsageTest) {=0D + EFI_DHCP6_PACKET_OPTION RequestOpt =3D { 0 }; // the data section doesn= 't really matter=0D +=0D + RequestOpt.OpCode =3D HTONS (0x1337);=0D + RequestOpt.OpLen =3D 0; // valid length=0D +=0D + UINT8 Index =3D 0;=0D +=0D + EFI_DHCP6_PACKET *Packet =3D (EFI_DHCP6_PACKET *)&Private.Dhcp6Request[= Index];=0D +=0D + UINT8 *Cursor =3D (UINT8 *)(Packet->Dhcp6.Option);=0D +=0D + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt));=0D + Cursor +=3D sizeof (RequestOpt);=0D +=0D + // Update the packet length=0D + Packet->Length =3D (UINT16)(Cursor - (UINT8 *)Packet);=0D + Packet->Size =3D PACKET_SIZE;=0D +=0D + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Priva= te), Index), EFI_SUCCESS);=0D +}=0D +=0D +TEST_F (PxeBcRequestBootServiceTest, AttemptRequestOverFlowExpectFailure) = {=0D + EFI_DHCP6_PACKET_OPTION RequestOpt =3D { 0 }; // the data section doesn= 't really matter=0D +=0D + RequestOpt.OpCode =3D HTONS (0x1337);=0D + RequestOpt.OpLen =3D 1500; // this length would overflow without a chec= k=0D +=0D + UINT8 Index =3D 0;=0D +=0D + EFI_DHCP6_PACKET *Packet =3D (EFI_DHCP6_PACKET *)&Private.Dhcp6Request[= Index];=0D +=0D + UINT8 *Cursor =3D (UINT8 *)(Packet->Dhcp6.Option);=0D +=0D + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt));=0D + Cursor +=3D sizeof (RequestOpt);=0D +=0D + // Update the packet length=0D + Packet->Length =3D (UINT16)(Cursor - (UINT8 *)Packet);=0D + Packet->Size =3D PACKET_SIZE;=0D +=0D + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Priva= te), Index), EFI_OUT_OF_RESOURCES);=0D +}=0D +=0D +//////////////////////////////////////////////////////////////////////////= /////=0D +// PxeBcDhcp6Discover Test=0D +//////////////////////////////////////////////////////////////////////////= /////=0D +=0D +class PxeBcDhcp6DiscoverTest : public ::testing::Test {=0D +public:=0D + PXEBC_PRIVATE_DATA Private =3D { 0 };=0D + EFI_UDP6_PROTOCOL Udp6Read;=0D +=0D +protected:=0D + MockUefiRuntimeServicesTableLib RtServicesMock;=0D +=0D + // Add any setup code if needed=0D + virtual void=0D + SetUp (=0D + )=0D + {=0D + Private.Dhcp6Request =3D (EFI_DHCP6_PACKET *)AllocateZeroPool (PACKET_= SIZE);=0D +=0D + // Need to setup the EFI_PXE_BASE_CODE_PROTOCOL=0D + // The function under test really only needs the following:=0D + // UdpWrite=0D + // UdpRead=0D +=0D + Private.PxeBc.UdpWrite =3D (EFI_PXE_BASE_CODE_UDP_WRITE)MockUdpWrite;= =0D + Private.PxeBc.UdpRead =3D (EFI_PXE_BASE_CODE_UDP_READ)MockUdpRead;=0D +=0D + // Need to setup EFI_UDP6_PROTOCOL=0D + // The function under test really only needs the following:=0D + // Configure=0D +=0D + Udp6Read.Configure =3D (EFI_UDP6_CONFIGURE)MockConfigure;=0D + Private.Udp6Read =3D &Udp6Read;=0D + }=0D +=0D + // Add any cleanup code if needed=0D + virtual void=0D + TearDown (=0D + )=0D + {=0D + if (Private.Dhcp6Request !=3D NULL) {=0D + FreePool (Private.Dhcp6Request);=0D + }=0D +=0D + // Clean up any resources or variables=0D + }=0D +};=0D +=0D +// Test Description=0D +// This will cause an overflow by an untrusted packet during the option pa= rsing=0D +TEST_F (PxeBcDhcp6DiscoverTest, BasicOverflowTest) {=0D + EFI_IPv6_ADDRESS DestIp =3D { 0 };=0D + EFI_DHCP6_PACKET_OPTION RequestOpt =3D { 0 }; // the data section doesn= 't really matter=0D +=0D + RequestOpt.OpCode =3D HTONS (0x1337);=0D + RequestOpt.OpLen =3D HTONS (0xFFFF); // overflow=0D +=0D + UINT8 *Cursor =3D (UINT8 *)(Private.Dhcp6Request->Dhcp6.Option);=0D +=0D + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt));=0D + Cursor +=3D sizeof (RequestOpt);=0D +=0D + Private.Dhcp6Request->Length =3D (UINT16)(Cursor - (UINT8 *)Private.Dhcp= 6Request);=0D +=0D + EXPECT_CALL (RtServicesMock, gRT_GetTime)=0D + .WillOnce (::testing::Return (0));=0D +=0D + ASSERT_EQ (=0D + PxeBcDhcp6Discover (=0D + &(PxeBcDhcp6DiscoverTest::Private),=0D + 0,=0D + NULL,=0D + FALSE,=0D + (EFI_IP_ADDRESS *)&DestIp=0D + ),=0D + EFI_OUT_OF_RESOURCES=0D + );=0D +}=0D +=0D +// Test Description=0D +// This will test that we can handle a packet with a valid option length=0D +TEST_F (PxeBcDhcp6DiscoverTest, BasicUsageTest) {=0D + EFI_IPv6_ADDRESS DestIp =3D { 0 };=0D + EFI_DHCP6_PACKET_OPTION RequestOpt =3D { 0 }; // the data section doesn= 't really matter=0D +=0D + RequestOpt.OpCode =3D HTONS (0x1337);=0D + RequestOpt.OpLen =3D HTONS (0x30);=0D +=0D + UINT8 *Cursor =3D (UINT8 *)(Private.Dhcp6Request->Dhcp6.Option);=0D +=0D + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt));=0D + Cursor +=3D sizeof (RequestOpt);=0D +=0D + Private.Dhcp6Request->Length =3D (UINT16)(Cursor - (UINT8 *)Private.Dhcp= 6Request);=0D +=0D + EXPECT_CALL (RtServicesMock, gRT_GetTime)=0D + .WillOnce (::testing::Return (0));=0D +=0D + ASSERT_EQ (=0D + PxeBcDhcp6Discover (=0D + &(PxeBcDhcp6DiscoverTest::Private),=0D + 0,=0D + NULL,=0D + FALSE,=0D + (EFI_IP_ADDRESS *)&DestIp=0D + ),=0D + EFI_SUCCESS=0D + );=0D +}=0D --=20 2.43.0 -=-=-=-=-=-=-=-=-=-=-=- 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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-