From 0cf3620092c1f5d2093e5bfa43ff137b888f18b3 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Fri, 15 Dec 2023 11:24:43 -0800 Subject: [PATCH 01/12] SECURITY PATCH TCBZ4535 - CVE-2023-45230 - Patch --- NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h | 39 +++ NetworkPkg/Dhcp6Dxe/Dhcp6Io.c | 411 +++++++++++++++++--------- NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c | 455 +++++++++++++++++++++-------- NetworkPkg/Dhcp6Dxe/Dhcp6Utility.h | 82 +++--- 4 files changed, 707 insertions(+), 280 deletions(-) diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h b/NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h index 0eb9c669b5..b552331767 100644 --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h @@ -45,6 +45,45 @@ typedef struct _DHCP6_INSTANCE DHCP6_INSTANCE; #define DHCP6_SERVICE_SIGNATURE SIGNATURE_32 ('D', 'H', '6', 'S') #define DHCP6_INSTANCE_SIGNATURE SIGNATURE_32 ('D', 'H', '6', 'I') +// +// For more information on DHCP options see RFC 8415, Section 21.1 +// +// The format of DHCP options is: +// +// 0 1 2 3 +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | option-code | option-len | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | option-data | +// | (option-len octets) | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// +#define DHCP6_SIZE_OF_OPT_CODE (sizeof(UINT16)) +#define DHCP6_SIZE_OF_OPT_LEN (sizeof(UINT16)) + +// Combined size of Code and Length +#define DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN (DHCP6_SIZE_OF_OPT_CODE + \ + DHCP6_SIZE_OF_OPT_LEN) + +STATIC_ASSERT ( + DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN == 4, + "Combined size of Code and Length must be 4 per RFC 8415" + ); + +// Offset to the length is just past the code +#define DHCP6_OPT_LEN_OFFSET(a) (a + DHCP6_SIZE_OF_OPT_CODE) +STATIC_ASSERT ( + DHCP6_OPT_LEN_OFFSET (0) == 2, + "Offset of length is + 2 past start of option" + ); + +#define DHCP6_OPT_DATA_OFFSET(a) (a + DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN) +STATIC_ASSERT ( + DHCP6_OPT_DATA_OFFSET(0) == 4, + "Offset to option data should be +4 from start of option" + ); + #define DHCP6_PACKET_ALL 0 #define DHCP6_PACKET_STATEFUL 1 #define DHCP6_PACKET_STATELESS 2 diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c index dcd01e6268..1401910950 100644 --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c @@ -3,9 +3,9 @@ (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) Microsoft Corporation SPDX-License-Identifier: BSD-2-Clause-Patent - **/ #include "Dhcp6Impl.h" @@ -930,7 +930,8 @@ Dhcp6SendSolicitMsg ( // Packet = AllocateZeroPool (DHCP6_BASE_PACKET_SIZE + UserLen); if (Packet == NULL) { - return EFI_OUT_OF_RESOURCES; + Status = EFI_OUT_OF_RESOURCES; + goto ON_ERROR; } Packet->Size = DHCP6_BASE_PACKET_SIZE + UserLen; @@ -944,54 +945,64 @@ Dhcp6SendSolicitMsg ( Cursor = Packet->Dhcp6.Option; Length = HTONS (ClientId->Length); - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptClientId), Length, ClientId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendETOption ( - Cursor, + Status = Dhcp6AppendETOption ( + Packet, + &Cursor, Instance, &Elapsed ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendIaOption ( - Cursor, + Status = Dhcp6AppendIaOption ( + Packet, + &Cursor, Instance->IaCb.Ia, Instance->IaCb.T1, Instance->IaCb.T2, Packet->Dhcp6.Header.MessageType ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } // // Append user-defined when configurate Dhcp6 service. // for (Index = 0; Index < Instance->Config->OptionCount; Index++) { UserOpt = Instance->Config->OptionList[Index]; - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, UserOpt->OpCode, UserOpt->OpLen, UserOpt->Data ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } } - // - // Determine the size/length of packet. - // - Packet->Length += (UINT32)(Cursor - Packet->Dhcp6.Option); ASSERT (Packet->Size > Packet->Length + 8); // // Callback to user with the packet to be sent and check the user's feedback. // Status = Dhcp6CallbackUser (Instance, Dhcp6SendSolicit, &Packet); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // @@ -1005,10 +1016,8 @@ Dhcp6SendSolicitMsg ( Instance->StartTime = 0; Status = Dhcp6TransmitPacket (Instance, Packet, Elapsed); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // @@ -1020,6 +1029,14 @@ Dhcp6SendSolicitMsg ( Elapsed, Instance->Config->SolicitRetransmission ); + +ON_ERROR: + + if (Packet) { + FreePool (Packet); + } + + return Status; } /** @@ -1110,7 +1127,8 @@ Dhcp6SendRequestMsg ( // Packet = AllocateZeroPool (DHCP6_BASE_PACKET_SIZE + UserLen); if (Packet == NULL) { - return EFI_OUT_OF_RESOURCES; + Status = EFI_OUT_OF_RESOURCES; + goto ON_ERROR; } Packet->Size = DHCP6_BASE_PACKET_SIZE + UserLen; @@ -1124,51 +1142,67 @@ Dhcp6SendRequestMsg ( Cursor = Packet->Dhcp6.Option; Length = HTONS (ClientId->Length); - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptClientId), Length, ClientId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendETOption ( - Cursor, + Status = Dhcp6AppendETOption ( + Packet, + &Cursor, Instance, &Elapsed ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptServerId), ServerId->Length, ServerId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendIaOption ( - Cursor, + Status = Dhcp6AppendIaOption ( + Packet, + &Cursor, Instance->IaCb.Ia, Instance->IaCb.T1, Instance->IaCb.T2, Packet->Dhcp6.Header.MessageType ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } // // Append user-defined when configurate Dhcp6 service. // for (Index = 0; Index < Instance->Config->OptionCount; Index++) { UserOpt = Instance->Config->OptionList[Index]; - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, UserOpt->OpCode, UserOpt->OpLen, UserOpt->Data ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } } - // - // Determine the size/length of packet. - // - Packet->Length += (UINT32)(Cursor - Packet->Dhcp6.Option); ASSERT (Packet->Size > Packet->Length + 8); // @@ -1177,8 +1211,7 @@ Dhcp6SendRequestMsg ( Status = Dhcp6CallbackUser (Instance, Dhcp6SendRequest, &Packet); if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // @@ -1194,14 +1227,21 @@ Dhcp6SendRequestMsg ( Status = Dhcp6TransmitPacket (Instance, Packet, Elapsed); if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // // Enqueue the sent packet for the retransmission in case reply timeout. // return Dhcp6EnqueueRetry (Instance, Packet, Elapsed, NULL); + +ON_ERROR: + + if (Packet) { + FreePool (Packet); + } + + return Status; } /** @@ -1266,7 +1306,8 @@ Dhcp6SendDeclineMsg ( // Packet = AllocateZeroPool (DHCP6_BASE_PACKET_SIZE); if (Packet == NULL) { - return EFI_OUT_OF_RESOURCES; + Status = EFI_OUT_OF_RESOURCES; + goto ON_ERROR; } Packet->Size = DHCP6_BASE_PACKET_SIZE; @@ -1280,42 +1321,58 @@ Dhcp6SendDeclineMsg ( Cursor = Packet->Dhcp6.Option; Length = HTONS (ClientId->Length); - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptClientId), Length, ClientId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendETOption ( - Cursor, + Status = Dhcp6AppendETOption ( + Packet, + &Cursor, Instance, &Elapsed ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptServerId), ServerId->Length, ServerId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendIaOption (Cursor, DecIa, 0, 0, Packet->Dhcp6.Header.MessageType); + Status = Dhcp6AppendIaOption ( + Packet, + &Cursor, + DecIa, + 0, + 0, + Packet->Dhcp6.Header.MessageType + ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - // - // Determine the size/length of packet. - // - Packet->Length += (UINT32)(Cursor - Packet->Dhcp6.Option); ASSERT (Packet->Size > Packet->Length + 8); // // Callback to user with the packet to be sent and check the user's feedback. // Status = Dhcp6CallbackUser (Instance, Dhcp6SendDecline, &Packet); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // @@ -1329,16 +1386,22 @@ Dhcp6SendDeclineMsg ( Instance->StartTime = 0; Status = Dhcp6TransmitPacket (Instance, Packet, Elapsed); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // // Enqueue the sent packet for the retransmission in case reply timeout. // return Dhcp6EnqueueRetry (Instance, Packet, Elapsed, NULL); + +ON_ERROR: + + if (Packet) { + FreePool (Packet); + } + + return Status; } /** @@ -1399,7 +1462,8 @@ Dhcp6SendReleaseMsg ( // Packet = AllocateZeroPool (DHCP6_BASE_PACKET_SIZE); if (Packet == NULL) { - return EFI_OUT_OF_RESOURCES; + Status = EFI_OUT_OF_RESOURCES; + goto ON_ERROR; } Packet->Size = DHCP6_BASE_PACKET_SIZE; @@ -1413,45 +1477,61 @@ Dhcp6SendReleaseMsg ( Cursor = Packet->Dhcp6.Option; Length = HTONS (ClientId->Length); - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptClientId), Length, ClientId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } // // ServerId is extracted from packet, it's network order. // - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptServerId), ServerId->Length, ServerId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendETOption ( - Cursor, + Status = Dhcp6AppendETOption ( + Packet, + &Cursor, Instance, &Elapsed ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendIaOption (Cursor, RelIa, 0, 0, Packet->Dhcp6.Header.MessageType); + Status = Dhcp6AppendIaOption ( + Packet, + &Cursor, + RelIa, + 0, + 0, + Packet->Dhcp6.Header.MessageType + ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - // - // Determine the size/length of packet - // - Packet->Length += (UINT32)(Cursor - Packet->Dhcp6.Option); ASSERT (Packet->Size > Packet->Length + 8); // // Callback to user with the packet to be sent and check the user's feedback. // Status = Dhcp6CallbackUser (Instance, Dhcp6SendRelease, &Packet); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // @@ -1461,16 +1541,22 @@ Dhcp6SendReleaseMsg ( Instance->IaCb.Ia->State = Dhcp6Releasing; Status = Dhcp6TransmitPacket (Instance, Packet, Elapsed); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // // Enqueue the sent packet for the retransmission in case reply timeout. // return Dhcp6EnqueueRetry (Instance, Packet, Elapsed, NULL); + +ON_ERROR: + + if (Packet) { + FreePool (Packet); + } + + return Status; } /** @@ -1524,12 +1610,14 @@ Dhcp6SendRenewRebindMsg ( UserLen += (NTOHS (Instance->Config->OptionList[Index]->OpLen) + 4); } + // // Create the Dhcp6 packet and initialize common fields. // Packet = AllocateZeroPool (DHCP6_BASE_PACKET_SIZE + UserLen); if (Packet == NULL) { - return EFI_OUT_OF_RESOURCES; + Status = EFI_OUT_OF_RESOURCES; + goto ON_ERROR; } Packet->Size = DHCP6_BASE_PACKET_SIZE + UserLen; @@ -1543,26 +1631,38 @@ Dhcp6SendRenewRebindMsg ( Cursor = Packet->Dhcp6.Option; Length = HTONS (ClientId->Length); - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptClientId), Length, ClientId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendETOption ( - Cursor, + Status = Dhcp6AppendETOption ( + Packet, + &Cursor, Instance, &Elapsed ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendIaOption ( - Cursor, + Status = Dhcp6AppendIaOption ( + Packet, + &Cursor, Instance->IaCb.Ia, Instance->IaCb.T1, Instance->IaCb.T2, Packet->Dhcp6.Header.MessageType ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } if (!RebindRequest) { // @@ -1578,18 +1678,22 @@ Dhcp6SendRenewRebindMsg ( Dhcp6OptServerId ); if (Option == NULL) { - FreePool (Packet); - return EFI_DEVICE_ERROR; + Status = EFI_DEVICE_ERROR; + goto ON_ERROR; } ServerId = (EFI_DHCP6_DUID *)(Option + 2); - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptServerId), ServerId->Length, ServerId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } } // @@ -1597,18 +1701,19 @@ Dhcp6SendRenewRebindMsg ( // for (Index = 0; Index < Instance->Config->OptionCount; Index++) { UserOpt = Instance->Config->OptionList[Index]; - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, UserOpt->OpCode, UserOpt->OpLen, UserOpt->Data ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } } - // - // Determine the size/length of packet. - // - Packet->Length += (UINT32)(Cursor - Packet->Dhcp6.Option); + ASSERT (Packet->Size > Packet->Length + 8); // @@ -1618,10 +1723,8 @@ Dhcp6SendRenewRebindMsg ( Event = (RebindRequest) ? Dhcp6EnterRebinding : Dhcp6EnterRenewing; Status = Dhcp6CallbackUser (Instance, Event, &Packet); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // @@ -1638,16 +1741,22 @@ Dhcp6SendRenewRebindMsg ( Instance->StartTime = 0; Status = Dhcp6TransmitPacket (Instance, Packet, Elapsed); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // // Enqueue the sent packet for the retransmission in case reply timeout. // return Dhcp6EnqueueRetry (Instance, Packet, Elapsed, NULL); + +ON_ERROR: + + if (Packet) { + FreePool (Packet); + } + + return Status; } /** @@ -1811,7 +1920,8 @@ Dhcp6SendInfoRequestMsg ( // Packet = AllocateZeroPool (DHCP6_BASE_PACKET_SIZE + UserLen); if (Packet == NULL) { - return EFI_OUT_OF_RESOURCES; + Status = EFI_OUT_OF_RESOURCES; + goto ON_ERROR; } Packet->Size = DHCP6_BASE_PACKET_SIZE + UserLen; @@ -1828,44 +1938,56 @@ Dhcp6SendInfoRequestMsg ( if (SendClientId) { Length = HTONS (ClientId->Length); - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptClientId), Length, ClientId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } } - Cursor = Dhcp6AppendETOption ( - Cursor, + Status = Dhcp6AppendETOption ( + Packet, + &Cursor, Instance, &Elapsed ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, OptionRequest->OpCode, OptionRequest->OpLen, OptionRequest->Data ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } // // Append user-defined when configurate Dhcp6 service. // for (Index = 0; Index < OptionCount; Index++) { UserOpt = OptionList[Index]; - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, UserOpt->OpCode, UserOpt->OpLen, UserOpt->Data ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } } - // - // Determine the size/length of packet. - // - Packet->Length += (UINT32)(Cursor - Packet->Dhcp6.Option); ASSERT (Packet->Size > Packet->Length + 8); // @@ -1877,16 +1999,22 @@ Dhcp6SendInfoRequestMsg ( // Send info-request packet with no state. // Status = Dhcp6TransmitPacket (Instance, Packet, Elapsed); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // // Enqueue the sent packet for the retransmission in case reply timeout. // return Dhcp6EnqueueRetry (Instance, Packet, Elapsed, Retransmission); + +ON_ERROR: + + if (Packet) { + FreePool (Packet); + } + + return Status; } /** @@ -1937,7 +2065,8 @@ Dhcp6SendConfirmMsg ( // Packet = AllocateZeroPool (DHCP6_BASE_PACKET_SIZE + UserLen); if (Packet == NULL) { - return EFI_OUT_OF_RESOURCES; + Status = EFI_OUT_OF_RESOURCES; + goto ON_ERROR; } Packet->Size = DHCP6_BASE_PACKET_SIZE + UserLen; @@ -1951,54 +2080,64 @@ Dhcp6SendConfirmMsg ( Cursor = Packet->Dhcp6.Option; Length = HTONS (ClientId->Length); - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, HTONS (Dhcp6OptClientId), Length, ClientId->Duid ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendETOption ( - Cursor, + Status = Dhcp6AppendETOption ( + Packet, + &Cursor, Instance, &Elapsed ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } - Cursor = Dhcp6AppendIaOption ( - Cursor, + Status = Dhcp6AppendIaOption ( + Packet, + &Cursor, Instance->IaCb.Ia, Instance->IaCb.T1, Instance->IaCb.T2, Packet->Dhcp6.Header.MessageType ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } // // Append user-defined when configurate Dhcp6 service. // for (Index = 0; Index < Instance->Config->OptionCount; Index++) { UserOpt = Instance->Config->OptionList[Index]; - Cursor = Dhcp6AppendOption ( - Cursor, + Status = Dhcp6AppendOption ( + Packet, + &Cursor, UserOpt->OpCode, UserOpt->OpLen, UserOpt->Data ); + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } } - // - // Determine the size/length of packet. - // - Packet->Length += (UINT32)(Cursor - Packet->Dhcp6.Option); ASSERT (Packet->Size > Packet->Length + 8); // // Callback to user with the packet to be sent and check the user's feedback. // Status = Dhcp6CallbackUser (Instance, Dhcp6SendConfirm, &Packet); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // @@ -2012,16 +2151,22 @@ Dhcp6SendConfirmMsg ( Instance->StartTime = 0; Status = Dhcp6TransmitPacket (Instance, Packet, Elapsed); - if (EFI_ERROR (Status)) { - FreePool (Packet); - return Status; + goto ON_ERROR; } // // Enqueue the sent packet for the retransmission in case reply timeout. // return Dhcp6EnqueueRetry (Instance, Packet, Elapsed, NULL); + +ON_ERROR: + + if (Packet) { + FreePool(Packet); + } + + return Status; } /** diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c b/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c index e6368b5b1c..8f4bd1eb0f 100644 --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c @@ -142,12 +142,12 @@ Dhcp6GenerateClientId ( } Status = gRT->SetVariable ( - L"ClientId", - &gEfiDhcp6ServiceBindingProtocolGuid, - (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS), - Duid->Length + 2, - (VOID *)Duid - ); + L"ClientId", + &gEfiDhcp6ServiceBindingProtocolGuid, + (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS), + Duid->Length + 2, + (VOID *)Duid + ); if (EFI_ERROR (Status)) { FreePool (Duid); return NULL; @@ -192,10 +192,10 @@ Dhcp6CopyConfigData ( } CopyMem ( - DstCfg->SolicitRetransmission, - SorCfg->SolicitRetransmission, - sizeof (EFI_DHCP6_RETRANSMISSION) - ); + DstCfg->SolicitRetransmission, + SorCfg->SolicitRetransmission, + sizeof (EFI_DHCP6_RETRANSMISSION) + ); } if ((SorCfg->OptionList != NULL) && (SorCfg->OptionCount != 0)) { @@ -221,10 +221,10 @@ Dhcp6CopyConfigData ( } CopyMem ( - DstCfg->OptionList[Index], - SorCfg->OptionList[Index], - OptionSize - ); + DstCfg->OptionList[Index], + SorCfg->OptionList[Index], + OptionSize + ); } } @@ -422,10 +422,10 @@ Dhcp6CheckAddress ( for (Index2 = 0; Index2 < Ia->IaAddressCount; Index2++) { if (CompareMem ( - &Addresses[Index1], - &Ia->IaAddress[Index2], - sizeof (EFI_IPv6_ADDRESS) - ) == 0) + &Addresses[Index1], + &Ia->IaAddress[Index2], + sizeof (EFI_IPv6_ADDRESS) + ) == 0) { Found = TRUE; break; @@ -503,28 +503,28 @@ Dhcp6DepriveAddress ( for (Index2 = 0; Index2 < Ia->IaAddressCount; Index2++) { if (CompareMem ( - &Addresses[Index1], - &Ia->IaAddress[Index2], - sizeof (EFI_IPv6_ADDRESS) - ) == 0) + &Addresses[Index1], + &Ia->IaAddress[Index2], + sizeof (EFI_IPv6_ADDRESS) + ) == 0) { // // Copy the deprived address to the copy of Ia // CopyMem ( - &IaCopy->IaAddress[Index1], - &Ia->IaAddress[Index2], - sizeof (EFI_DHCP6_IA_ADDRESS) - ); + &IaCopy->IaAddress[Index1], + &Ia->IaAddress[Index2], + sizeof (EFI_DHCP6_IA_ADDRESS) + ); // // Delete the deprived address from the instance Ia // if (Index2 + 1 < Ia->IaAddressCount) { CopyMem ( - &Ia->IaAddress[Index2], - &Ia->IaAddress[Index2 + 1], - (Ia->IaAddressCount - Index2 - 1) * sizeof (EFI_DHCP6_IA_ADDRESS) - ); + &Ia->IaAddress[Index2], + &Ia->IaAddress[Index2 + 1], + (Ia->IaAddressCount - Index2 - 1) * sizeof (EFI_DHCP6_IA_ADDRESS) + ); } Found = TRUE; @@ -577,24 +577,33 @@ Dhcp6OnTransmitted ( } /** - Append the option to Buf, and move Buf to the end. + Append the option to Buf, update the length of packet, and move Buf to the end. - @param[in, out] Buf The pointer to the buffer. - @param[in] OptType The option type. - @param[in] OptLen The length of option contents. - @param[in] Data The pointer to the option content. + @param[in, out] Packet A pointer to the packet, on success Packet->Length + will be updated. + @param[in, out] PacketCursor The pointer in the packet, on success PacketCursor + will be moved to the end of the option. + @param[in] OptType The option type. + @param[in] OptLen The length of option contents. + @param[in] Data The pointer to the option content. - @return Buf The position to append the next option. + @retval EFI_INVALID_PARAMETER An argument provided to the function was invalid + @retval EFI_BUFFER_TOO_SMALL The buffer is too small to append the option. + @retval EFI_SUCCESS The option is appended successfully. **/ -UINT8 * +EFI_STATUS Dhcp6AppendOption ( - IN OUT UINT8 *Buf, - IN UINT16 OptType, - IN UINT16 OptLen, - IN UINT8 *Data + IN OUT EFI_DHCP6_PACKET *Packet, + IN OUT UINT8 **PacketCursor, + IN UINT16 OptType, + IN UINT16 OptLen, + IN UINT8 *Data ) { + UINT32 Length; + UINT32 BytesNeeded; + // // The format of Dhcp6 option: // @@ -607,35 +616,95 @@ Dhcp6AppendOption ( // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // - ASSERT (OptLen != 0); + // + // Verify the arguments are valid + // + if (Packet == NULL) { + return EFI_INVALID_PARAMETER; + } + + if ((PacketCursor == NULL) || (*PacketCursor == NULL)) { + return EFI_INVALID_PARAMETER; + } + + if (Data == NULL) { + return EFI_INVALID_PARAMETER; + } + + if (OptLen == 0) { + return EFI_INVALID_PARAMETER; + } + + // + // Verify the PacketCursor is within the packet + // + if ( (*PacketCursor < Packet->Dhcp6.Option) + || (*PacketCursor >= Packet->Dhcp6.Option + (Packet->Size - sizeof (EFI_DHCP6_HEADER)))) + { + return EFI_INVALID_PARAMETER; + } + + // + // Calculate the bytes needed for the option + // + BytesNeeded = DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN + NTOHS(OptLen); - WriteUnaligned16 ((UINT16 *)Buf, OptType); - Buf += 2; - WriteUnaligned16 ((UINT16 *)Buf, OptLen); - Buf += 2; - CopyMem (Buf, Data, NTOHS (OptLen)); - Buf += NTOHS (OptLen); + // + // Space remaining in the packet + // + Length = Packet->Size - Packet->Length; + if (Length < BytesNeeded) { + return EFI_BUFFER_TOO_SMALL; + } + + // + // Verify the PacketCursor is within the packet + // + if ( (*PacketCursor < Packet->Dhcp6.Option) + || (*PacketCursor >= Packet->Dhcp6.Option + (Packet->Size - sizeof (EFI_DHCP6_HEADER)))) + { + return EFI_INVALID_PARAMETER; + } - return Buf; + WriteUnaligned16 ((UINT16 *)*PacketCursor, OptType); + *PacketCursor += DHCP6_SIZE_OF_OPT_CODE; + WriteUnaligned16 ((UINT16 *)*PacketCursor, OptLen); + *PacketCursor += DHCP6_SIZE_OF_OPT_LEN; + CopyMem (*PacketCursor, Data, NTOHS (OptLen)); + *PacketCursor += NTOHS (OptLen); + + // Update the packet length by the length of the option + 4 bytes + Packet->Length += BytesNeeded; + + return EFI_SUCCESS; } /** Append the appointed IA Address option to Buf, and move Buf to the end. - @param[in, out] Buf The pointer to the position to append. + @param[in, out] Packet A pointer to the packet, on success Packet->Length + will be updated. + @param[in, out] PacketCursor The pointer in the packet, on success PacketCursor + will be moved to the end of the option. @param[in] IaAddr The pointer to the IA Address. @param[in] MessageType Message type of DHCP6 package. - @return Buf The position to append the next option. + @retval EFI_INVALID_PARAMETER An argument provided to the function was invalid + @retval EFI_BUFFER_TOO_SMALL The buffer is too small to append the option. + @retval EFI_SUCCESS The option is appended successfully. **/ -UINT8 * +EFI_STATUS Dhcp6AppendIaAddrOption ( - IN OUT UINT8 *Buf, + IN OUT EFI_DHCP6_PACKET *Packet, + IN OUT UINT8 **PacketCursor, IN EFI_DHCP6_IA_ADDRESS *IaAddr, IN UINT32 MessageType ) { + UINT32 BytesNeeded; + UINT32 Length; + // The format of the IA Address option is: // // 0 1 2 3 @@ -657,17 +726,60 @@ Dhcp6AppendIaAddrOption ( // . . // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // + // Verify the arguments are valid + // + if (Packet == NULL) { + return EFI_INVALID_PARAMETER; + } + + if ((PacketCursor == NULL) || (*PacketCursor == NULL)) { + return EFI_INVALID_PARAMETER; + } + + if (IaAddr == NULL) { + return EFI_INVALID_PARAMETER; + } + + // + // Verify the PacketCursor is within the packet + // + if ( (*PacketCursor < Packet->Dhcp6.Option) + || (*PacketCursor >= Packet->Dhcp6.Option + (Packet->Size - sizeof (EFI_DHCP6_HEADER)))) + { + return EFI_INVALID_PARAMETER; + } + + BytesNeeded = DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN; + BytesNeeded += sizeof (EFI_IPv6_ADDRESS); + // + // Even if the preferred-lifetime is 0, it still needs to store it. + // + BytesNeeded += sizeof (IaAddr->PreferredLifetime); + // + // Even if the valid-lifetime is 0, it still needs to store it. + // + BytesNeeded += sizeof (IaAddr->ValidLifetime); + + // + // Space remaining in the packet + // + Length = Packet->Size - Packet->Length; + if (Length < BytesNeeded) { + return EFI_BUFFER_TOO_SMALL; + } + // // Fill the value of Ia Address option type // - WriteUnaligned16 ((UINT16 *)Buf, HTONS (Dhcp6OptIaAddr)); - Buf += 2; + WriteUnaligned16 ((UINT16 *)*PacketCursor, HTONS (Dhcp6OptIaAddr)); + *PacketCursor += DHCP6_SIZE_OF_OPT_CODE; - WriteUnaligned16 ((UINT16 *)Buf, HTONS (sizeof (EFI_DHCP6_IA_ADDRESS))); - Buf += 2; + WriteUnaligned16 ((UINT16 *)*PacketCursor, HTONS (sizeof (EFI_DHCP6_IA_ADDRESS))); + *PacketCursor += DHCP6_SIZE_OF_OPT_LEN; - CopyMem (Buf, &IaAddr->IpAddress, sizeof (EFI_IPv6_ADDRESS)); - Buf += sizeof (EFI_IPv6_ADDRESS); + CopyMem (*PacketCursor, &IaAddr->IpAddress, sizeof (EFI_IPv6_ADDRESS)); + *PacketCursor += sizeof (EFI_IPv6_ADDRESS); // // Fill the value of preferred-lifetime and valid-lifetime. @@ -675,44 +787,58 @@ Dhcp6AppendIaAddrOption ( // should set to 0 when initiate a Confirm message. // if (MessageType != Dhcp6MsgConfirm) { - WriteUnaligned32 ((UINT32 *)Buf, HTONL (IaAddr->PreferredLifetime)); + WriteUnaligned32 ((UINT32 *)*PacketCursor, HTONL (IaAddr->PreferredLifetime)); } - Buf += 4; + *PacketCursor += sizeof (IaAddr->PreferredLifetime); if (MessageType != Dhcp6MsgConfirm) { - WriteUnaligned32 ((UINT32 *)Buf, HTONL (IaAddr->ValidLifetime)); + WriteUnaligned32 ((UINT32 *)*PacketCursor, HTONL (IaAddr->ValidLifetime)); } - Buf += 4; + *PacketCursor += sizeof (IaAddr->ValidLifetime); + + // + // Update the packet length + // + Packet->Length += BytesNeeded; - return Buf; + return EFI_SUCCESS; } /** Append the appointed Ia option to Buf, and move Buf to the end. - @param[in, out] Buf The pointer to the position to append. + @param[in, out] Packet A pointer to the packet, on success Packet->Length + will be updated. + @param[in, out] PacketCursor The pointer in the packet, on success PacketCursor + will be moved to the end of the option. @param[in] Ia The pointer to the Ia. @param[in] T1 The time of T1. @param[in] T2 The time of T2. @param[in] MessageType Message type of DHCP6 package. - @return Buf The position to append the next Ia option. + @retval EFI_INVALID_PARAMETER An argument provided to the function was invalid + @retval EFI_BUFFER_TOO_SMALL The buffer is too small to append the option. + @retval EFI_SUCCESS The option is appended successfully. **/ -UINT8 * +EFI_STATUS Dhcp6AppendIaOption ( - IN OUT UINT8 *Buf, - IN EFI_DHCP6_IA *Ia, - IN UINT32 T1, - IN UINT32 T2, - IN UINT32 MessageType + IN OUT EFI_DHCP6_PACKET *Packet, + IN OUT UINT8 **PacketCursor, + IN EFI_DHCP6_IA *Ia, + IN UINT32 T1, + IN UINT32 T2, + IN UINT32 MessageType ) { - UINT8 *AddrOpt; - UINT16 *Len; - UINTN Index; + UINT8 *AddrOpt; + UINT16 *Len; + UINTN Index; + UINT32 BytesNeeded; + UINT32 Length; + EFI_STATUS Status; // // The format of IA_NA and IA_TA option: @@ -733,32 +859,74 @@ Dhcp6AppendIaOption ( // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // + // + // Verify the arguments are valid + // + if (Packet == NULL) { + return EFI_INVALID_PARAMETER; + } + + if ((PacketCursor == NULL) || (*PacketCursor == NULL)) { + return EFI_INVALID_PARAMETER; + } + + if (Ia == NULL) { + return EFI_INVALID_PARAMETER; + } + + // + // Verify the PacketCursor is within the packet + // + if ( (*PacketCursor < Packet->Dhcp6.Option) + || (*PacketCursor >= Packet->Dhcp6.Option + (Packet->Size - sizeof (EFI_DHCP6_HEADER)))) + { + return EFI_INVALID_PARAMETER; + } + + BytesNeeded = DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN; + BytesNeeded += sizeof (Ia->Descriptor.IaId); + // + // + N for the IA_NA-options/IA_TA-options + // Dhcp6AppendIaAddrOption will need to check the length for each address + // + if (Ia->Descriptor.Type == Dhcp6OptIana) { + BytesNeeded += sizeof (T1) + sizeof (T2); + } + + // + // Space remaining in the packet + // + Length = (UINT16)(Packet->Size - Packet->Length); + if (Length < BytesNeeded) { + return EFI_BUFFER_TOO_SMALL; + } + // // Fill the value of Ia option type // - WriteUnaligned16 ((UINT16 *)Buf, HTONS (Ia->Descriptor.Type)); - Buf += 2; + WriteUnaligned16 ((UINT16 *)*PacketCursor, HTONS (Ia->Descriptor.Type)); + *PacketCursor += DHCP6_SIZE_OF_OPT_CODE; // // Fill the len of Ia option later, keep the pointer first // - Len = (UINT16 *)Buf; - Buf += 2; + Len = (UINT16 *)*PacketCursor; + *PacketCursor += DHCP6_SIZE_OF_OPT_LEN; // // Fill the value of iaid // - WriteUnaligned32 ((UINT32 *)Buf, HTONL (Ia->Descriptor.IaId)); - Buf += 4; + WriteUnaligned32 ((UINT32 *)*PacketCursor, HTONL (Ia->Descriptor.IaId)); + *PacketCursor += sizeof (Ia->Descriptor.IaId); // // Fill the value of t1 and t2 if iana, keep it 0xffffffff if no specified. // if (Ia->Descriptor.Type == Dhcp6OptIana) { - WriteUnaligned32 ((UINT32 *)Buf, HTONL ((T1 != 0) ? T1 : 0xffffffff)); - Buf += 4; - WriteUnaligned32 ((UINT32 *)Buf, HTONL ((T2 != 0) ? T2 : 0xffffffff)); - Buf += 4; + WriteUnaligned32 ((UINT32 *)*PacketCursor, HTONL ((T1 != 0) ? T1 : 0xffffffff)); + *PacketCursor += sizeof (T1); + WriteUnaligned32 ((UINT32 *)*PacketCursor, HTONL ((T2 != 0) ? T2 : 0xffffffff)); + *PacketCursor += sizeof (T2); } // @@ -766,35 +934,51 @@ Dhcp6AppendIaOption ( // for (Index = 0; Index < Ia->IaAddressCount; Index++) { AddrOpt = (UINT8 *)Ia->IaAddress + Index * sizeof (EFI_DHCP6_IA_ADDRESS); - Buf = Dhcp6AppendIaAddrOption (Buf, (EFI_DHCP6_IA_ADDRESS *)AddrOpt, MessageType); + Status = Dhcp6AppendIaAddrOption (Packet, PacketCursor, (EFI_DHCP6_IA_ADDRESS *)AddrOpt, MessageType); + if (EFI_ERROR (Status)) { + return Status; + } } // // Fill the value of Ia option length // - *Len = HTONS ((UINT16)(Buf - (UINT8 *)Len - 2)); + *Len = HTONS ((UINT16)(*PacketCursor - (UINT8 *)Len - 2)); - return Buf; + // + // Update the packet length + // + Packet->Length += BytesNeeded; + + return EFI_SUCCESS; } /** Append the appointed Elapsed time option to Buf, and move Buf to the end. - @param[in, out] Buf The pointer to the position to append. + @param[in, out] Packet A pointer to the packet, on success Packet->Length + @param[in, out] PacketCursor The pointer in the packet, on success PacketCursor + will be moved to the end of the option. @param[in] Instance The pointer to the Dhcp6 instance. @param[out] Elapsed The pointer to the elapsed time value in - the generated packet. + the generated packet. - @return Buf The position to append the next Ia option. + @retval EFI_INVALID_PARAMETER An argument provided to the function was invalid + @retval EFI_BUFFER_TOO_SMALL The buffer is too small to append the option. + @retval EFI_SUCCESS The option is appended successfully. **/ -UINT8 * +EFI_STATUS Dhcp6AppendETOption ( - IN OUT UINT8 *Buf, - IN DHCP6_INSTANCE *Instance, - OUT UINT16 **Elapsed + IN OUT EFI_DHCP6_PACKET *Packet, + IN OUT UINT8 **PacketCursor, + IN DHCP6_INSTANCE *Instance, + OUT UINT16 **Elapsed ) { + UINT32 BytesNeeded; + UINT32 Length; + // // The format of elapsed time option: // @@ -806,27 +990,70 @@ Dhcp6AppendETOption ( // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // + // + // Verify the arguments are valid + // + if (Packet == NULL) { + return EFI_INVALID_PARAMETER; + } + + if ((PacketCursor == NULL) || (*PacketCursor == NULL)) { + return EFI_INVALID_PARAMETER; + } + + if (Instance == NULL) { + return EFI_INVALID_PARAMETER; + } + + if ((Elapsed == NULL)) { + return EFI_INVALID_PARAMETER; + } + + // + // Verify the PacketCursor is within the packet + // + if ( (*PacketCursor < Packet->Dhcp6.Option) + || (*PacketCursor >= Packet->Dhcp6.Option + (Packet->Size - sizeof (EFI_DHCP6_HEADER)))) + { + return EFI_INVALID_PARAMETER; + } + + BytesNeeded = DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN; + // + // + 2 for elapsed-time + // + BytesNeeded += sizeof (UINT16); + // + // Space remaining in the packet + // + Length = Packet->Size - Packet->Length; + if (Length < BytesNeeded) { + return EFI_BUFFER_TOO_SMALL; + } + // // Fill the value of elapsed-time option type. // - WriteUnaligned16 ((UINT16 *)Buf, HTONS (Dhcp6OptElapsedTime)); - Buf += 2; + WriteUnaligned16 ((UINT16 *)*PacketCursor, HTONS (Dhcp6OptElapsedTime)); + *PacketCursor += DHCP6_SIZE_OF_OPT_CODE; // // Fill the len of elapsed-time option, which is fixed. // - WriteUnaligned16 ((UINT16 *)Buf, HTONS (2)); - Buf += 2; + WriteUnaligned16 ((UINT16 *)*PacketCursor, HTONS (2)); + *PacketCursor += DHCP6_SIZE_OF_OPT_LEN; // // Fill in elapsed time value with 0 value for now. The actual value is // filled in later just before the packet is transmitted. // - WriteUnaligned16 ((UINT16 *)Buf, HTONS (0)); - *Elapsed = (UINT16 *)Buf; - Buf += 2; + WriteUnaligned16 ((UINT16 *)*PacketCursor, HTONS (0)); + *Elapsed = (UINT16 *)*PacketCursor; + *PacketCursor += sizeof (UINT16); + + Packet->Length += BytesNeeded; - return Buf; + return EFI_SUCCESS; } /** @@ -852,13 +1079,13 @@ SetElapsedTime ( // gRT->GetTime (&Time, NULL); CurrentStamp = MultU64x32 ( - ((((UINT32)(Time.Year - 2000) * 360 + (Time.Month - 1) * 30 + (Time.Day - 1)) * 24 + Time.Hour) * 60 + Time.Minute) * 60 + Time.Second, - 100 - ) + + ((((UINT32)(Time.Year - 2000) * 360 + (Time.Month - 1) * 30 + (Time.Day - 1)) * 24 + Time.Hour) * 60 + Time.Minute) * 60 + Time.Second, + 100 + ) + DivU64x32 ( - Time.Nanosecond, - 10000000 - ); + Time.Nanosecond, + 10000000 + ); // // Sentinel value of 0 means that this is the first DHCP packet that we are @@ -1290,11 +1517,11 @@ Dhcp6GetMappingTimeOut ( DataSize = sizeof (EFI_IP6_CONFIG_DUP_ADDR_DETECT_TRANSMITS); Status = Ip6Cfg->GetData ( - Ip6Cfg, - Ip6ConfigDataTypeDupAddrDetectTransmits, - &DataSize, - &DadXmits - ); + Ip6Cfg, + Ip6ConfigDataTypeDupAddrDetectTransmits, + &DataSize, + &DadXmits + ); if (EFI_ERROR (Status)) { return Status; } diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.h b/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.h index 046454ff4a..06947f6c1f 100644 --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.h +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.h @@ -160,69 +160,85 @@ Dhcp6OnTransmitted ( ); /** - Append the appointed option to the buf, and move the buf to the end. - - @param[in, out] Buf The pointer to buffer. - @param[in] OptType The option type. - @param[in] OptLen The length of option content.s - @param[in] Data The pointer to the option content. - - @return Buf The position to append the next option. - + Append the option to Buf, update the length of packet, and move Buf to the end. + + @param[in, out] Packet A pointer to the packet, on success Packet->Length + will be updated. + @param[in, out] PacketCursor The pointer in the packet, on success PacketCursor + will be moved to the end of the option. + @param[in] OptType The option type. + @param[in] OptLen The length of option contents. + @param[in] Data The pointer to the option content. + + @retval EFI_INVALID_PARAMETER An argument provided to the function was invalid + @retval EFI_BUFFER_TOO_SMALL The buffer is too small to append the option. + @retval EFI_SUCCESS The option is appended successfully. **/ -UINT8 * +EFI_STATUS Dhcp6AppendOption ( - IN OUT UINT8 *Buf, - IN UINT16 OptType, - IN UINT16 OptLen, - IN UINT8 *Data + IN OUT EFI_DHCP6_PACKET *Packet, + IN OUT UINT8 **PacketCursor, + IN UINT16 OptType, + IN UINT16 OptLen, + IN UINT8 *Data ); /** - Append the Ia option to Buf, and move Buf to the end. - - @param[in, out] Buf The pointer to the position to append. + Append the appointed Ia option to Buf, update the Ia option length, and move Buf + to the end of the option. + @param[in, out] Packet A pointer to the packet, on success Packet->Length + will be updated. + @param[in, out] PacketCursor The pointer in the packet, on success PacketCursor + will be moved to the end of the option. @param[in] Ia The pointer to the Ia. @param[in] T1 The time of T1. @param[in] T2 The time of T2. @param[in] MessageType Message type of DHCP6 package. - @return Buf The position to append the next Ia option. - + @retval EFI_INVALID_PARAMETER An argument provided to the function was invalid + @retval EFI_BUFFER_TOO_SMALL The buffer is too small to append the option. + @retval EFI_SUCCESS The option is appended successfully. **/ -UINT8 * +EFI_STATUS Dhcp6AppendIaOption ( - IN OUT UINT8 *Buf, - IN EFI_DHCP6_IA *Ia, - IN UINT32 T1, - IN UINT32 T2, - IN UINT32 MessageType + IN OUT EFI_DHCP6_PACKET *Packet, + IN OUT UINT8 **PacketCursor, + IN EFI_DHCP6_IA *Ia, + IN UINT32 T1, + IN UINT32 T2, + IN UINT32 MessageType ); /** Append the appointed Elapsed time option to Buf, and move Buf to the end. - @param[in, out] Buf The pointer to the position to append. + @param[in, out] Packet A pointer to the packet, on success Packet->Length + @param[in, out] PacketCursor The pointer in the packet, on success PacketCursor + will be moved to the end of the option. @param[in] Instance The pointer to the Dhcp6 instance. @param[out] Elapsed The pointer to the elapsed time value in the generated packet. - @return Buf The position to append the next Ia option. + @retval EFI_INVALID_PARAMETER An argument provided to the function was invalid + @retval EFI_BUFFER_TOO_SMALL The buffer is too small to append the option. + @retval EFI_SUCCESS The option is appended successfully. **/ -UINT8 * +EFI_STATUS Dhcp6AppendETOption ( - IN OUT UINT8 *Buf, - IN DHCP6_INSTANCE *Instance, - OUT UINT16 **Elapsed + IN OUT EFI_DHCP6_PACKET *Packet, + IN OUT UINT8 **PacketCursor, + IN DHCP6_INSTANCE *Instance, + OUT UINT16 **Elapsed ); /** Set the elapsed time based on the given instance and the pointer to the elapsed time option. - @param[in] Elapsed The pointer to the position to append. - @param[in] Instance The pointer to the Dhcp6 instance. + @retval EFI_INVALID_PARAMETER An argument provided to the function was invalid + @retval EFI_BUFFER_TOO_SMALL The buffer is too small to append the option. + @retval EFI_SUCCESS The option is appended successfully. **/ VOID SetElapsedTime ( -- 2.41.0 From 1bd40aba86eecc8acc07547e6d14e563590b1074 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Fri, 15 Dec 2023 11:26:04 -0800 Subject: [PATCH 02/12] SECURITY PATCH TCBZ4535 - CVE-2023-45230 - Host Based Unit Test --- .../GoogleTest/Dhcp6DxeGoogleTest.cpp | 27 + .../GoogleTest/Dhcp6DxeGoogleTest.inf | 44 ++ .../Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp | 478 ++++++++++++++++++ NetworkPkg/NetworkPkg.ci.yaml | 3 + NetworkPkg/Test/NetworkPkgHostTest.dsc | 102 ++++ 5 files changed, 654 insertions(+) create mode 100644 NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp create mode 100644 NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf create mode 100644 NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp create mode 100644 NetworkPkg/Test/NetworkPkgHostTest.dsc diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp new file mode 100644 index 0000000000..b1fe72e195 --- /dev/null +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp @@ -0,0 +1,27 @@ +/** @file + Acts as the main entry point for the tests for the Dhcp6Dxe module. + + Copyright (c) Microsoft Corporation + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ +#include + +//////////////////////////////////////////////////////////////////////////////// +// Add test files here +// Google Test will only pick up the tests from the files that are included +// here. +//////////////////////////////////////////////////////////////////////////////// +#include "Dhcp6IoGoogleTest.cpp" + +//////////////////////////////////////////////////////////////////////////////// +// Run the tests +//////////////////////////////////////////////////////////////////////////////// +int +main ( + int argc, + char *argv[] + ) +{ + testing::InitGoogleTest (&argc, argv); + return RUN_ALL_TESTS (); +} diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf new file mode 100644 index 0000000000..c7ec42b322 --- /dev/null +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf @@ -0,0 +1,44 @@ +## @file +# Unit test suite for the Dhcp6Dxe using Google Test +# +# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent +## +[Defines] + INF_VERSION = 0x00010017 + BASE_NAME = Dhcp6DxeGoogleTest + FILE_GUID = 1D2A4C65-38C8-4C2F-BB60-B5FA49625AA9 + 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] + Dhcp6DxeGoogleTest.cpp + Dhcp6IoGoogleTest.cpp + ../Dhcp6Io.c + ../Dhcp6Utility.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] + gZeroGuid \ No newline at end of file diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp new file mode 100644 index 0000000000..dad6a42b12 --- /dev/null +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp @@ -0,0 +1,478 @@ +/** @file + Tests for Dhcp6Io.c. + + Copyright (c) Microsoft Corporation + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ +#include + +extern "C" { + #include + #include + #include + #include + #include "../Dhcp6Impl.h" + #include "../Dhcp6Utility.h" +} + +//////////////////////////////////////////////////////////////////////// +// Defines +//////////////////////////////////////////////////////////////////////// + +#define DHCP6_PACKET_MAX_LEN 1500 + +//////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////// +// Symbol Definitions +// These functions are not directly under test - but required to compile +//////////////////////////////////////////////////////////////////////// + +// This definition is used by this test but is also required to compile +// by Dhcp6Io.c +EFI_IPv6_ADDRESS mAllDhcpRelayAndServersAddress = { + { 0xFF, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 2 } +}; + +EFI_STATUS +EFIAPI +UdpIoSendDatagram ( + IN UDP_IO *UdpIo, + IN NET_BUF *Packet, + IN UDP_END_POINT *EndPoint OPTIONAL, + IN EFI_IP_ADDRESS *Gateway OPTIONAL, + IN UDP_IO_CALLBACK CallBack, + IN VOID *Context + ) +{ + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI +UdpIoRecvDatagram ( + IN UDP_IO *UdpIo, + IN UDP_IO_CALLBACK CallBack, + IN VOID *Context, + IN UINT32 HeadLen + ) +{ + return EFI_SUCCESS; +} + +//////////////////////////////////////////////////////////////////////// +// Dhcp6AppendOptionTest Tests +//////////////////////////////////////////////////////////////////////// + +class Dhcp6AppendOptionTest : public ::testing::Test { +public: + UINT8 *Buffer = NULL; + EFI_DHCP6_PACKET *Packet; + +protected: + // Add any setup code if needed + virtual void + SetUp ( + ) + { + // Initialize any resources or variables + Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); + ASSERT_NE (Buffer, (UINT8 *)NULL); + + Packet = (EFI_DHCP6_PACKET *)Buffer; + Packet->Size = DHCP6_PACKET_MAX_LEN; + } + + // Add any cleanup code if needed + virtual void + TearDown ( + ) + { + // Clean up any resources or variables + if (Buffer != NULL) { + FreePool (Buffer); + } + } +}; + +// Test Description: +// Attempt to append an option to a packet that is too small by a duid that is too large +TEST_F (Dhcp6AppendOptionTest, InvalidDataExpectBufferTooSmall) { + UINT8 *Cursor; + EFI_DHCP6_DUID *UntrustedDuid; + EFI_STATUS Status; + + UntrustedDuid = (EFI_DHCP6_DUID *)AllocateZeroPool (sizeof (EFI_DHCP6_DUID)); + ASSERT_NE (UntrustedDuid, (EFI_DHCP6_DUID *)NULL); + + UntrustedDuid->Length = NTOHS (0xFFFF); + + Cursor = Dhcp6AppendOptionTest::Packet->Dhcp6.Option; + + Status = Dhcp6AppendOption ( + Dhcp6AppendOptionTest::Packet, + &Cursor, + HTONS (Dhcp6OptServerId), + UntrustedDuid->Length, + UntrustedDuid->Duid + ); + + ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); +} + +// Test Description: +// Attempt to append an option to a packet that is large enough +TEST_F (Dhcp6AppendOptionTest, ValidDataExpectSuccess) { + UINT8 *Cursor; + EFI_DHCP6_DUID *UntrustedDuid; + EFI_STATUS Status; + UINTN OriginalLength; + + UINT8 Duid[6] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 }; + + Packet->Length = sizeof (EFI_DHCP6_HEADER); + OriginalLength = Packet->Length; + + UntrustedDuid = (EFI_DHCP6_DUID *)AllocateZeroPool (sizeof (EFI_DHCP6_DUID)); + ASSERT_NE (UntrustedDuid, (EFI_DHCP6_DUID *)NULL); + + UntrustedDuid->Length = NTOHS (sizeof (Duid)); + CopyMem (UntrustedDuid->Duid, Duid, sizeof (Duid)); + + Cursor = Dhcp6AppendOptionTest::Packet->Dhcp6.Option; + + Status = Dhcp6AppendOption ( + Dhcp6AppendOptionTest::Packet, + &Cursor, + HTONS (Dhcp6OptServerId), + UntrustedDuid->Length, + UntrustedDuid->Duid + ); + + ASSERT_EQ (Status, EFI_SUCCESS); + + // verify that the pointer to cursor moved by the expected amount + ASSERT_EQ (Cursor, (UINT8 *)Dhcp6AppendOptionTest::Packet->Dhcp6.Option + sizeof (Duid) + 4); + + // verify that the length of the packet is now the expected amount + ASSERT_EQ (Dhcp6AppendOptionTest::Packet->Length, OriginalLength + sizeof (Duid) + 4); +} + +//////////////////////////////////////////////////////////////////////// +// Dhcp6AppendETOption Tests +//////////////////////////////////////////////////////////////////////// + +class Dhcp6AppendETOptionTest : public ::testing::Test { +public: + UINT8 *Buffer = NULL; + EFI_DHCP6_PACKET *Packet; + +protected: + // Add any setup code if needed + virtual void + SetUp ( + ) + { + // Initialize any resources or variables + Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); + ASSERT_NE (Buffer, (UINT8 *)NULL); + + Packet = (EFI_DHCP6_PACKET *)Buffer; + Packet->Size = DHCP6_PACKET_MAX_LEN; + Packet->Length = sizeof (EFI_DHCP6_HEADER); + } + + // Add any cleanup code if needed + virtual void + TearDown ( + ) + { + // Clean up any resources or variables + if (Buffer != NULL) { + FreePool (Buffer); + } + } +}; + +// Test Description: +// Attempt to append an option to a packet that is too small by a duid that is too large +TEST_F (Dhcp6AppendETOptionTest, InvalidDataExpectBufferTooSmall) { + UINT8 *Cursor; + EFI_STATUS Status; + DHCP6_INSTANCE Instance; + UINT16 ElapsedTimeVal; + UINT16 *ElapsedTime; + + Cursor = Dhcp6AppendETOptionTest::Packet->Dhcp6.Option; + ElapsedTime = &ElapsedTimeVal; + + Packet->Length = Packet->Size - 2; + + Status = Dhcp6AppendETOption ( + Dhcp6AppendETOptionTest::Packet, + &Cursor, + &Instance, // Instance is not used in this function + &ElapsedTime + ); + + // verify that we error out because the packet is too small for the option header + ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); + + // reset the length + Packet->Length = sizeof (EFI_DHCP6_HEADER); +} + +// Test Description: +// Attempt to append an option to a packet that is large enough +TEST_F (Dhcp6AppendETOptionTest, ValidDataExpectSuccess) { + UINT8 *Cursor; + EFI_STATUS Status; + DHCP6_INSTANCE Instance; + UINT16 ElapsedTimeVal; + UINT16 *ElapsedTime; + UINTN ExpectedSize; + UINTN OriginalLength; + + Cursor = Dhcp6AppendETOptionTest::Packet->Dhcp6.Option; + ElapsedTime = &ElapsedTimeVal; + ExpectedSize = 6; + OriginalLength = Packet->Length; + + Status = Dhcp6AppendETOption ( + Dhcp6AppendETOptionTest::Packet, + &Cursor, + &Instance, // Instance is not used in this function + &ElapsedTime + ); + + // verify that the status is EFI_SUCCESS + ASSERT_EQ (Status, EFI_SUCCESS); + + // verify that the pointer to cursor moved by the expected amount + ASSERT_EQ (Cursor, (UINT8 *)Dhcp6AppendETOptionTest::Packet->Dhcp6.Option + ExpectedSize); + + // verify that the length of the packet is now the expected amount + ASSERT_EQ (Dhcp6AppendETOptionTest::Packet->Length, OriginalLength + ExpectedSize); +} + +//////////////////////////////////////////////////////////////////////// +// Dhcp6AppendIaOption Tests +//////////////////////////////////////////////////////////////////////// + +class Dhcp6AppendIaOptionTest : public ::testing::Test { +public: + UINT8 *Buffer = NULL; + EFI_DHCP6_PACKET *Packet; + EFI_DHCP6_IA *Ia; + +protected: + // Add any setup code if needed + virtual void + SetUp ( + ) + { + // Initialize any resources or variables + Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); + ASSERT_NE (Buffer, (UINT8 *)NULL); + + Packet = (EFI_DHCP6_PACKET *)Buffer; + Packet->Size = DHCP6_PACKET_MAX_LEN; + + Ia = (EFI_DHCP6_IA *)AllocateZeroPool (sizeof (EFI_DHCP6_IA) + sizeof (EFI_DHCP6_IA_ADDRESS) * 2); + ASSERT_NE (Ia, (EFI_DHCP6_IA *)NULL); + + CopyMem (Ia->IaAddress, mAllDhcpRelayAndServersAddress.Addr, sizeof (EFI_IPv6_ADDRESS)); + CopyMem (Ia->IaAddress + 1, mAllDhcpRelayAndServersAddress.Addr, sizeof (EFI_IPv6_ADDRESS)); + + Ia->IaAddressCount = 2; + } + + // Add any cleanup code if needed + virtual void + TearDown ( + ) + { + // Clean up any resources or variables + if (Buffer != NULL) { + FreePool (Buffer); + } + + if (Ia != NULL) { + FreePool (Ia); + } + } +}; + +// Test Description: +// Attempt to append an option to a packet that doesn't have enough space +// for the option header +TEST_F (Dhcp6AppendIaOptionTest, IaNaInvalidDataExpectBufferTooSmall) { + UINT8 *Cursor; + EFI_STATUS Status; + + Packet->Length = Packet->Size - 2; + + Ia->Descriptor.Type = Dhcp6OptIana; + Ia->Descriptor.IaId = 0x12345678; + + Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; + + Status = Dhcp6AppendIaOption ( + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0x12345678, + 0x11111111, + Dhcp6OptIana + ); + + // verify that we error out because the packet is too small for the option header + ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); + + // reset the length + Packet->Length = sizeof (EFI_DHCP6_HEADER); +} + +// Test Description: +// Attempt to append an option to a packet that doesn't have enough space +// for the option header +TEST_F (Dhcp6AppendIaOptionTest, IaTaInvalidDataExpectBufferTooSmall) { + UINT8 *Cursor; + EFI_STATUS Status; + + // Use up nearly all the space in the packet + Packet->Length = Packet->Size - 2; + + Ia->Descriptor.Type = Dhcp6OptIata; + Ia->Descriptor.IaId = 0x12345678; + + Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; + + Status = Dhcp6AppendIaOption ( + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0, + 0, + Dhcp6OptIata + ); + + // verify that we error out because the packet is too small for the option header + ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); + + // reset the length + Packet->Length = sizeof (EFI_DHCP6_HEADER); +} + +TEST_F (Dhcp6AppendIaOptionTest, IaNaValidDataExpectSuccess) { + UINT8 *Cursor; + EFI_STATUS Status; + UINTN ExpectedSize; + UINTN OriginalLength; + + // + // 2 bytes for the option header type + // + ExpectedSize = 2; + // + // 2 bytes for the option header length + // + ExpectedSize += 2; + // + // 4 bytes for the IAID + // + ExpectedSize += 4; + // + // + 4 bytes for the T1 + // + ExpectedSize += 4; + // + // + 4 bytes for the T2 + // + ExpectedSize += 4; + // + // + (4 + sizeof (EFI_DHCP6_IA_ADDRESS)) * 2; + // + 2 bytes for the option header type + // + 2 bytes for the option header length + // + sizeof (EFI_DHCP6_IA_ADDRESS) for the IA Address + // + ExpectedSize += (4 + sizeof (EFI_DHCP6_IA_ADDRESS)) * 2; + + Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; + + Packet->Length = sizeof (EFI_DHCP6_HEADER); + OriginalLength = Packet->Length; + + Ia->Descriptor.Type = Dhcp6OptIana; + Ia->Descriptor.IaId = 0x12345678; + + Status = Dhcp6AppendIaOption ( + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0x12345678, + 0x12345678, + Dhcp6OptIana + ); + + // verify that the pointer to cursor moved by the expected amount + ASSERT_EQ (Cursor, (UINT8 *)Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option + ExpectedSize); + + // verify that the length of the packet is now the expected amount + ASSERT_EQ (Dhcp6AppendIaOptionTest::Packet->Length, OriginalLength + ExpectedSize); + + // verify that the status is EFI_SUCCESS + ASSERT_EQ (Status, EFI_SUCCESS); +} + +TEST_F (Dhcp6AppendIaOptionTest, IaTaValidDataExpectSuccess) { + UINT8 *Cursor; + EFI_STATUS Status; + UINTN ExpectedSize; + UINTN OriginalLength; + + // + // 2 bytes for the option header type + // + ExpectedSize = 2; + // + // 2 bytes for the option header length + // + ExpectedSize += 2; + // + // 4 bytes for the IAID + // + ExpectedSize += 4; + // + // + (4 + sizeof (EFI_DHCP6_IA_ADDRESS)) * 2; + // + 2 bytes for the option header type + // + 2 bytes for the option header length + // + sizeof (EFI_DHCP6_IA_ADDRESS) for the IA Address + // + ExpectedSize += (4 + sizeof (EFI_DHCP6_IA_ADDRESS)) * 2; + + Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; + + Packet->Length = sizeof (EFI_DHCP6_HEADER); + OriginalLength = Packet->Length; + + Ia->Descriptor.Type = Dhcp6OptIata; + Ia->Descriptor.IaId = 0x12345678; + + Status = Dhcp6AppendIaOption ( + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0, + 0, + Dhcp6OptIata + ); + + // verify that the pointer to cursor moved by the expected amount + ASSERT_EQ (Cursor, (UINT8 *)Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option + ExpectedSize); + + // verify that the length of the packet is now the expected amount + ASSERT_EQ (Dhcp6AppendIaOptionTest::Packet->Length, OriginalLength + ExpectedSize); + + // verify that the status is EFI_SUCCESS + ASSERT_EQ (Status, EFI_SUCCESS); +} diff --git a/NetworkPkg/NetworkPkg.ci.yaml b/NetworkPkg/NetworkPkg.ci.yaml index 07dc7abd69..e82e0c6256 100644 --- a/NetworkPkg/NetworkPkg.ci.yaml +++ b/NetworkPkg/NetworkPkg.ci.yaml @@ -24,6 +24,9 @@ "CompilerPlugin": { "DscPath": "NetworkPkg.dsc" }, + "HostUnitTestCompilerPlugin": { + "DscPath": "Test/NetworkPkgHostTest.dsc" + }, "CharEncodingCheck": { "IgnoreFiles": [] }, diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc new file mode 100644 index 0000000000..5befdf7688 --- /dev/null +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -0,0 +1,102 @@ +## @file +# NetworkPkgHostTest DSC file used to build host-based unit tests. +# +# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent +# +## +[Defines] + PLATFORM_NAME = NetworkPkgHostTest + PLATFORM_GUID = 3b68324e-fc07-4d49-9520-9347ede65879 + PLATFORM_VERSION = 0.1 + DSC_SPECIFICATION = 0x00010005 + OUTPUT_DIRECTORY = Build/NetworkPkg/HostTest + SUPPORTED_ARCHITECTURES = IA32|X64|AARCH64 + BUILD_TARGETS = NOOPT + SKUID_IDENTIFIER = DEFAULT + +!include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc +[Packages] + MdePkg/MdePkg.dec + UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec + +[Components] + # + # Build HOST_APPLICATION that tests NetworkPkg + # + NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf + +# Despite these library classes being listed in [LibraryClasses] below, they are not needed for the host-based unit tests. +[LibraryClasses] + NetLib|NetworkPkg/Library/DxeNetLib/DxeNetLib.inf + DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf + BaseLib|MdePkg/Library/BaseLib/BaseLib.inf + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf + DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf + HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf + MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf + PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf + PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf + UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf + UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf + UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf + UefiLib|MdePkg/Library/UefiLib/UefiLib.inf + UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf + UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf + UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf + TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf + PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf + PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf + DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf + DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf + SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf + RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf +!ifdef CONTINUOUS_INTEGRATION + BaseCryptLib|CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf + TlsLib|CryptoPkg/Library/TlsLibNull/TlsLibNull.inf +!else + BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf + OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf + TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf +!endif + DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf + FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf + FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf + SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf + IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf + +!if $(TOOL_CHAIN_TAG) == VS2019 or $(TOOL_CHAIN_TAG) == VS2022 +[LibraryClasses.X64] + # Provide StackCookie support lib so that we can link to /GS exports for VS builds + RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf + NULL|MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.inf +!endif + +[LibraryClasses.common.UEFI_DRIVER] + HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf + ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf + DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf +[LibraryClasses.common.UEFI_APPLICATION] + DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf + ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf +[LibraryClasses.ARM, LibraryClasses.AARCH64] + # + # It is not possible to prevent ARM compiler calls to generic intrinsic functions. + # This library provides the instrinsic functions generated by a given compiler. + # [LibraryClasses.ARM] and NULL mean link this library into all ARM images. + # + # MU_CHANGE Start +!if $(TOOL_CHAIN_TAG) != VS2017 and $(TOOL_CHAIN_TAG) != VS2015 and $(TOOL_CHAIN_TAG) != VS2019 + NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf +!endif + # MU_CHANGE End + NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf +[LibraryClasses.ARM] + RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf +[LibraryClasses.RISCV64] + RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf + +[PcdsFixedAtBuild] + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2 + gEfiNetworkPkgTokenSpaceGuid.PcdDhcp6UidType|0x4 \ No newline at end of file -- 2.41.0 From 9613e15be77676e119291f28ae46cb13bf37f235 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Fri, 15 Dec 2023 11:26:47 -0800 Subject: [PATCH 03/12] SECURITY PATCH TCBZ4534 - CVE-2023-45229 - Patch --- NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h | 134 ++++++++++++++++++--- NetworkPkg/Dhcp6Dxe/Dhcp6Io.c | 202 ++++++++++++++++++++++---------- 2 files changed, 257 insertions(+), 79 deletions(-) diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h b/NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h index b552331767..5247b324ac 100644 --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h @@ -45,6 +45,20 @@ typedef struct _DHCP6_INSTANCE DHCP6_INSTANCE; #define DHCP6_SERVICE_SIGNATURE SIGNATURE_32 ('D', 'H', '6', 'S') #define DHCP6_INSTANCE_SIGNATURE SIGNATURE_32 ('D', 'H', '6', 'I') +#define DHCP6_PACKET_ALL 0 +#define DHCP6_PACKET_STATEFUL 1 +#define DHCP6_PACKET_STATELESS 2 + +#define DHCP6_BASE_PACKET_SIZE 1024 + +#define DHCP6_PORT_CLIENT 546 +#define DHCP6_PORT_SERVER 547 + +#define DHCP_CHECK_MEDIA_WAITING_TIME EFI_TIMER_PERIOD_SECONDS(20) + +#define DHCP6_INSTANCE_FROM_THIS(Instance) CR ((Instance), DHCP6_INSTANCE, Dhcp6, DHCP6_INSTANCE_SIGNATURE) +#define DHCP6_SERVICE_FROM_THIS(Service) CR ((Service), DHCP6_SERVICE, ServiceBinding, DHCP6_SERVICE_SIGNATURE) + // // For more information on DHCP options see RFC 8415, Section 21.1 // @@ -59,8 +73,8 @@ typedef struct _DHCP6_INSTANCE DHCP6_INSTANCE; // | (option-len octets) | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // -#define DHCP6_SIZE_OF_OPT_CODE (sizeof(UINT16)) -#define DHCP6_SIZE_OF_OPT_LEN (sizeof(UINT16)) +#define DHCP6_SIZE_OF_OPT_CODE (sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpCode)) +#define DHCP6_SIZE_OF_OPT_LEN (sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpLen)) // Combined size of Code and Length #define DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN (DHCP6_SIZE_OF_OPT_CODE + \ @@ -72,31 +86,121 @@ STATIC_ASSERT ( ); // Offset to the length is just past the code -#define DHCP6_OPT_LEN_OFFSET(a) (a + DHCP6_SIZE_OF_OPT_CODE) +#define DHCP6_OFFSET_OF_OPT_LEN(a) (a + DHCP6_SIZE_OF_OPT_CODE) STATIC_ASSERT ( - DHCP6_OPT_LEN_OFFSET (0) == 2, + DHCP6_OFFSET_OF_OPT_LEN (0) == 2, "Offset of length is + 2 past start of option" ); -#define DHCP6_OPT_DATA_OFFSET(a) (a + DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN) +#define DHCP6_OFFSET_OF_OPT_DATA(a) (a + DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN) STATIC_ASSERT ( - DHCP6_OPT_DATA_OFFSET(0) == 4, + DHCP6_OFFSET_OF_OPT_DATA(0) == 4, "Offset to option data should be +4 from start of option" ); +// +// Identity Association options (both NA (Non-Temporary) and TA (Temporary Association)) +// are defined in RFC 8415 and are a deriviation of a TLV stucture +// For more information on IA_NA see Section 21.4 +// For more information on IA_TA see Section 21.5 +// +// +// The format of IA_NA and IA_TA option: +// +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | OPTION_IA_NA | option-len | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | IAID (4 octets) | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | T1 (only for IA_NA) | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | T2 (only for IA_NA) | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | | +// . IA_NA-options/IA_TA-options . +// . . +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// +#define DHCP6_SIZE_OF_IAID (sizeof(UINT32)) +#define DHCP6_SIZE_OF_TIME_INTERVAL (sizeof(UINT32)) -#define DHCP6_PACKET_ALL 0 -#define DHCP6_PACKET_STATEFUL 1 -#define DHCP6_PACKET_STATELESS 2 +// Combined size of IAID, T1, and T2 +#define DHCP6_SIZE_OF_COMBINED_IAID_T1_T2 (DHCP6_SIZE_OF_IAID + \ + DHCP6_SIZE_OF_TIME_INTERVAL + \ + DHCP6_SIZE_OF_TIME_INTERVAL) +STATIC_ASSERT ( + DHCP6_SIZE_OF_COMBINED_IAID_T1_T2 == 12, + "Combined size of IAID, T1, T2 must be 12 per RFC 8415" + ); -#define DHCP6_BASE_PACKET_SIZE 1024 +// This is the size of IA_TA without options +#define DHCP6_MIN_SIZE_OF_IA_TA (DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN + \ + DHCP6_SIZE_OF_IAID) +STATIC_ASSERT ( + DHCP6_MIN_SIZE_OF_IA_TA == 8, + "Minimum combined size of IA_TA per RFC 8415" + ); -#define DHCP6_PORT_CLIENT 546 -#define DHCP6_PORT_SERVER 547 +// Offset to a IA_TA inner option +#define DHCP6_OFFSET_OF_IA_TA_INNER_OPT(a) (a + DHCP6_MIN_SIZE_OF_IA_TA) +STATIC_ASSERT ( + DHCP6_OFFSET_OF_IA_TA_INNER_OPT (0) == 8, + "Offset of IA_TA Inner option is + 8 past start of option" + ); -#define DHCP_CHECK_MEDIA_WAITING_TIME EFI_TIMER_PERIOD_SECONDS(20) +// This is the size of IA_NA without options (16) +#define DHCP6_MIN_SIZE_OF_IA_NA DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN + \ + DHCP6_SIZE_OF_COMBINED_IAID_T1_T2 +STATIC_ASSERT ( + DHCP6_MIN_SIZE_OF_IA_NA == 16, + "Minimum combined size of IA_TA per RFC 8415" + ); -#define DHCP6_INSTANCE_FROM_THIS(Instance) CR ((Instance), DHCP6_INSTANCE, Dhcp6, DHCP6_INSTANCE_SIGNATURE) -#define DHCP6_SERVICE_FROM_THIS(Service) CR ((Service), DHCP6_SERVICE, ServiceBinding, DHCP6_SERVICE_SIGNATURE) +#define DHCP6_OFFSET_OF_IA_NA_INNER_OPT(a) (a + DHCP6_MIN_SIZE_OF_IA_NA) +STATIC_ASSERT ( + DHCP6_OFFSET_OF_IA_NA_INNER_OPT (0) == 16, + "Offset of IA_NA Inner option is + 16 past start of option" + ); + +#define DHCP6_OFFSET_OF_IA_NA_T1(a) (a + \ + DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN + \ + DHCP6_SIZE_OF_IAID) +STATIC_ASSERT ( + DHCP6_OFFSET_OF_IA_NA_T1 (0) == 8, + "Offset of IA_NA Inner option is + 8 past start of option" + ); + +#define DHCP6_OFFSET_OF_IA_NA_T2(a) (a + \ + DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN +\ + DHCP6_SIZE_OF_IAID + \ + DHCP6_SIZE_OF_TIME_INTERVAL) +STATIC_ASSERT ( + DHCP6_OFFSET_OF_IA_NA_T2 (0) == 12, + "Offset of IA_NA Inner option is + 12 past start of option" + ); + +// +// For more information see RFC 8415 Section 21.13 +// +// The format of the Status Code Option: +// +// 0 1 2 3 +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | OPTION_STATUS_CODE | option-len | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | status-code | | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | +// . . +// . status-message . +// . . +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// +#define DHCP6_OFFSET_OF_STATUS_CODE(a) (a + DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN) +STATIC_ASSERT ( + DHCP6_OFFSET_OF_STATUS_CODE (0) == 4, + "Offset of status is + 4 past start of option" + ); extern EFI_IPv6_ADDRESS mAllDhcpRelayAndServersAddress; extern EFI_DHCP6_PROTOCOL gDhcp6ProtocolTemplate; diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c index 1401910950..f96b4c7962 100644 --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c @@ -598,8 +598,9 @@ Dhcp6UpdateIaInfo ( // The inner options still start with 2 bytes option-code and 2 bytes option-len. // if (Instance->Config->IaDescriptor.Type == Dhcp6OptIana) { - T1 = NTOHL (ReadUnaligned32 ((UINT32 *)(Option + 8))); - T2 = NTOHL (ReadUnaligned32 ((UINT32 *)(Option + 12))); + + T1 = NTOHL (ReadUnaligned32 ((UINT32 *)(DHCP6_OFFSET_OF_IA_NA_T1(Option)))); + T2 = NTOHL (ReadUnaligned32 ((UINT32 *)(DHCP6_OFFSET_OF_IA_NA_T2(Option)))); // // Refer to RFC3155 Chapter 22.4. If a client receives an IA_NA with T1 greater than T2, // and both T1 and T2 are greater than 0, the client discards the IA_NA option and processes @@ -609,13 +610,14 @@ Dhcp6UpdateIaInfo ( return EFI_DEVICE_ERROR; } - IaInnerOpt = Option + 16; - IaInnerLen = (UINT16)(NTOHS (ReadUnaligned16 ((UINT16 *)(Option + 2))) - 12); + IaInnerOpt = DHCP6_OFFSET_OF_IA_NA_INNER_OPT(Option); + IaInnerLen = (UINT16)(NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN(Option)))) - DHCP6_SIZE_OF_COMBINED_IAID_T1_T2); } else { T1 = 0; T2 = 0; - IaInnerOpt = Option + 8; - IaInnerLen = (UINT16)(NTOHS (ReadUnaligned16 ((UINT16 *)(Option + 2))) - 4); + + IaInnerOpt = DHCP6_OFFSET_OF_IA_TA_INNER_OPT(Option); + IaInnerLen = (UINT16)(NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN(Option)))) - DHCP6_SIZE_OF_IAID); } // @@ -641,7 +643,7 @@ Dhcp6UpdateIaInfo ( Option = Dhcp6SeekOption (IaInnerOpt, IaInnerLen, Dhcp6OptStatusCode); if (Option != NULL) { - StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)(Option + 4))); + StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN(Option)))); if (StsCode != Dhcp6StsSuccess) { return EFI_DEVICE_ERROR; } @@ -661,6 +663,88 @@ Dhcp6UpdateIaInfo ( return Status; } +/* + Seeks the Inner Options from a DHCP6 Option + + @param[in] IaType The type of the IA option. + @param[in] Option The pointer to the DHCP6 Option. + @param[in] OptionLen The length of the DHCP6 Option. + @param[out] IaInnerOpt The pointer to the IA inner option. + @param[out] IaInnerLen The length of the IA inner option. + + @retval EFI_SUCCESS Seek the inner option successfully. + @retval EFI_DEVICE_ERROR The OptionLen is invalid. On Error, + the pointers are not modified +*/ +EFI_STATUS +Dhcp6SeekInnerOptionSafe ( + IN UINT16 IaType, + IN UINT8 *Option, + IN UINT32 OptionLen, + OUT UINT8 **IaInnerOpt, + OUT UINT16 *IaInnerLen + ) +{ + + UINT16 IaInnerLenTmp; + UINT8 *IaInnerOptTmp; + + if (Option == NULL) { + ASSERT (Option != NULL); + return EFI_DEVICE_ERROR; + } + + if (IaInnerOpt == NULL) { + ASSERT (IaInnerOpt != NULL); + return EFI_DEVICE_ERROR; + } + + if (IaInnerLen == NULL) { + ASSERT (IaInnerLen != NULL); + return EFI_DEVICE_ERROR; + } + + if (IaType == Dhcp6OptIana) { + // Verify we have a fully formed IA_NA + if (OptionLen < DHCP6_MIN_SIZE_OF_IA_NA) { + return EFI_DEVICE_ERROR; + } + + // + IaInnerOptTmp = DHCP6_OFFSET_OF_IA_NA_INNER_OPT(Option); + + // Verify the IaInnerLen is valid. + IaInnerLenTmp = (UINT16)NTOHS (ReadUnaligned16 ((UINT16 *)DHCP6_OFFSET_OF_OPT_LEN(Option))); + if (IaInnerLenTmp < DHCP6_SIZE_OF_COMBINED_IAID_T1_T2) { + return EFI_DEVICE_ERROR; + } + + IaInnerLenTmp -= DHCP6_SIZE_OF_COMBINED_IAID_T1_T2; + } else if (IaType == Dhcp6OptIata) { + // Verify the OptionLen is valid. + if (OptionLen < DHCP6_MIN_SIZE_OF_IA_TA) { + return EFI_DEVICE_ERROR; + } + + IaInnerOptTmp = DHCP6_OFFSET_OF_IA_TA_INNER_OPT(Option); + + // Verify the IaInnerLen is valid. + IaInnerLenTmp = (UINT16)NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN(Option)))); + if (IaInnerLenTmp < DHCP6_SIZE_OF_IAID) { + return EFI_DEVICE_ERROR; + } + + IaInnerLenTmp -= DHCP6_SIZE_OF_IAID; + } else { + return EFI_DEVICE_ERROR; + } + + *IaInnerOpt = IaInnerOptTmp; + *IaInnerLen = IaInnerLenTmp; + + return EFI_SUCCESS; +} + /** Seek StatusCode Option in package. A Status Code option may appear in the options field of a DHCP message and/or in the options field of another option. @@ -684,6 +768,12 @@ Dhcp6SeekStsOption ( UINT8 *IaInnerOpt; UINT16 IaInnerLen; UINT16 StsCode; + UINT32 OptionLen; + + // OptionLen is the length of the Options excluding the DHCP header. + // Length of the EFI_DHCP6_PACKET from the first byte of the Header field to the last + // byte of the Option[] field. + OptionLen = Packet->Length - sizeof (Packet->Dhcp6.Header); // // Seek StatusCode option directly in DHCP message body. That is, search in @@ -691,12 +781,12 @@ Dhcp6SeekStsOption ( // *Option = Dhcp6SeekOption ( Packet->Dhcp6.Option, - Packet->Length - 4, + OptionLen, Dhcp6OptStatusCode ); if (*Option != NULL) { - StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)(*Option + 4))); + StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_STATUS_CODE(*Option)))); if (StsCode != Dhcp6StsSuccess) { return EFI_DEVICE_ERROR; } @@ -707,7 +797,7 @@ Dhcp6SeekStsOption ( // *Option = Dhcp6SeekIaOption ( Packet->Dhcp6.Option, - Packet->Length - sizeof (EFI_DHCP6_HEADER), + OptionLen, &Instance->Config->IaDescriptor ); if (*Option == NULL) { @@ -715,52 +805,35 @@ Dhcp6SeekStsOption ( } // - // The format of the IA_NA option is: + // Calculate the distance from Packet->Dhcp6.Option to the IA option. // - // 0 1 2 3 - // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | OPTION_IA_NA | option-len | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | IAID (4 octets) | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | T1 | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | T2 | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | | - // . IA_NA-options . - // . . - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // Packet->Size and Packet->Length are both UINT32 type, and Packet->Size is + // the size of the whole packet, including the DHCP header, and Packet->Length + // is the length of the DHCP message body, excluding the DHCP header. // - // The format of the IA_TA option is: + // (*Option - Packet->Dhcp6.Option) is the number of bytes from the start of + // DHCP6 option area to the start of the IA option. // - // 0 1 2 3 - // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | OPTION_IA_TA | option-len | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | IAID (4 octets) | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | | - // . IA_TA-options . - // . . - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // Dhcp6SeekInnerOptionSafe() is searching starting from the start of the + // IA option to the end of the DHCP6 option area, thus subtract the space + // up until this option // + OptionLen = OptionLen - (*Option - Packet->Dhcp6.Option); // - // sizeof (option-code + option-len + IaId) = 8 - // sizeof (option-code + option-len + IaId + T1) = 12 - // sizeof (option-code + option-len + IaId + T1 + T2) = 16 + // Seek the inner option // - // The inner options still start with 2 bytes option-code and 2 bytes option-len. - // - if (Instance->Config->IaDescriptor.Type == Dhcp6OptIana) { - IaInnerOpt = *Option + 16; - IaInnerLen = (UINT16)(NTOHS (ReadUnaligned16 ((UINT16 *)(*Option + 2))) - 12); - } else { - IaInnerOpt = *Option + 8; - IaInnerLen = (UINT16)(NTOHS (ReadUnaligned16 ((UINT16 *)(*Option + 2))) - 4); + if (EFI_ERROR ( + Dhcp6SeekInnerOptionSafe ( + Instance->Config->IaDescriptor.Type, + *Option, + OptionLen, + &IaInnerOpt, + &IaInnerLen + ) + )) + { + return EFI_DEVICE_ERROR; } // @@ -784,7 +857,7 @@ Dhcp6SeekStsOption ( // *Option = Dhcp6SeekOption (IaInnerOpt, IaInnerLen, Dhcp6OptStatusCode); if (*Option != NULL) { - StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)(*Option + 4))); + StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)((DHCP6_OFFSET_OF_STATUS_CODE(*Option))))); if (StsCode != Dhcp6StsSuccess) { return EFI_DEVICE_ERROR; } @@ -1105,7 +1178,7 @@ Dhcp6SendRequestMsg ( // Option = Dhcp6SeekOption ( Instance->AdSelect->Dhcp6.Option, - Instance->AdSelect->Length - 4, + Instance->AdSelect->Length - sizeof(EFI_DHCP6_HEADER), Dhcp6OptServerId ); if (Option == NULL) { @@ -1289,7 +1362,7 @@ Dhcp6SendDeclineMsg ( // Option = Dhcp6SeekOption ( LastReply->Dhcp6.Option, - LastReply->Length - 4, + LastReply->Length - sizeof(EFI_DHCP6_HEADER), Dhcp6OptServerId ); if (Option == NULL) { @@ -1448,7 +1521,7 @@ Dhcp6SendReleaseMsg ( // Option = Dhcp6SeekOption ( LastReply->Dhcp6.Option, - LastReply->Length - 4, + LastReply->Length - sizeof(EFI_DHCP6_HEADER), Dhcp6OptServerId ); if (Option == NULL) { @@ -1674,7 +1747,7 @@ Dhcp6SendRenewRebindMsg ( Option = Dhcp6SeekOption ( LastReply->Dhcp6.Option, - LastReply->Length - 4, + LastReply->Length - sizeof(EFI_DHCP6_HEADER), Dhcp6OptServerId ); if (Option == NULL) { @@ -2210,7 +2283,7 @@ Dhcp6HandleReplyMsg ( // Option = Dhcp6SeekOption ( Packet->Dhcp6.Option, - Packet->Length - 4, + Packet->Length - sizeof(EFI_DHCP6_HEADER), Dhcp6OptRapidCommit ); @@ -2356,7 +2429,7 @@ Dhcp6HandleReplyMsg ( // // Any error status code option is found. // - StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)(Option + 4))); + StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)((DHCP6_OFFSET_OF_STATUS_CODE(Option))))); switch (StsCode) { case Dhcp6StsUnspecFail: // @@ -2489,7 +2562,7 @@ Dhcp6SelectAdvertiseMsg ( // Option = Dhcp6SeekOption ( AdSelect->Dhcp6.Option, - AdSelect->Length - 4, + AdSelect->Length - sizeof(EFI_DHCP6_HEADER), Dhcp6OptServerUnicast ); @@ -2500,7 +2573,7 @@ Dhcp6SelectAdvertiseMsg ( return EFI_OUT_OF_RESOURCES; } - CopyMem (Instance->Unicast, Option + 4, sizeof (EFI_IPv6_ADDRESS)); + CopyMem (Instance->Unicast, DHCP6_OFFSET_OF_OPT_DATA(Option), sizeof (EFI_IPv6_ADDRESS)); } // @@ -2553,7 +2626,7 @@ Dhcp6HandleAdvertiseMsg ( // Option = Dhcp6SeekOption ( Packet->Dhcp6.Option, - Packet->Length - 4, + Packet->Length - sizeof(EFI_DHCP6_HEADER), Dhcp6OptRapidCommit ); @@ -2647,7 +2720,8 @@ Dhcp6HandleAdvertiseMsg ( CopyMem (Instance->AdSelect, Packet, Packet->Size); if (Option != NULL) { - Instance->AdPref = *(Option + 4); + Instance->AdPref = *(DHCP6_OFFSET_OF_OPT_DATA(Option)); + } } else { // @@ -2716,11 +2790,11 @@ Dhcp6HandleStateful ( // Option = Dhcp6SeekOption ( Packet->Dhcp6.Option, - Packet->Length - 4, + Packet->Length - DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN, Dhcp6OptClientId ); - if ((Option == NULL) || (CompareMem (Option + 4, ClientId->Duid, ClientId->Length) != 0)) { + if ((Option == NULL) || (CompareMem (DHCP6_OFFSET_OF_OPT_DATA(Option), ClientId->Duid, ClientId->Length) != 0)) { goto ON_CONTINUE; } @@ -2729,7 +2803,7 @@ Dhcp6HandleStateful ( // Option = Dhcp6SeekOption ( Packet->Dhcp6.Option, - Packet->Length - 4, + Packet->Length - DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN, Dhcp6OptServerId ); @@ -2834,7 +2908,7 @@ Dhcp6HandleStateless ( // Option = Dhcp6SeekOption ( Packet->Dhcp6.Option, - Packet->Length - 4, + Packet->Length - sizeof(EFI_DHCP6_HEADER), Dhcp6OptServerId ); -- 2.41.0 From ac0130907b8a88cca3c4d8eb590f3b4aa33133d8 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Fri, 15 Dec 2023 13:31:38 -0800 Subject: [PATCH 04/12] SECURITY PATCH TCBZ4534 - CVE-2023-45229 - Host Based Unit Test --- .../GoogleTest/Dhcp6DxeGoogleTest.cpp | 54 +- .../GoogleTest/Dhcp6DxeGoogleTest.inf | 86 +-- .../Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp | 631 ++++++++++++++---- .../Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h | 58 ++ NetworkPkg/Test/NetworkPkgHostTest.dsc | 204 +++--- 5 files changed, 726 insertions(+), 307 deletions(-) create mode 100644 NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp index b1fe72e195..36fd708cfc 100644 --- a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.cpp @@ -1,27 +1,27 @@ -/** @file - Acts as the main entry point for the tests for the Dhcp6Dxe module. - - Copyright (c) Microsoft Corporation - SPDX-License-Identifier: BSD-2-Clause-Patent -**/ -#include - -//////////////////////////////////////////////////////////////////////////////// -// Add test files here -// Google Test will only pick up the tests from the files that are included -// here. -//////////////////////////////////////////////////////////////////////////////// -#include "Dhcp6IoGoogleTest.cpp" - -//////////////////////////////////////////////////////////////////////////////// -// Run the tests -//////////////////////////////////////////////////////////////////////////////// -int -main ( - int argc, - char *argv[] - ) -{ - testing::InitGoogleTest (&argc, argv); - return RUN_ALL_TESTS (); -} +/** @file + Acts as the main entry point for the tests for the Dhcp6Dxe module. + + Copyright (c) Microsoft Corporation + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ +#include + +//////////////////////////////////////////////////////////////////////////////// +// Add test files here +// Google Test will only pick up the tests from the files that are included +// here. +//////////////////////////////////////////////////////////////////////////////// +#include "Dhcp6IoGoogleTest.cpp" + +//////////////////////////////////////////////////////////////////////////////// +// Run the tests +//////////////////////////////////////////////////////////////////////////////// +int +main ( + int argc, + char *argv[] + ) +{ + testing::InitGoogleTest (&argc, argv); + return RUN_ALL_TESTS (); +} diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf index c7ec42b322..b74497b6b3 100644 --- a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf @@ -1,44 +1,44 @@ -## @file -# Unit test suite for the Dhcp6Dxe using Google Test -# -# Copyright (c) Microsoft Corporation.
-# SPDX-License-Identifier: BSD-2-Clause-Patent -## -[Defines] - INF_VERSION = 0x00010017 - BASE_NAME = Dhcp6DxeGoogleTest - FILE_GUID = 1D2A4C65-38C8-4C2F-BB60-B5FA49625AA9 - 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] - Dhcp6DxeGoogleTest.cpp - Dhcp6IoGoogleTest.cpp - ../Dhcp6Io.c - ../Dhcp6Utility.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] +## @file +# Unit test suite for the Dhcp6Dxe using Google Test +# +# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent +## +[Defines] + INF_VERSION = 0x00010017 + BASE_NAME = Dhcp6DxeGoogleTest + FILE_GUID = 1D2A4C65-38C8-4C2F-BB60-B5FA49625AA9 + 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] + Dhcp6DxeGoogleTest.cpp + Dhcp6IoGoogleTest.cpp + ../Dhcp6Io.c + ../Dhcp6Utility.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] gZeroGuid \ No newline at end of file diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp index dad6a42b12..31e848543d 100644 --- a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.cpp @@ -7,12 +7,13 @@ #include extern "C" { - #include - #include - #include - #include - #include "../Dhcp6Impl.h" - #include "../Dhcp6Utility.h" +#include +#include +#include +#include +#include "../Dhcp6Impl.h" +#include "../Dhcp6Utility.h" +#include "Dhcp6IoGoogleTest.h" } //////////////////////////////////////////////////////////////////////// @@ -21,7 +22,35 @@ extern "C" { #define DHCP6_PACKET_MAX_LEN 1500 +// This definition is used by this test but is also required to compile +// by Dhcp6Io.c +#define DHCPV6_OPTION_IA_NA 3 +#define DHCPV6_OPTION_IA_TA 4 + +#define SEARCH_PATTERN 0xDEADC0DE +#define SEARCH_PATTERN_LEN sizeof(SEARCH_PATTERN) + //////////////////////////////////////////////////////////////////////// +// Test structures for IA_NA and IA_TA options +//////////////////////////////////////////////////////////////////////// +typedef struct { + UINT16 Code; + UINT16 Len; + UINT32 IAID; +} DHCPv6_OPTION; + +typedef struct { + DHCPv6_OPTION Header; + UINT32 T1; + UINT32 T2; + UINT8 InnerOptions[0]; +} DHCPv6_OPTION_IA_NA; + +typedef struct { + DHCPv6_OPTION Header; + UINT8 InnerOptions[0]; +} DHCPv6_OPTION_IA_TA; + //////////////////////////////////////////////////////////////////////// // Symbol Definitions // These functions are not directly under test - but required to compile @@ -65,33 +94,33 @@ UdpIoRecvDatagram ( class Dhcp6AppendOptionTest : public ::testing::Test { public: - UINT8 *Buffer = NULL; - EFI_DHCP6_PACKET *Packet; +UINT8 *Buffer = NULL; +EFI_DHCP6_PACKET *Packet; protected: - // Add any setup code if needed - virtual void - SetUp ( - ) - { - // Initialize any resources or variables - Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); - ASSERT_NE (Buffer, (UINT8 *)NULL); - - Packet = (EFI_DHCP6_PACKET *)Buffer; - Packet->Size = DHCP6_PACKET_MAX_LEN; - } +// Add any setup code if needed +virtual void +SetUp ( + ) +{ + // Initialize any resources or variables + Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); + ASSERT_NE (Buffer, (UINT8 *)NULL); + + Packet = (EFI_DHCP6_PACKET *)Buffer; + Packet->Size = DHCP6_PACKET_MAX_LEN; +} - // Add any cleanup code if needed - virtual void - TearDown ( - ) - { - // Clean up any resources or variables - if (Buffer != NULL) { - FreePool (Buffer); - } +// Add any cleanup code if needed +virtual void +TearDown ( + ) +{ + // Clean up any resources or variables + if (Buffer != NULL) { + FreePool (Buffer); } +} }; // Test Description: @@ -109,12 +138,12 @@ TEST_F (Dhcp6AppendOptionTest, InvalidDataExpectBufferTooSmall) { Cursor = Dhcp6AppendOptionTest::Packet->Dhcp6.Option; Status = Dhcp6AppendOption ( - Dhcp6AppendOptionTest::Packet, - &Cursor, - HTONS (Dhcp6OptServerId), - UntrustedDuid->Length, - UntrustedDuid->Duid - ); + Dhcp6AppendOptionTest::Packet, + &Cursor, + HTONS (Dhcp6OptServerId), + UntrustedDuid->Length, + UntrustedDuid->Duid + ); ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); } @@ -141,12 +170,12 @@ TEST_F (Dhcp6AppendOptionTest, ValidDataExpectSuccess) { Cursor = Dhcp6AppendOptionTest::Packet->Dhcp6.Option; Status = Dhcp6AppendOption ( - Dhcp6AppendOptionTest::Packet, - &Cursor, - HTONS (Dhcp6OptServerId), - UntrustedDuid->Length, - UntrustedDuid->Duid - ); + Dhcp6AppendOptionTest::Packet, + &Cursor, + HTONS (Dhcp6OptServerId), + UntrustedDuid->Length, + UntrustedDuid->Duid + ); ASSERT_EQ (Status, EFI_SUCCESS); @@ -163,34 +192,34 @@ TEST_F (Dhcp6AppendOptionTest, ValidDataExpectSuccess) { class Dhcp6AppendETOptionTest : public ::testing::Test { public: - UINT8 *Buffer = NULL; - EFI_DHCP6_PACKET *Packet; +UINT8 *Buffer = NULL; +EFI_DHCP6_PACKET *Packet; protected: - // Add any setup code if needed - virtual void - SetUp ( - ) - { - // Initialize any resources or variables - Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); - ASSERT_NE (Buffer, (UINT8 *)NULL); - - Packet = (EFI_DHCP6_PACKET *)Buffer; - Packet->Size = DHCP6_PACKET_MAX_LEN; - Packet->Length = sizeof (EFI_DHCP6_HEADER); - } +// Add any setup code if needed +virtual void +SetUp ( + ) +{ + // Initialize any resources or variables + Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); + ASSERT_NE (Buffer, (UINT8 *)NULL); + + Packet = (EFI_DHCP6_PACKET *)Buffer; + Packet->Size = DHCP6_PACKET_MAX_LEN; + Packet->Length = sizeof (EFI_DHCP6_HEADER); +} - // Add any cleanup code if needed - virtual void - TearDown ( - ) - { - // Clean up any resources or variables - if (Buffer != NULL) { - FreePool (Buffer); - } +// Add any cleanup code if needed +virtual void +TearDown ( + ) +{ + // Clean up any resources or variables + if (Buffer != NULL) { + FreePool (Buffer); } +} }; // Test Description: @@ -208,11 +237,11 @@ TEST_F (Dhcp6AppendETOptionTest, InvalidDataExpectBufferTooSmall) { Packet->Length = Packet->Size - 2; Status = Dhcp6AppendETOption ( - Dhcp6AppendETOptionTest::Packet, - &Cursor, - &Instance, // Instance is not used in this function - &ElapsedTime - ); + Dhcp6AppendETOptionTest::Packet, + &Cursor, + &Instance, // Instance is not used in this function + &ElapsedTime + ); // verify that we error out because the packet is too small for the option header ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); @@ -238,11 +267,11 @@ TEST_F (Dhcp6AppendETOptionTest, ValidDataExpectSuccess) { OriginalLength = Packet->Length; Status = Dhcp6AppendETOption ( - Dhcp6AppendETOptionTest::Packet, - &Cursor, - &Instance, // Instance is not used in this function - &ElapsedTime - ); + Dhcp6AppendETOptionTest::Packet, + &Cursor, + &Instance, // Instance is not used in this function + &ElapsedTime + ); // verify that the status is EFI_SUCCESS ASSERT_EQ (Status, EFI_SUCCESS); @@ -260,46 +289,46 @@ TEST_F (Dhcp6AppendETOptionTest, ValidDataExpectSuccess) { class Dhcp6AppendIaOptionTest : public ::testing::Test { public: - UINT8 *Buffer = NULL; - EFI_DHCP6_PACKET *Packet; - EFI_DHCP6_IA *Ia; +UINT8 *Buffer = NULL; +EFI_DHCP6_PACKET *Packet; +EFI_DHCP6_IA *Ia; protected: - // Add any setup code if needed - virtual void - SetUp ( - ) - { - // Initialize any resources or variables - Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); - ASSERT_NE (Buffer, (UINT8 *)NULL); +// Add any setup code if needed +virtual void +SetUp ( + ) +{ + // Initialize any resources or variables + Buffer = (UINT8 *)AllocateZeroPool (DHCP6_PACKET_MAX_LEN); + ASSERT_NE (Buffer, (UINT8 *)NULL); + + Packet = (EFI_DHCP6_PACKET *)Buffer; + Packet->Size = DHCP6_PACKET_MAX_LEN; - Packet = (EFI_DHCP6_PACKET *)Buffer; - Packet->Size = DHCP6_PACKET_MAX_LEN; + Ia = (EFI_DHCP6_IA *)AllocateZeroPool (sizeof (EFI_DHCP6_IA) + sizeof (EFI_DHCP6_IA_ADDRESS) * 2); + ASSERT_NE (Ia, (EFI_DHCP6_IA *)NULL); - Ia = (EFI_DHCP6_IA *)AllocateZeroPool (sizeof (EFI_DHCP6_IA) + sizeof (EFI_DHCP6_IA_ADDRESS) * 2); - ASSERT_NE (Ia, (EFI_DHCP6_IA *)NULL); + CopyMem (Ia->IaAddress, mAllDhcpRelayAndServersAddress.Addr, sizeof (EFI_IPv6_ADDRESS)); + CopyMem (Ia->IaAddress + 1, mAllDhcpRelayAndServersAddress.Addr, sizeof (EFI_IPv6_ADDRESS)); - CopyMem (Ia->IaAddress, mAllDhcpRelayAndServersAddress.Addr, sizeof (EFI_IPv6_ADDRESS)); - CopyMem (Ia->IaAddress + 1, mAllDhcpRelayAndServersAddress.Addr, sizeof (EFI_IPv6_ADDRESS)); + Ia->IaAddressCount = 2; +} - Ia->IaAddressCount = 2; +// Add any cleanup code if needed +virtual void +TearDown ( + ) +{ + // Clean up any resources or variables + if (Buffer != NULL) { + FreePool (Buffer); } - // Add any cleanup code if needed - virtual void - TearDown ( - ) - { - // Clean up any resources or variables - if (Buffer != NULL) { - FreePool (Buffer); - } - - if (Ia != NULL) { - FreePool (Ia); - } + if (Ia != NULL) { + FreePool (Ia); } +} }; // Test Description: @@ -317,13 +346,13 @@ TEST_F (Dhcp6AppendIaOptionTest, IaNaInvalidDataExpectBufferTooSmall) { Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; Status = Dhcp6AppendIaOption ( - Dhcp6AppendIaOptionTest::Packet, - &Cursor, - Ia, - 0x12345678, - 0x11111111, - Dhcp6OptIana - ); + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0x12345678, + 0x11111111, + Dhcp6OptIana + ); // verify that we error out because the packet is too small for the option header ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); @@ -348,13 +377,13 @@ TEST_F (Dhcp6AppendIaOptionTest, IaTaInvalidDataExpectBufferTooSmall) { Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; Status = Dhcp6AppendIaOption ( - Dhcp6AppendIaOptionTest::Packet, - &Cursor, - Ia, - 0, - 0, - Dhcp6OptIata - ); + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0, + 0, + Dhcp6OptIata + ); // verify that we error out because the packet is too small for the option header ASSERT_EQ (Status, EFI_BUFFER_TOO_SMALL); @@ -406,13 +435,13 @@ TEST_F (Dhcp6AppendIaOptionTest, IaNaValidDataExpectSuccess) { Ia->Descriptor.IaId = 0x12345678; Status = Dhcp6AppendIaOption ( - Dhcp6AppendIaOptionTest::Packet, - &Cursor, - Ia, - 0x12345678, - 0x12345678, - Dhcp6OptIana - ); + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0x12345678, + 0x12345678, + Dhcp6OptIana + ); // verify that the pointer to cursor moved by the expected amount ASSERT_EQ (Cursor, (UINT8 *)Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option + ExpectedSize); @@ -450,7 +479,7 @@ TEST_F (Dhcp6AppendIaOptionTest, IaTaValidDataExpectSuccess) { // ExpectedSize += (4 + sizeof (EFI_DHCP6_IA_ADDRESS)) * 2; - Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; + Cursor = Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option; Packet->Length = sizeof (EFI_DHCP6_HEADER); OriginalLength = Packet->Length; @@ -459,13 +488,13 @@ TEST_F (Dhcp6AppendIaOptionTest, IaTaValidDataExpectSuccess) { Ia->Descriptor.IaId = 0x12345678; Status = Dhcp6AppendIaOption ( - Dhcp6AppendIaOptionTest::Packet, - &Cursor, - Ia, - 0, - 0, - Dhcp6OptIata - ); + Dhcp6AppendIaOptionTest::Packet, + &Cursor, + Ia, + 0, + 0, + Dhcp6OptIata + ); // verify that the pointer to cursor moved by the expected amount ASSERT_EQ (Cursor, (UINT8 *)Dhcp6AppendIaOptionTest::Packet->Dhcp6.Option + ExpectedSize); @@ -476,3 +505,335 @@ TEST_F (Dhcp6AppendIaOptionTest, IaTaValidDataExpectSuccess) { // verify that the status is EFI_SUCCESS ASSERT_EQ (Status, EFI_SUCCESS); } + +//////////////////////////////////////////////////////////////////////// +// Dhcp6SeekInnerOptionSafe Tests +//////////////////////////////////////////////////////////////////////// + +// Define a fixture for your tests if needed +class Dhcp6SeekInnerOptionSafeTest : 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: +// This test verifies that Dhcp6SeekInnerOptionSafe returns EFI_SUCCESS when the IANA option is found. +TEST_F (Dhcp6SeekInnerOptionSafeTest, IANAValidOptionExpectSuccess) { + EFI_STATUS Result; + UINT8 Option[sizeof (DHCPv6_OPTION_IA_NA) + SEARCH_PATTERN_LEN] = { 0 }; + UINT32 OptionLength = sizeof (Option); + DHCPv6_OPTION_IA_NA *OptionPtr = (DHCPv6_OPTION_IA_NA *)Option; + UINT32 SearchPattern = SEARCH_PATTERN; + + UINTN SearchPatternLength = SEARCH_PATTERN_LEN; + UINT8 *InnerOptionPtr = NULL; + UINT16 InnerOptionLength = 0; + + OptionPtr->Header.Code = Dhcp6OptIana; + OptionPtr->Header.Len = HTONS (4 + 12); // Valid length has to be more than 12 + OptionPtr->Header.IAID = 0x12345678; + OptionPtr->T1 = 0x11111111; + OptionPtr->T2 = 0x22222222; + CopyMem (OptionPtr->InnerOptions, &SearchPattern, SearchPatternLength); + + Result = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIana, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Result, EFI_SUCCESS); + ASSERT_EQ (InnerOptionLength, 4); + ASSERT_EQ (CompareMem (InnerOptionPtr, &SearchPattern, SearchPatternLength), 0); +} + +// Test Description: +// This test verifies that Dhcp6SeekInnerOptionSafe returns EFI_DEIVCE_ERROR when the IANA option size is invalid. +TEST_F (Dhcp6SeekInnerOptionSafeTest, IANAInvalidSizeExpectFail) { + // Lets add an inner option of bytes we expect to find + EFI_STATUS Status; + UINT8 Option[sizeof (DHCPv6_OPTION_IA_NA) + SEARCH_PATTERN_LEN] = { 0 }; + UINT32 OptionLength = sizeof (Option); + DHCPv6_OPTION_IA_NA *OptionPtr = (DHCPv6_OPTION_IA_NA *)Option; + UINT32 SearchPattern = SEARCH_PATTERN; + + UINTN SearchPatternLength = SEARCH_PATTERN_LEN; + UINT8 *InnerOptionPtr = NULL; + UINT16 InnerOptionLength = 0; + + OptionPtr->Header.Code = Dhcp6OptIana; + OptionPtr->Header.Len = HTONS (4); // Set the length to lower than expected (12) + OptionPtr->Header.IAID = 0x12345678; + OptionPtr->T1 = 0x11111111; + OptionPtr->T2 = 0x22222222; + CopyMem (OptionPtr->InnerOptions, &SearchPattern, SearchPatternLength); + + // Set the InnerOptionLength to be less than the size of the option + Status = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIana, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Status, EFI_DEVICE_ERROR); + + // Now set the OptionLength to be less than the size of the option + OptionLength = sizeof (DHCPv6_OPTION_IA_NA) - 1; + Status = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIana, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Status, EFI_DEVICE_ERROR); +} + +// Test Description: +// This test verifies that Dhcp6SeekInnerOptionSafe returns EFI_SUCCESS when the IATA option is found +TEST_F (Dhcp6SeekInnerOptionSafeTest, IATAValidOptionExpectSuccess) { + // Lets add an inner option of bytes we expect to find + EFI_STATUS Status; + UINT8 Option[sizeof (DHCPv6_OPTION_IA_TA) + SEARCH_PATTERN_LEN] = { 0 }; + UINT32 OptionLength = sizeof (Option); + DHCPv6_OPTION_IA_TA *OptionPtr = (DHCPv6_OPTION_IA_TA *)Option; + UINT32 SearchPattern = SEARCH_PATTERN; + + UINTN SearchPatternLength = SEARCH_PATTERN_LEN; + UINT8 *InnerOptionPtr = NULL; + UINT16 InnerOptionLength = 0; + + OptionPtr->Header.Code = Dhcp6OptIata; + OptionPtr->Header.Len = HTONS (4 + 4); // Valid length has to be more than 4 + OptionPtr->Header.IAID = 0x12345678; + CopyMem (OptionPtr->InnerOptions, &SearchPattern, SearchPatternLength); + + Status = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIata, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Status, EFI_SUCCESS); + ASSERT_EQ (InnerOptionLength, 4); + ASSERT_EQ (CompareMem (InnerOptionPtr, &SearchPattern, SearchPatternLength), 0); +} + +// Test Description: +// This test verifies that Dhcp6SeekInnerOptionSafe returns EFI_SUCCESS when the IATA option size is invalid. +TEST_F (Dhcp6SeekInnerOptionSafeTest, IATAInvalidSizeExpectFail) { + // Lets add an inner option of bytes we expect to find + EFI_STATUS Status; + UINT8 Option[sizeof (DHCPv6_OPTION_IA_TA) + SEARCH_PATTERN_LEN] = { 0 }; + UINT32 OptionLength = sizeof (Option); + DHCPv6_OPTION_IA_TA *OptionPtr = (DHCPv6_OPTION_IA_TA *)Option; + UINT32 SearchPattern = SEARCH_PATTERN; + + UINTN SearchPatternLength = SEARCH_PATTERN_LEN; + UINT8 *InnerOptionPtr = NULL; + UINT16 InnerOptionLength = 0; + + OptionPtr->Header.Code = Dhcp6OptIata; + OptionPtr->Header.Len = HTONS (2); // Set the length to lower than expected (4) + OptionPtr->Header.IAID = 0x12345678; + CopyMem (OptionPtr->InnerOptions, &SearchPattern, SearchPatternLength); + + Status = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIata, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Status, EFI_DEVICE_ERROR); + + // Now lets try modifying the OptionLength to be less than the size of the option + OptionLength = sizeof (DHCPv6_OPTION_IA_TA) - 1; + Status = Dhcp6SeekInnerOptionSafe ( + Dhcp6OptIata, + Option, + OptionLength, + &InnerOptionPtr, + &InnerOptionLength + ); + ASSERT_EQ (Status, EFI_DEVICE_ERROR); +} + +// Test Description: +// This test verifies that any other Option Type fails +TEST_F (Dhcp6SeekInnerOptionSafeTest, InvalidOption) { + // Lets add an inner option of bytes we expect to find + EFI_STATUS Result; + UINT8 Option[sizeof (DHCPv6_OPTION_IA_TA) + SEARCH_PATTERN_LEN] = { 0 }; + UINT32 OptionLength = sizeof (Option); + DHCPv6_OPTION_IA_TA *OptionPtr = (DHCPv6_OPTION_IA_TA *)Option; + UINT32 SearchPattern = SEARCH_PATTERN; + + UINTN SearchPatternLength = SEARCH_PATTERN_LEN; + UINT8 *InnerOptionPtr = NULL; + UINT16 InnerOptionLength = 0; + + OptionPtr->Header.Code = 0xC0DE; + OptionPtr->Header.Len = HTONS (2); // Set the length to lower than expected (4) + OptionPtr->Header.IAID = 0x12345678; + CopyMem (OptionPtr->InnerOptions, &SearchPattern, SearchPatternLength); + + Result = Dhcp6SeekInnerOptionSafe (0xC0DE, Option, OptionLength, &InnerOptionPtr, &InnerOptionLength); + ASSERT_EQ (Result, EFI_DEVICE_ERROR); +} + +//////////////////////////////////////////////////////////////////////// +// Dhcp6SeekStsOption Tests +//////////////////////////////////////////////////////////////////////// + +#define PACKET_SIZE (1500) + +class Dhcp6SeekStsOptionTest : public ::testing::Test { +public: +DHCP6_INSTANCE Instance = { 0 }; +EFI_DHCP6_PACKET *Packet = NULL; +EFI_DHCP6_CONFIG_DATA Config = { 0 }; + +protected: +// Add any setup code if needed +virtual void +SetUp ( + ) +{ + // Allocate a packet + Packet = (EFI_DHCP6_PACKET *)AllocateZeroPool (PACKET_SIZE); + ASSERT_NE (Packet, nullptr); + + // Initialize the packet + Packet->Size = PACKET_SIZE; + + Instance.Config = &Config; +} + +// Add any cleanup code if needed +virtual void +TearDown ( + ) +{ + // Clean up any resources or variables + FreePool (Packet); +} +}; + +// Test Description: +// This test verifies that Dhcp6SeekStsOption returns EFI_DEVICE_ERROR when the option is invalid +// This verifies that the calling function is working as expected +TEST_F (Dhcp6SeekStsOptionTest, SeekIATAOptionExpectFail) { + EFI_STATUS Status; + UINT8 *Option = NULL; + UINT32 SearchPattern = SEARCH_PATTERN; + UINT16 SearchPatternLength = SEARCH_PATTERN_LEN; + UINT16 *Len = NULL; + EFI_DHCP6_IA Ia = { 0 }; + + Ia.Descriptor.Type = DHCPV6_OPTION_IA_TA; + Ia.IaAddressCount = 1; + Ia.IaAddress[0].PreferredLifetime = 0xDEADBEEF; + Ia.IaAddress[0].ValidLifetime = 0xDEADAAAA; + Ia.IaAddress[0].IpAddress = mAllDhcpRelayAndServersAddress; + + Packet->Length = sizeof (EFI_DHCP6_HEADER); + + Option = Dhcp6SeekStsOptionTest::Packet->Dhcp6.Option; + + // Let's append the option to the packet + Status = Dhcp6AppendOption ( + Dhcp6SeekStsOptionTest::Packet, + &Option, + Dhcp6OptStatusCode, + SearchPatternLength, + (UINT8 *)&SearchPattern + ); + ASSERT_EQ (Status, EFI_SUCCESS); + + // Inner option length - this will be overwritten later + Len = (UINT16 *)(Option + 2); + + // Fill in the inner IA option + Status = Dhcp6AppendIaOption ( + Dhcp6SeekStsOptionTest::Packet, + &Option, + &Ia, + 0x12345678, + 0x11111111, + 0x22222222 + ); + ASSERT_EQ (Status, EFI_SUCCESS); + + // overwrite the len of inner Ia option + *Len = HTONS (3); + + Dhcp6SeekStsOptionTest::Instance.Config->IaDescriptor.Type = DHCPV6_OPTION_IA_TA; + + Option = NULL; + Status = Dhcp6SeekStsOption (&(Dhcp6SeekStsOptionTest::Instance), Dhcp6SeekStsOptionTest::Packet, &Option); + + ASSERT_EQ (Status, EFI_DEVICE_ERROR); +} + +// Test Description: +// This test verifies that Dhcp6SeekInnerOptionSafe returns EFI_SUCCESS when the IATA option size is invalid. +TEST_F (Dhcp6SeekStsOptionTest, SeekIANAOptionExpectSuccess) { + EFI_STATUS Status = EFI_NOT_FOUND; + UINT8 *Option = NULL; + UINT32 SearchPattern = SEARCH_PATTERN; + UINT16 SearchPatternLength = SEARCH_PATTERN_LEN; + EFI_DHCP6_IA Ia = { 0 }; + + Ia.Descriptor.Type = DHCPV6_OPTION_IA_NA; + Ia.IaAddressCount = 1; + Ia.IaAddress[0].PreferredLifetime = 0x11111111; + Ia.IaAddress[0].ValidLifetime = 0x22222222; + Ia.IaAddress[0].IpAddress = mAllDhcpRelayAndServersAddress; + Packet->Length = sizeof (EFI_DHCP6_HEADER); + + Option = Dhcp6SeekStsOptionTest::Packet->Dhcp6.Option; + + Status = Dhcp6AppendOption ( + Dhcp6SeekStsOptionTest::Packet, + &Option, + Dhcp6OptStatusCode, + SearchPatternLength, + (UINT8 *)&SearchPattern + ); + ASSERT_EQ (Status, EFI_SUCCESS); + + Status = Dhcp6AppendIaOption ( + Dhcp6SeekStsOptionTest::Packet, + &Option, + &Ia, + 0x12345678, + 0x11111111, + 0x22222222 + ); + ASSERT_EQ (Status, EFI_SUCCESS); + + Dhcp6SeekStsOptionTest::Instance.Config->IaDescriptor.Type = DHCPV6_OPTION_IA_NA; + + Option = NULL; + Status = Dhcp6SeekStsOption (&(Dhcp6SeekStsOptionTest::Instance), Dhcp6SeekStsOptionTest::Packet, &Option); + + ASSERT_EQ (Status, EFI_SUCCESS); +} diff --git a/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h new file mode 100644 index 0000000000..c5e38daf9c --- /dev/null +++ b/NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6IoGoogleTest.h @@ -0,0 +1,58 @@ +/** @file + Acts as header for private functions under test in Dhcp6Io.c + + Copyright (c) Microsoft Corporation + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ + +#ifndef DHCP6_IO_GOOGLE_TEST_H +#define DHCP6_IO_GOOGLE_TEST_H + +//////////////////////////////////////////////////////////////////////////////// +// These are the functions that are being unit tested +//////////////////////////////////////////////////////////////////////////////// + +#include + +/** + Seeks the Inner Options from a DHCP6 Option + + @param[in] IaType The type of the IA option. + @param[in] Option The pointer to the DHCP6 Option. + @param[in] OptionLen The length of the DHCP6 Option. + @param[out] IaInnerOpt The pointer to the IA inner option. + @param[out] IaInnerLen The length of the IA inner option. + + @retval EFI_SUCCESS Seek the inner option successfully. + @retval EFI_DEVICE_ERROR The OptionLen is invalid. +*/ +EFI_STATUS +Dhcp6SeekInnerOptionSafe ( + UINT16 IaType, + UINT8 *Option, + UINT32 OptionLen, + UINT8 **IaInnerOpt, + UINT16 *IaInnerLen + ); + +/** + Seek StatusCode Option in package. A Status Code option may appear in the + options field of a DHCP message and/or in the options field of another option. + See details in section 22.13, RFC3315. + + @param[in] Instance The pointer to the Dhcp6 instance. + @param[in] Packet The pointer to reply messages. + @param[out] Option The pointer to status code option. + + @retval EFI_SUCCESS Seek status code option successfully. + @retval EFI_DEVICE_ERROR An unexpected error. + +**/ +EFI_STATUS +Dhcp6SeekStsOption ( + IN DHCP6_INSTANCE *Instance, + IN EFI_DHCP6_PACKET *Packet, + OUT UINT8 **Option + ); + +#endif // DHCP6_IO_GOOGLE_TEST_H diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc index 5befdf7688..f6459b124f 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -1,102 +1,102 @@ -## @file -# NetworkPkgHostTest DSC file used to build host-based unit tests. -# -# Copyright (c) Microsoft Corporation.
-# SPDX-License-Identifier: BSD-2-Clause-Patent -# -## -[Defines] - PLATFORM_NAME = NetworkPkgHostTest - PLATFORM_GUID = 3b68324e-fc07-4d49-9520-9347ede65879 - PLATFORM_VERSION = 0.1 - DSC_SPECIFICATION = 0x00010005 - OUTPUT_DIRECTORY = Build/NetworkPkg/HostTest - SUPPORTED_ARCHITECTURES = IA32|X64|AARCH64 - BUILD_TARGETS = NOOPT - SKUID_IDENTIFIER = DEFAULT - -!include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc -[Packages] - MdePkg/MdePkg.dec - UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec - -[Components] - # - # Build HOST_APPLICATION that tests NetworkPkg - # - NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf - -# Despite these library classes being listed in [LibraryClasses] below, they are not needed for the host-based unit tests. -[LibraryClasses] - NetLib|NetworkPkg/Library/DxeNetLib/DxeNetLib.inf - DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf - BaseLib|MdePkg/Library/BaseLib/BaseLib.inf - BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf - DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf - HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf - MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf - PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf - PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf - UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf - UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf - UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf - UefiLib|MdePkg/Library/UefiLib/UefiLib.inf - UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf - UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf - UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf - TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf - PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf - PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf - DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf - DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf - SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf - RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf - VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf -!ifdef CONTINUOUS_INTEGRATION - BaseCryptLib|CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf - TlsLib|CryptoPkg/Library/TlsLibNull/TlsLibNull.inf -!else - BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf - OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf - TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf -!endif - DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf - FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf - FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf - SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf - IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf - -!if $(TOOL_CHAIN_TAG) == VS2019 or $(TOOL_CHAIN_TAG) == VS2022 -[LibraryClasses.X64] - # Provide StackCookie support lib so that we can link to /GS exports for VS builds - RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf - NULL|MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.inf -!endif - -[LibraryClasses.common.UEFI_DRIVER] - HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf - ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf - DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf -[LibraryClasses.common.UEFI_APPLICATION] - DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf - ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf -[LibraryClasses.ARM, LibraryClasses.AARCH64] - # - # It is not possible to prevent ARM compiler calls to generic intrinsic functions. - # This library provides the instrinsic functions generated by a given compiler. - # [LibraryClasses.ARM] and NULL mean link this library into all ARM images. - # - # MU_CHANGE Start -!if $(TOOL_CHAIN_TAG) != VS2017 and $(TOOL_CHAIN_TAG) != VS2015 and $(TOOL_CHAIN_TAG) != VS2019 - NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf -!endif - # MU_CHANGE End - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf -[LibraryClasses.ARM] - RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf -[LibraryClasses.RISCV64] - RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf - -[PcdsFixedAtBuild] - gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2 - gEfiNetworkPkgTokenSpaceGuid.PcdDhcp6UidType|0x4 \ No newline at end of file +## @file +# NetworkPkgHostTest DSC file used to build host-based unit tests. +# +# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent +# +## +[Defines] + PLATFORM_NAME = NetworkPkgHostTest + PLATFORM_GUID = 3b68324e-fc07-4d49-9520-9347ede65879 + PLATFORM_VERSION = 0.1 + DSC_SPECIFICATION = 0x00010005 + OUTPUT_DIRECTORY = Build/NetworkPkg/HostTest + SUPPORTED_ARCHITECTURES = IA32|X64|AARCH64 + BUILD_TARGETS = NOOPT + SKUID_IDENTIFIER = DEFAULT + +!include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc +[Packages] + MdePkg/MdePkg.dec + UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec + +[Components] + # + # Build HOST_APPLICATION that tests NetworkPkg + # + NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf + +# Despite these library classes being listed in [LibraryClasses] below, they are not needed for the host-based unit tests. +[LibraryClasses] + NetLib|NetworkPkg/Library/DxeNetLib/DxeNetLib.inf + DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf + BaseLib|MdePkg/Library/BaseLib/BaseLib.inf + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf + DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf + HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf + MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf + PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf + PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf + UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf + UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf + UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf + UefiLib|MdePkg/Library/UefiLib/UefiLib.inf + UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf + UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf + UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf + TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf + PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf + PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf + DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf + DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf + SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf + RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf +!ifdef CONTINUOUS_INTEGRATION + BaseCryptLib|CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf + TlsLib|CryptoPkg/Library/TlsLibNull/TlsLibNull.inf +!else + BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf + OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf + TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf +!endif + DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf + FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf + FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf + SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf + IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf + +!if $(TOOL_CHAIN_TAG) == VS2019 or $(TOOL_CHAIN_TAG) == VS2022 +[LibraryClasses.X64] + # Provide StackCookie support lib so that we can link to /GS exports for VS builds + RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf + NULL|MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.inf +!endif + +[LibraryClasses.common.UEFI_DRIVER] + HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf + ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf + DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf +[LibraryClasses.common.UEFI_APPLICATION] + DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf + ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf +[LibraryClasses.ARM, LibraryClasses.AARCH64] + # + # It is not possible to prevent ARM compiler calls to generic intrinsic functions. + # This library provides the instrinsic functions generated by a given compiler. + # [LibraryClasses.ARM] and NULL mean link this library into all ARM images. + # + # MU_CHANGE Start +!if $(TOOL_CHAIN_TAG) != VS2017 and $(TOOL_CHAIN_TAG) != VS2015 and $(TOOL_CHAIN_TAG) != VS2019 + NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf +!endif + # MU_CHANGE End + NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf +[LibraryClasses.ARM] + RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf +[LibraryClasses.RISCV64] + RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf + +[PcdsFixedAtBuild] + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2 + gEfiNetworkPkgTokenSpaceGuid.PcdDhcp6UidType|0x4 -- 2.41.0 From d74c1be05f439ab3b94ac50f8b46cece775c8ee7 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Fri, 15 Dec 2023 13:32:28 -0800 Subject: [PATCH 05/12] SECURITY PATCH TCBZ4536 - CVE-2023-45231 - Patch --- NetworkPkg/Ip6Dxe/Ip6Option.c | 112 ++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 52 deletions(-) diff --git a/NetworkPkg/Ip6Dxe/Ip6Option.c b/NetworkPkg/Ip6Dxe/Ip6Option.c index 199eea124d..bc74d52a6c 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Option.c +++ b/NetworkPkg/Ip6Dxe/Ip6Option.c @@ -76,27 +76,27 @@ Ip6IsOptionValid ( case Ip6OptionParameterProblem: Pointer = Pointer + Offset + sizeof (EFI_IP6_HEADER); Ip6SendIcmpError ( - IpSb, - Packet, - NULL, - &Packet->Ip.Ip6->SourceAddress, - ICMP_V6_PARAMETER_PROBLEM, - 2, - &Pointer - ); + IpSb, + Packet, + NULL, + &Packet->Ip.Ip6->SourceAddress, + ICMP_V6_PARAMETER_PROBLEM, + 2, + &Pointer + ); return FALSE; case Ip6OptionMask: if (!IP6_IS_MULTICAST (&Packet->Ip.Ip6->DestinationAddress)) { Pointer = Pointer + Offset + sizeof (EFI_IP6_HEADER); Ip6SendIcmpError ( - IpSb, - Packet, - NULL, - &Packet->Ip.Ip6->SourceAddress, - ICMP_V6_PARAMETER_PROBLEM, - 2, - &Pointer - ); + IpSb, + Packet, + NULL, + &Packet->Ip.Ip6->SourceAddress, + ICMP_V6_PARAMETER_PROBLEM, + 2, + &Pointer + ); } return FALSE; @@ -137,6 +137,14 @@ Ip6IsNDOptionValid ( return FALSE; } + // + // Cannot process truncated options. + // Cannot process options with a length of 0 as there is no Type field. + // + if (OptionLen < sizeof (IP6_OPTION_HEADER)) { + return FALSE; + } + Offset = 0; // @@ -358,14 +366,14 @@ Ip6IsExtsValid ( !IP6_IS_MULTICAST (&Packet->Ip.Ip6->DestinationAddress)) { Ip6SendIcmpError ( - IpSb, - Packet, - NULL, - &Packet->Ip.Ip6->SourceAddress, - ICMP_V6_PARAMETER_PROBLEM, - 1, - &Pointer - ); + IpSb, + Packet, + NULL, + &Packet->Ip.Ip6->SourceAddress, + ICMP_V6_PARAMETER_PROBLEM, + 1, + &Pointer + ); } return FALSE; @@ -438,14 +446,14 @@ Ip6IsExtsValid ( !IP6_IS_MULTICAST (&Packet->Ip.Ip6->DestinationAddress)) { Ip6SendIcmpError ( - IpSb, - Packet, - NULL, - &Packet->Ip.Ip6->SourceAddress, - ICMP_V6_PARAMETER_PROBLEM, - 0, - &Pointer - ); + IpSb, + Packet, + NULL, + &Packet->Ip.Ip6->SourceAddress, + ICMP_V6_PARAMETER_PROBLEM, + 0, + &Pointer + ); } return FALSE; @@ -484,14 +492,14 @@ Ip6IsExtsValid ( { Pointer = sizeof (UINT32); Ip6SendIcmpError ( - IpSb, - Packet, - NULL, - &Packet->Ip.Ip6->SourceAddress, - ICMP_V6_PARAMETER_PROBLEM, - 0, - &Pointer - ); + IpSb, + Packet, + NULL, + &Packet->Ip.Ip6->SourceAddress, + ICMP_V6_PARAMETER_PROBLEM, + 0, + &Pointer + ); return FALSE; } } @@ -560,14 +568,14 @@ Ip6IsExtsValid ( !IP6_IS_MULTICAST (&Packet->Ip.Ip6->DestinationAddress)) { Ip6SendIcmpError ( - IpSb, - Packet, - NULL, - &Packet->Ip.Ip6->SourceAddress, - ICMP_V6_PARAMETER_PROBLEM, - 1, - &Pointer - ); + IpSb, + Packet, + NULL, + &Packet->Ip.Ip6->SourceAddress, + ICMP_V6_PARAMETER_PROBLEM, + 1, + &Pointer + ); } return FALSE; @@ -774,10 +782,10 @@ Ip6FillFragmentHeader ( // Append the part2 (fragmentable part) of Extension headers // CopyMem ( - Buffer + Part1Len + sizeof (IP6_FRAGMENT_HEADER), - ExtHdrs + Part1Len, - ExtHdrsLen - Part1Len - ); + Buffer + Part1Len + sizeof (IP6_FRAGMENT_HEADER), + ExtHdrs + Part1Len, + ExtHdrsLen - Part1Len + ); } *UpdatedExtHdrs = Buffer; -- 2.41.0 From 2f237a59ec4ea3df4b1614566ac29c90fb106669 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Fri, 15 Dec 2023 13:55:30 -0800 Subject: [PATCH 06/12] SECURITY PATCH TCBZ4536 - CVE-2023-45231 - Host Based Unit Test --- .../Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp | 27 ++++ .../Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf | 42 ++++++ .../Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp | 129 ++++++++++++++++++ NetworkPkg/Test/NetworkPkgHostTest.dsc | 1 + 4 files changed, 199 insertions(+) create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp new file mode 100644 index 0000000000..9dd5577249 --- /dev/null +++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp @@ -0,0 +1,27 @@ +/** @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 + +//////////////////////////////////////////////////////////////////////////////// +// Add test files here +// Google Test will only pick up the tests from the files that are included +// here. +//////////////////////////////////////////////////////////////////////////////// +#include "Ip6OptionGoogleTest.cpp" + +//////////////////////////////////////////////////////////////////////////////// +// Run the tests +//////////////////////////////////////////////////////////////////////////////// +int +main ( + int argc, + char *argv[] + ) +{ + testing::InitGoogleTest (&argc, argv); + return RUN_ALL_TESTS (); +} diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf new file mode 100644 index 0000000000..b85584b796 --- /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.
+# 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] + gZeroGuid \ No newline at end of file diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp new file mode 100644 index 0000000000..c4bcfacb92 --- /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 + +extern "C" { + #include + #include + #include + #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, OPTION_HEADER_IP6_PREFIX_DATA_LEN); + + EXPECT_FALSE (Ip6IsNDOptionValid (option, OPTION_HEADER_IP6_PREFIX_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)); +} \ No newline at end of file diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc index f6459b124f..8ed3585c06 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -25,6 +25,7 @@ # 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] -- 2.41.0 From 7ec488242f6c634a6e08fda25e4196ab24bf2d42 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Mon, 18 Dec 2023 10:37:40 -0800 Subject: [PATCH 07/12] SECURITY PATCH TCBZ4537 / TCBZ4538 - CVE-2023-45232 / CVE-2023-45233 - Patch --- NetworkPkg/Ip6Dxe/Ip6Option.c | 81 ++++++++++++++++++++++++++----- NetworkPkg/Ip6Dxe/Ip6Option.h | 89 +++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 13 deletions(-) diff --git a/NetworkPkg/Ip6Dxe/Ip6Option.c b/NetworkPkg/Ip6Dxe/Ip6Option.c index bc74d52a6c..9ca4d6ae5d 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Option.c +++ b/NetworkPkg/Ip6Dxe/Ip6Option.c @@ -9,6 +9,7 @@ #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 @@ -17,7 +18,8 @@ @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] OptionLen The length of all options, expressed in byte length of octets. + Maximum length is 2046 bytes or ((n + 1) * 8) - 2 where n is 255. @param[in] Pointer Identifies the octet offset within the invoking packet where the error was detected. @@ -31,12 +33,33 @@ Ip6IsOptionValid ( IN IP6_SERVICE *IpSb, IN NET_BUF *Packet, IN UINT8 *Option, - IN UINT8 OptionLen, + IN UINT16 OptionLen, IN UINT32 Pointer ) { - UINT8 Offset; - UINT8 OptionType; + UINT16 Offset; + UINT8 OptionType; + UINT8 OptDataLen; + + if (Option == NULL) { + ASSERT (Option != NULL); + return FALSE; + } + + if (OptionLen <= 0 || OptionLen > IP6_MAX_EXT_DATA_LENGTH) { + ASSERT (OptionLen > 0 && OptionLen <= IP6_MAX_EXT_DATA_LENGTH); + return FALSE; + } + + if (Packet == NULL) { + ASSERT (Packet != NULL); + return FALSE; + } + + if (IpSb == NULL) { + ASSERT (IpSb != NULL); + return FALSE; + } Offset = 0; @@ -54,7 +77,8 @@ Ip6IsOptionValid ( // // It is a PadN option // - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2); + OptDataLen = *(IP6_OFFSET_OF_OPT_LEN(Option + Offset)); + Offset = IP6_NEXT_OPTION_OFFSET(Offset, OptDataLen); break; case Ip6OptionRouterAlert: // @@ -69,7 +93,8 @@ Ip6IsOptionValid ( // switch (OptionType & Ip6OptionMask) { case Ip6OptionSkip: - Offset = (UINT8)(Offset + *(Option + Offset + 1)); + OptDataLen = *(IP6_OFFSET_OF_OPT_LEN(Option + Offset)); + Offset = IP6_NEXT_OPTION_OFFSET(Offset, OptDataLen); break; case Ip6OptionDiscard: return FALSE; @@ -308,7 +333,7 @@ Ip6IsExtsValid ( UINT32 Pointer; UINT32 Offset; UINT8 *Option; - UINT8 OptionLen; + UINT16 OptionLen; BOOLEAN Flag; UINT8 CountD; UINT8 CountA; @@ -385,6 +410,36 @@ Ip6IsExtsValid ( // Fall through // case IP6_DESTINATION: + // + // See https://www.rfc-editor.org/rfc/rfc2460#section-4.2 page 23 + // + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | Next Header | Hdr Ext Len | | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + + // | | + // . . + // . Options . + // . . + // | | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // + // + // Next Header 8-bit selector. Identifies the type of header + // immediately following the Destination Options + // header. Uses the same values as the IPv4 + // Protocol field [RFC-1700 et seq.]. + // + // Hdr Ext Len 8-bit unsigned integer. Length of the + // Destination Options header in 8-octet units, not + // including the first 8 octets. + // + // Options Variable-length field, of length such that the + // complete Destination Options header is an + // integer multiple of 8 octets long. Contains one + // or more TLV-encoded options, as described in + // section 4.2. + // + if (*NextHeader == IP6_DESTINATION) { CountD++; } @@ -397,8 +452,8 @@ Ip6IsExtsValid ( Pointer = Offset; Offset++; - Option = ExtHdrs + Offset; - OptionLen = (UINT8)((*Option + 1) * 8 - 2); + Option = ExtHdrs + Offset; + OptionLen = IP6_HDR_EXT_LEN(*Option) - IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN; Option++; Offset++; @@ -430,7 +485,7 @@ Ip6IsExtsValid ( // // Ignore the routing header and proceed to process the next header. // - Offset = Offset + (RoutingHead->HeaderLen + 1) * 8; + Offset = Offset + IP6_HDR_EXT_LEN(RoutingHead->HeaderLen); if (UnFragmentLen != NULL) { *UnFragmentLen = Offset; @@ -441,7 +496,7 @@ Ip6IsExtsValid ( // to the packet's source address, pointing to the unrecognized routing // type. // - Pointer = Offset + 2 + sizeof (EFI_IP6_HEADER); + Pointer = Offset + IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN + sizeof (EFI_IP6_HEADER); if ((IpSb != NULL) && (Packet != NULL) && !IP6_IS_MULTICAST (&Packet->Ip.Ip6->DestinationAddress)) { @@ -527,8 +582,8 @@ Ip6IsExtsValid ( // // RFC2402, Payload length is specified in 32-bit words, minus "2". // - OptionLen = (UINT8)((*Option + 2) * 4); - Offset = Offset + OptionLen; + OptionLen = ((UINT16)(*Option + 2) * 4); + Offset = Offset + OptionLen; break; case IP6_NO_NEXT_HEADER: diff --git a/NetworkPkg/Ip6Dxe/Ip6Option.h b/NetworkPkg/Ip6Dxe/Ip6Option.h index bd8e223c8a..1f9237b4e9 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Option.h +++ b/NetworkPkg/Ip6Dxe/Ip6Option.h @@ -12,6 +12,95 @@ #define IP6_FRAGMENT_OFFSET_MASK (~0x3) +// +// Per RFC8200 Section 4.2 +// +// Two of the currently-defined extension headers -- the Hop-by-Hop +// Options header and the Destination Options header -- carry a variable +// number of type-length-value (TLV) encoded "options", of the following +// format: +// +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+- - - - - - - - - +// | Option Type | Opt Data Len | Option Data +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+- - - - - - - - - +// +// Option Type 8-bit identifier of the type of option. +// +// Opt Data Len 8-bit unsigned integer. Length of the Option +// Data field of this option, in octets. +// +// Option Data Variable-length field. Option-Type-specific +// data. +// +#define IP6_SIZE_OF_OPT_TYPE (sizeof(UINT8)) +#define IP6_SIZE_OF_OPT_LEN (sizeof(UINT8)) +#define IP6_COMBINED_SIZE_OF_OPT_TAG_AND_LEN (IP6_SIZE_OF_OPT_TYPE + IP6_SIZE_OF_OPT_LEN) +#define IP6_OFFSET_OF_OPT_LEN(a) (a + IP6_SIZE_OF_OPT_TYPE) +STATIC_ASSERT ( + IP6_OFFSET_OF_OPT_LEN (0) == 1, + "The Length field should be 1 octet (8 bits) past the start of the option" + ); + +#define IP6_NEXT_OPTION_OFFSET(offset, length) (offset + IP6_COMBINED_SIZE_OF_OPT_TAG_AND_LEN + length) +STATIC_ASSERT ( + IP6_NEXT_OPTION_OFFSET (0, 0) == 2, + "The next option is minimally the combined size of the option tag and length" + ); + +// +// For more information see RFC 8200, Section 4.3, 4.4, and 4.6 +// +// This example format is from section 4.6 +// This does not apply to fragment headers +// +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | Next Header | Hdr Ext Len | | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + +// | | +// . . +// . Header-Specific Data . +// . . +// | | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// +// Next Header 8-bit selector. Identifies the type of +// header immediately following the extension +// header. Uses the same values as the IPv4 +// Protocol field [IANA-PN]. +// +// Hdr Ext Len 8-bit unsigned integer. Length of the +// Destination Options header in 8-octet units, +// not including the first 8 octets. + +// +// These defines apply to the following: +// 1. Hop by Hop +// 2. Routing +// 3. Destination +// +#define IP6_SIZE_OF_EXT_NEXT_HDR (sizeof(UINT8)) +#define IP6_SIZE_OF_HDR_EXT_LEN (sizeof(UINT8)) + +#define IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN (IP6_SIZE_OF_EXT_NEXT_HDR + IP6_SIZE_OF_HDR_EXT_LEN) +STATIC_ASSERT ( + IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN == 2, + "The combined size of Next Header and Len is two 8 bit fields" + ); + +// +// The "+ 1" in this calculation is because of the "not including the first 8 octets" +// part of the definition (meaning the value of 0 represents 64 bits) +// +#define IP6_HDR_EXT_LEN(a) (((UINT16)(UINT8)(a) + 1) * 8) + +// This is the maxmimum length permissible by a extension header +// Length is UINT8 of 8 octets not including the first 8 octets +#define IP6_MAX_EXT_DATA_LENGTH (IP6_HDR_EXT_LEN (MAX_UINT8) - IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN) +STATIC_ASSERT ( + IP6_MAX_EXT_DATA_LENGTH == 2046, + "Maximum data length is ((MAX_UINT8 + 1) * 8) - 2" + ); + typedef struct _IP6_FRAGMENT_HEADER { UINT8 NextHeader; UINT8 Reserved; -- 2.41.0 From d925ff1f00e769bcdbd04c1cb81560a8dec4e235 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Mon, 18 Dec 2023 10:49:41 -0800 Subject: [PATCH 08/12] SECURITY PATCH TCBZ4537 / TCBZ4538 - CVE-2023-45232 / CVE-2023-45233 - Host Based Unit Test --- .../Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp | 3 +- .../Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf | 5 +- .../Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp | 344 +++++++++++++++++- .../Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.h | 33 ++ NetworkPkg/Test/NetworkPkgHostTest.dsc | 2 +- 5 files changed, 376 insertions(+), 11 deletions(-) create mode 100644 NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.h diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp index 9dd5577249..5525da4231 100644 --- a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp +++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.cpp @@ -1,6 +1,5 @@ /** @file - Acts as the main entry point for the tests for the Ip6Dxe module. - + Acts as the main entry point for the tests for the Ip6Dxe driver. Copyright (c) Microsoft Corporation SPDX-License-Identifier: BSD-2-Clause-Patent **/ diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf index b85584b796..9f4ce85157 100644 --- a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf +++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf @@ -1,5 +1,5 @@ ## @file -# Unit test suite for the Ip6Dxe using Google Test +# Unit test suite for the Ip6DxeGoogleTest using Google Test # # Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: BSD-2-Clause-Patent @@ -31,7 +31,6 @@ DebugLib NetLib PcdLib - [Protocols] gEfiDhcp6ServiceBindingProtocolGuid @@ -39,4 +38,4 @@ gEfiNetworkPkgTokenSpaceGuid.PcdDhcp6UidType [Guids] - gZeroGuid \ No newline at end of file + gZeroGuid diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp index c4bcfacb92..640e96a17c 100644 --- a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp +++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.cpp @@ -1,5 +1,5 @@ /** @file - Tests for Ip6Option.c. + Host based unit test for Ip6Option.c. Copyright (c) Microsoft Corporation SPDX-License-Identifier: BSD-2-Clause-Patent @@ -12,18 +12,23 @@ extern "C" { #include #include "../Ip6Impl.h" #include "../Ip6Option.h" + #include "Ip6OptionGoogleTest.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 +/////////////////////////////////////////////////////////////////////// +// Symbol definitions +// +// These symbols / stub functions are required to be defined in order +// to compile but are not under test. These can be converted to +// Mock functions if required in the future. +// //////////////////////////////////////////////////////////////////////// UINT32 mIp6Id; @@ -126,4 +131,333 @@ TEST_F (Ip6OptionValidationTest, InvalidPrefixInfoOptionLengthShouldReturnFalse) UINT16 optionLen = sizeof (IP6_OPTION_HEADER); 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 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 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 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)); +} + +// Test Description +// Verify that a OptionLength that is too small fails +TEST_F (Ip6IsOptionValidTest, VerifyOptionLengthTooSmall) { + 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_FALSE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, 0, 0)); +} + +// Test Description +// Verify that a OptionLength that is too large fails +TEST_F (Ip6IsOptionValidTest, VerifyOptionLengthTooLarge) { + 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_FALSE (Ip6IsOptionValid (IpSb, &Packet, (UINT8 *)&optionHeader, MAX_UINT8, 0)); } \ No newline at end of file diff --git a/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.h b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.h new file mode 100644 index 0000000000..43ad56b4ab --- /dev/null +++ b/NetworkPkg/Ip6Dxe/GoogleTest/Ip6OptionGoogleTest.h @@ -0,0 +1,33 @@ +#ifndef __EFI_IP6_OPTION_GOOGLE_TEST_H__ +#define __EFI_IP6_OPTION_GOOGLE_TEST_H__ + +#include +#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 // __EFI_IP6_OPTION_GOOGLE_TEST_H__ \ No newline at end of file diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc index 8ed3585c06..a95a617d98 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -14,7 +14,7 @@ SUPPORTED_ARCHITECTURES = IA32|X64|AARCH64 BUILD_TARGETS = NOOPT SKUID_IDENTIFIER = DEFAULT - + !include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc [Packages] MdePkg/MdePkg.dec -- 2.41.0 From 813bfde4aa9b099c1eb6d9b38d3dbf3b9e3fa50d Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Mon, 18 Dec 2023 11:16:54 -0800 Subject: [PATCH 09/12] SECURITY PATCH TCBZ4539 - CVE-2023-45234 - Patch --- NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 71 +++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 6 deletions(-) diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c index 425e0cf806..4059fae5fc 100644 --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c @@ -3,6 +3,7 @@ (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) Microsoft Corporation SPDX-License-Identifier: BSD-2-Clause-Patent @@ -1312,6 +1313,65 @@ PxeBcSelectDhcp6Offer ( } } +/** + Cache the DHCPv6 DNS Server addresses + + @param[in] Private The pointer to PXEBC_PRIVATE_DATA. + @param[in] Cache6 The pointer to PXEBC_DHCP6_PACKET_CACHE. + + @retval EFI_SUCCESS Cache the DHCPv6 DNS Server address successfully. + @retval EFI_OUT_OF_RESOURCES Failed to allocate resources. + @retval EFI_DEVICE_ERROR The DNS Server Address Length provided by a untrusted + option is not a multiple of 16 bytes (sizeof (EFI_IPv6_ADDRESS)). +*/ +EFI_STATUS +PxeBcCacheDnsServerAddresses ( + IN PXEBC_PRIVATE_DATA *Private, + IN PXEBC_DHCP6_PACKET_CACHE *Cache6 + ) +{ + UINT16 DnsServerLen; + + DnsServerLen = NTOHS (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen); + // + // Make sure that the number is nonzero + // + if (DnsServerLen == 0) { + return EFI_DEVICE_ERROR; + } + + // + // Make sure the DnsServerlen is a multiple of EFI_IPv6_ADDRESS (16) + // + if (DnsServerLen % sizeof (EFI_IPv6_ADDRESS) != 0) { + return EFI_DEVICE_ERROR; + } + + // + // This code is currently written to only support a single DNS Server instead + // of multiple such as is spec defined (RFC3646, Section 3). The proper behavior + // would be to allocate the full space requested, CopyMem all of the data, + // and then add a DnsServerCount field to Private and update additional code + // that depends on this. + // + // To support multiple DNS servers the `AllocationSize` would need to be changed to DnsServerLen + // + // This is tracked in https://bugzilla.tianocore.org/show_bug.cgi?id=1886 + // + Private->DnsServer = AllocateZeroPool (sizeof (EFI_IPv6_ADDRESS)); + if (Private->DnsServer == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + // + // Intentionally only copy over the first server address. + // To support multiple DNS servers, the `Length` would need to be changed to DnsServerLen + // + CopyMem (Private->DnsServer, Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, sizeof (EFI_IPv6_ADDRESS)); + + return EFI_SUCCESS; +} + /** Handle the DHCPv6 offer packet. @@ -1335,6 +1395,7 @@ PxeBcHandleDhcp6Offer ( UINT32 SelectIndex; UINT32 Index; + ASSERT (Private != NULL); ASSERT (Private->SelectIndex > 0); SelectIndex = (UINT32)(Private->SelectIndex - 1); ASSERT (SelectIndex < PXEBC_OFFER_MAX_NUM); @@ -1342,15 +1403,13 @@ PxeBcHandleDhcp6Offer ( Status = EFI_SUCCESS; // - // First try to cache DNS server address if DHCP6 offer provides. + // First try to cache DNS server addresses if DHCP6 offer provides. // if (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] != NULL) { - Private->DnsServer = AllocateZeroPool (NTOHS (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen)); - if (Private->DnsServer == NULL) { - return EFI_OUT_OF_RESOURCES; + Status = PxeBcCacheDnsServerAddresses (Private, Cache6); + if (EFI_ERROR (Status)) { + return Status; } - - CopyMem (Private->DnsServer, Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, sizeof (EFI_IPv6_ADDRESS)); } if (Cache6->OfferType == PxeOfferTypeDhcpBinl) { -- 2.41.0 From 688eb3679f795d8699d09acc7a8c2322b389c440 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Mon, 18 Dec 2023 11:17:25 -0800 Subject: [PATCH 10/12] SECURITY PATCH TCBZ4539 - CVE-2023-45234 - Host Based Unit Test --- NetworkPkg/Test/NetworkPkgHostTest.dsc | 1 + .../GoogleTest/PxeBcDhcp6GoogleTest.cpp | 301 ++++++++++++++++++ .../GoogleTest/PxeBcDhcp6GoogleTest.h | 48 +++ .../GoogleTest/UefiPxeBcDxeGoogleTest.cpp | 21 ++ .../GoogleTest/UefiPxeBcDxeGoogleTest.inf | 47 +++ 5 files changed, 418 insertions(+) create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp create mode 100644 NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc index a95a617d98..b9d031a611 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -26,6 +26,7 @@ # NetworkPkg/Dhcp6Dxe/GoogleTest/Dhcp6DxeGoogleTest.inf NetworkPkg/Ip6Dxe/GoogleTest/Ip6DxeGoogleTest.inf + NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.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/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp new file mode 100644 index 0000000000..9ee805a284 --- /dev/null +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp @@ -0,0 +1,301 @@ +/** @file + Host based unit test for PxeBcDhcp6.c. + + Copyright (c) Microsoft Corporation + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ +#include + +extern "C" { +#include +#include +#include +#include "../PxeBcImpl.h" +#include "../PxeBcDhcp6.h" +#include "PxeBcDhcp6GoogleTest.h" +} + +/////////////////////////////////////////////////////////////////////////////// +// Definitions +/////////////////////////////////////////////////////////////////////////////// + +#define PACKET_SIZE (1500) + +typedef struct { + UINT16 OptionCode; // The option code for DHCP6_OPT_SERVER_ID (e.g., 0x03) + UINT16 OptionLen; // The length of the option (e.g., 16 bytes) + UINT8 ServerId[16]; // The 16-byte DHCPv6 Server Identifier +} DHCP6_OPTION_SERVER_ID; + +/////////////////////////////////////////////////////////////////////////////// +/// Symbol Definitions +/////////////////////////////////////////////////////////////////////////////// + +EFI_STATUS +MockUdpWrite ( + IN EFI_PXE_BASE_CODE_PROTOCOL *This, + IN UINT16 OpFlags, + IN EFI_IP_ADDRESS *DestIp, + IN EFI_PXE_BASE_CODE_UDP_PORT *DestPort, + IN EFI_IP_ADDRESS *GatewayIp OPTIONAL, + IN EFI_IP_ADDRESS *SrcIp OPTIONAL, + IN OUT EFI_PXE_BASE_CODE_UDP_PORT *SrcPort OPTIONAL, + IN UINTN *HeaderSize OPTIONAL, + IN VOID *HeaderPtr OPTIONAL, + IN UINTN *BufferSize, + IN VOID *BufferPtr + ) +{ + return EFI_SUCCESS; +} + +EFI_STATUS +MockUdpRead ( + IN EFI_PXE_BASE_CODE_PROTOCOL *This, + IN UINT16 OpFlags, + IN OUT EFI_IP_ADDRESS *DestIp OPTIONAL, + IN OUT EFI_PXE_BASE_CODE_UDP_PORT *DestPort OPTIONAL, + IN OUT EFI_IP_ADDRESS *SrcIp OPTIONAL, + IN OUT EFI_PXE_BASE_CODE_UDP_PORT *SrcPort OPTIONAL, + IN UINTN *HeaderSize OPTIONAL, + IN VOID *HeaderPtr OPTIONAL, + IN OUT UINTN *BufferSize, + IN VOID *BufferPtr + ) +{ + return EFI_SUCCESS; +} + +EFI_STATUS +MockConfigure ( + IN EFI_UDP6_PROTOCOL *This, + IN EFI_UDP6_CONFIG_DATA *UdpConfigData OPTIONAL + ) +{ + return EFI_SUCCESS; +} + +// Needed by PxeBcSupport +EFI_STATUS +EFIAPI +QueueDpc ( + IN EFI_TPL DpcTpl, + IN EFI_DPC_PROCEDURE DpcProcedure, + IN VOID *DpcContext OPTIONAL + ) +{ + return EFI_SUCCESS; +} + +/////////////////////////////////////////////////////////////////////////////// +// PxeBcHandleDhcp6OfferTest Tests +/////////////////////////////////////////////////////////////////////////////// + +class PxeBcHandleDhcp6OfferTest : public ::testing::Test { +public: +PXEBC_PRIVATE_DATA Private = { 0 }; +EFI_UDP6_PROTOCOL Udp6Read; +EFI_PXE_BASE_CODE_MODE Mode = { 0 }; + +protected: +// Add any setup code if needed +virtual void +SetUp ( + ) +{ + Private.Dhcp6Request = (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 = (EFI_PXE_BASE_CODE_UDP_WRITE)MockUdpWrite; + Private.PxeBc.UdpRead = (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 = (EFI_UDP6_CONFIGURE)MockConfigure; + Private.Udp6Read = &Udp6Read; + + // Need to setup the EFI_PXE_BASE_CODE_MODE + Private.PxeBc.Mode = &Mode; + + // for this test it doesn't really matter what the Dhcpv6 ack is set to +} + +// Add any cleanup code if needed +virtual void +TearDown ( + ) +{ + if (Private.Dhcp6Request != NULL) { + FreePool (Private.Dhcp6Request); + } + + // Clean up any resources or variables +} +}; + +// Note: +// Testing PxeBcHandleDhcp6Offer() is difficult because it depends on a +// properly setup Private structure. Attempting to properly test this function +// without a signficant refactor is a fools errand. Instead, we will test +// that we can prevent an overflow in the function. +TEST_F (PxeBcHandleDhcp6OfferTest, BasicUsageTest) { + PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; + EFI_DHCP6_PACKET_OPTION Option = { 0 }; + + Private.SelectIndex = 1; // SelectIndex is 1-based + Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; + + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; + // Setup the DHCPv6 offer packet + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (1337); + + ASSERT_EQ (PxeBcHandleDhcp6Offer (&(PxeBcHandleDhcp6OfferTest::Private)), EFI_DEVICE_ERROR); +} + +class PxeBcCacheDnsServerAddressesTest : public ::testing::Test { +public: +PXEBC_PRIVATE_DATA Private = { 0 }; + +protected: +// Add any setup code if needed +virtual void +SetUp ( + ) +{ +} + +// Add any cleanup code if needed +virtual void +TearDown ( + ) +{ +} +}; + +// Test Description +// Test that we cache the DNS server address from the DHCPv6 offer packet +TEST_F (PxeBcCacheDnsServerAddressesTest, BasicUsageTest) { + UINT8 SearchPattern[16] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF }; + EFI_DHCP6_PACKET_OPTION *Option; + PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; + + Option = (EFI_DHCP6_PACKET_OPTION *)AllocateZeroPool (sizeof (EFI_DHCP6_PACKET_OPTION) + sizeof (SearchPattern)); + ASSERT_NE (Option, nullptr); + + Option->OpCode = DHCP6_OPT_SERVER_ID; + Option->OpLen = NTOHS (sizeof (SearchPattern)); + CopyMem (Option->Data, SearchPattern, sizeof (SearchPattern)); + + Private.SelectIndex = 1; // SelectIndex is 1-based + Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = Option; + + Private.DnsServer = nullptr; + + ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_SUCCESS); + ASSERT_NE (Private.DnsServer, nullptr); + ASSERT_EQ (CompareMem (Private.DnsServer, SearchPattern, sizeof (SearchPattern)), 0); + + if (Private.DnsServer) { + FreePool (Private.DnsServer); + } + + if (Option) { + FreePool (Option); + } +} +// Test Description +// Test that we can prevent an overflow in the function +TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptOverflowTest) { + EFI_DHCP6_PACKET_OPTION Option = { 0 }; + PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; + + Private.SelectIndex = 1; // SelectIndex is 1-based + Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; + // Setup the DHCPv6 offer packet + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (1337); + + Private.DnsServer = NULL; + + ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_DEVICE_ERROR); + ASSERT_EQ (Private.DnsServer, nullptr); + + if (Private.DnsServer) { + FreePool (Private.DnsServer); + } +} + +// Test Description +// Test that we can prevent an underflow in the function +TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptUnderflowTest) { + EFI_DHCP6_PACKET_OPTION Option = { 0 }; + PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; + + Private.SelectIndex = 1; // SelectIndex is 1-based + Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; + // Setup the DHCPv6 offer packet + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (2); + + Private.DnsServer = NULL; + + ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_DEVICE_ERROR); + ASSERT_EQ (Private.DnsServer, nullptr); + + if (Private.DnsServer) { + FreePool (Private.DnsServer); + } +} + + +// Test Description +// Test that we can handle recursive dns (multiple dns entries) +TEST_F (PxeBcCacheDnsServerAddressesTest, MultipleDnsEntries) { + EFI_DHCP6_PACKET_OPTION Option = { 0 }; + PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; + + Private.SelectIndex = 1; // SelectIndex is 1-based + Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; + // Setup the DHCPv6 offer packet + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; + + EFI_IPv6_ADDRESS addresses[2] = { + // 2001:db8:85a3::8a2e:370:7334 + {0x20, 0x01, 0x0d, 0xb8, 0x85, 0xa3, 0x00, 0x00, 0x00, 0x00, 0x8a, 0x2e, 0x03, 0x70, 0x73, 0x34}, + // fe80::d478:91c3:ecd7:4ff9 + {0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xd4, 0x78, 0x91, 0xc3, 0xec, 0xd7, 0x4f, 0xf9} + }; + + + CopyMem(Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, &addresses, sizeof(addresses)); + + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (sizeof(addresses)); + + Private.DnsServer = NULL; + + ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_SUCCESS); + + ASSERT_NE (Private.DnsServer, nullptr); + + // + // This is expected to fail until DnsServer supports multiple DNS servers + // + // This is tracked in https://bugzilla.tianocore.org/show_bug.cgi?id=1886 + // + ASSERT_EQ (CompareMem(Private.DnsServer, &addresses, sizeof(addresses)), 0); + + if (Private.DnsServer) { + FreePool (Private.DnsServer); + } +} diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h new file mode 100644 index 0000000000..724ee468cb --- /dev/null +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h @@ -0,0 +1,48 @@ +/** @file + + This file exposes the internal interfaces which may be unit tested + for the PxeBcDhcp6Dxe driver. + + Copyright (c) Microsoft Corporation.
+ SPDX-License-Identifier: BSD-2-Clause-Patent +**/ +#ifndef __PXE_BC_DHCP6_GOOGLE_TEST_H__ +#define __PXE_BC_DHCP6_GOOGLE_TEST_H__ + +// Minimal includes needed to compile +#include +#include "../PxeBcImpl.h" + +/** + Handle the DHCPv6 offer packet. + + @param[in] Private The pointer to PXEBC_PRIVATE_DATA. + + @retval EFI_SUCCESS Handled the DHCPv6 offer packet successfully. + @retval EFI_NO_RESPONSE No response to the following request packet. + @retval EFI_OUT_OF_RESOURCES Failed to allocate resources. + @retval EFI_BUFFER_TOO_SMALL Can't cache the offer pacet. + +**/ +EFI_STATUS +PxeBcHandleDhcp6Offer ( + IN PXEBC_PRIVATE_DATA *Private + ); + + /** + Cache the DHCPv6 Server address + + @param[in] Private The pointer to PXEBC_PRIVATE_DATA. + @param[in] Cache6 The pointer to PXEBC_DHCP6_PACKET_CACHE. + + @retval EFI_SUCCESS Cache the DHCPv6 Server address successfully. + @retval EFI_OUT_OF_RESOURCES Failed to allocate resources. + @retval EFI_DEVICE_ERROR Failed to cache the DHCPv6 Server address. +*/ +EFI_STATUS +PxeBcCacheDnsServerAddresses ( + IN PXEBC_PRIVATE_DATA *Private, + IN PXEBC_DHCP6_PACKET_CACHE *Cache6 + ); + +#endif // __PXE_BC_DHCP6_GOOGLE_TEST_H__ \ No newline at end of file diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp new file mode 100644 index 0000000000..cc295f5c88 --- /dev/null +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp @@ -0,0 +1,21 @@ +/** @file + Acts as the main entry point for the tests for the UefiPxeBcDxe module. + Copyright (c) Microsoft Corporation + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ +#include + +//////////////////////////////////////////////////////////////////////////////// +// Add test files here +// Google Test will only pick up the tests from the files that are included +// here. +//////////////////////////////////////////////////////////////////////////////// +#include "PxeBcDhcp6GoogleTest.cpp" + +//////////////////////////////////////////////////////////////////////////////// +// Run the tests +//////////////////////////////////////////////////////////////////////////////// +int main(int argc, char* argv[]) { + testing::InitGoogleTest (&argc, argv); + return RUN_ALL_TESTS (); +} \ No newline at end of file diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf new file mode 100644 index 0000000000..c2fd7ae7a9 --- /dev/null +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf @@ -0,0 +1,47 @@ +## @file +# Unit test suite for the UefiPxeBcDxe using Google Test +# +# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent +## +[Defines] +INF_VERSION = 0x00010005 +BASE_NAME = UefiPxeBcDxeGoogleTest +FILE_GUID = 77D45C64-EC1E-4174-887B-886E89FD1EDF +MODULE_TYPE = HOST_APPLICATION +VERSION_STRING = 1.0 + +# +# The following information is for reference only and not required by the build tools. +# +# VALID_ARCHITECTURES = IA32 X64 +# + +[Sources] + UefiPxeBcDxeGoogleTest.cpp + PxeBcDhcp6GoogleTest.cpp + ../PxeBcDhcp6.c + ../PxeBcSupport.c + +[Packages] + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec + UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec + NetworkPkg/NetworkPkg.dec + +[LibraryClasses] + GoogleTestLib + DebugLib + NetLib + PcdLib + +[Protocols] + gEfiDhcp6ServiceBindingProtocolGuid + gEfiDns6ServiceBindingProtocolGuid + gEfiDns6ProtocolGuid + +[Pcd] + gEfiNetworkPkgTokenSpaceGuid.PcdDhcp6UidType + +[Guids] + gZeroGuid \ No newline at end of file -- 2.41.0 From 4412e98d2835b0a8a0c80763bb1ef4d62015aa12 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Mon, 18 Dec 2023 12:45:31 -0800 Subject: [PATCH 11/12] SECURITY PATCH TCBZ4540 - CVE-2023-45235 - Patch --- NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 85 +++++++++++++++++++++------- NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h | 17 ++++++ 2 files changed, 83 insertions(+), 19 deletions(-) diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c index 4059fae5fc..a3b977fe2a 100644 --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c @@ -887,6 +887,7 @@ PxeBcRequestBootService ( EFI_STATUS Status; EFI_DHCP6_PACKET *IndexOffer; UINT8 *Option; + UINTN DiscoverLenNeeded; PxeBc = &Private->PxeBc; Request = Private->Dhcp6Request; @@ -898,8 +899,9 @@ PxeBcRequestBootService ( if (Request == NULL) { return EFI_DEVICE_ERROR; } - - Discover = AllocateZeroPool (sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET)); + + DiscoverLenNeeded = sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET); + Discover = AllocateZeroPool (DiscoverLenNeeded); if (Discover == NULL) { return EFI_OUT_OF_RESOURCES; } @@ -924,16 +926,34 @@ PxeBcRequestBootService ( DHCP6_OPT_SERVER_ID ); if (Option == NULL) { - return EFI_NOT_FOUND; + Status = EFI_NOT_FOUND; + goto ON_ERROR; } // // Add Server ID Option. // OpLen = NTOHS (((EFI_DHCP6_PACKET_OPTION *)Option)->OpLen); - CopyMem (DiscoverOpt, Option, OpLen + 4); - DiscoverOpt += (OpLen + 4); - DiscoverLen += (OpLen + 4); + + // + // Check that the minimum and maximum requirements are met + // + if (OpLen < PXEBC_MIN_SIZE_OF_DUID || OpLen > PXEBC_MAX_SIZE_OF_DUID) { + Status = EFI_INVALID_PARAMETER; + goto ON_ERROR; + } + + // + // Check that the option length is valid. + // + if ((DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN) > DiscoverLenNeeded) { + Status = EFI_OUT_OF_RESOURCES; + goto ON_ERROR; + } + + CopyMem (DiscoverOpt, Option, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); + DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); + DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); } while (RequestLen < Request->Length) { @@ -944,16 +964,25 @@ PxeBcRequestBootService ( (OpCode != DHCP6_OPT_SERVER_ID) ) { + + // + // Check that the option length is valid. + // + if (DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN > DiscoverLenNeeded) { + Status = EFI_OUT_OF_RESOURCES; + goto ON_ERROR; + } + // // Copy all the options except IA option and Server ID // - CopyMem (DiscoverOpt, RequestOpt, OpLen + 4); - DiscoverOpt += (OpLen + 4); - DiscoverLen += (OpLen + 4); + CopyMem (DiscoverOpt, RequestOpt, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); + DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); + DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); } - RequestOpt += (OpLen + 4); - RequestLen += (OpLen + 4); + RequestOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); + RequestLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); } // @@ -2154,7 +2183,8 @@ PxeBcDhcp6Discover ( UINT16 OpLen; UINT32 Xid; EFI_STATUS Status; - + UINTN DiscoverLenNeeded; + PxeBc = &Private->PxeBc; Mode = PxeBc->Mode; Request = Private->Dhcp6Request; @@ -2168,8 +2198,9 @@ PxeBcDhcp6Discover ( if (Request == NULL) { return EFI_DEVICE_ERROR; } - - Discover = AllocateZeroPool (sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET)); + + DiscoverLenNeeded = sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET); + Discover = AllocateZeroPool (DiscoverLenNeeded); if (Discover == NULL) { return EFI_OUT_OF_RESOURCES; } @@ -2185,22 +2216,38 @@ PxeBcDhcp6Discover ( DiscoverLen = sizeof (EFI_DHCP6_HEADER); RequestLen = DiscoverLen; + // + // The request packet is generated by the UEFI network stack. In the DHCP4 DORA and DHCP6 SARR sequence, + // the first (discover in DHCP4 and solicit in DHCP6) and third (request in both DHCP4 and DHCP6) are + // generated by the DHCP client (the UEFI network stack in this case). By the time this function executes, + // the DHCP sequence already has been executed once (see UEFI Specification Figures 24.2 and 24.3), with + // Private->Dhcp6Request being a cached copy of the DHCP6 request packet that UEFI network stack previously + // generated and sent. + // + // Therefore while this code looks like it could overflow, in practice it's not possible. + // while (RequestLen < Request->Length) { OpCode = NTOHS (((EFI_DHCP6_PACKET_OPTION *)RequestOpt)->OpCode); OpLen = NTOHS (((EFI_DHCP6_PACKET_OPTION *)RequestOpt)->OpLen); if ((OpCode != EFI_DHCP6_IA_TYPE_NA) && (OpCode != EFI_DHCP6_IA_TYPE_TA)) { + + if (DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN > DiscoverLenNeeded) { + Status = EFI_OUT_OF_RESOURCES; + goto ON_ERROR; + } + // // Copy all the options except IA option. // - CopyMem (DiscoverOpt, RequestOpt, OpLen + 4); - DiscoverOpt += (OpLen + 4); - DiscoverLen += (OpLen + 4); + CopyMem (DiscoverOpt, RequestOpt, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); + DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); + DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); } - RequestOpt += (OpLen + 4); - RequestLen += (OpLen + 4); + RequestOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); + RequestLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN); } Status = PxeBc->UdpWrite ( diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h index c86f6d391b..2f11f0e1d9 100644 --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h @@ -34,6 +34,23 @@ #define PXEBC_ADDR_START_DELIMITER '[' #define PXEBC_ADDR_END_DELIMITER ']' +// +// A DUID consists of a 2-octet type code represented in network byte +// order, followed by a variable number of octets that make up the +// actual identifier. The length of the DUID (not including the type +// code) is at least 1 octet and at most 128 octets. +// +#define PXEBC_MIN_SIZE_OF_DUID (sizeof(UINT16) + 1) +#define PXEBC_MAX_SIZE_OF_DUID (sizeof(UINT16) + 128) + +// +// This define represents the combineds code and length field from +// https://datatracker.ietf.org/doc/html/rfc3315#section-22.1 +// +#define PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN \ + (sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpCode) + \ + sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpLen)) + #define GET_NEXT_DHCP6_OPTION(Opt) \ (EFI_DHCP6_PACKET_OPTION *) ((UINT8 *) (Opt) + \ sizeof (EFI_DHCP6_PACKET_OPTION) + (NTOHS ((Opt)->OpLen)) - 1) -- 2.41.0 From bef5afa1c2c6868323332dbad0478b85830cd134 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Mon, 18 Dec 2023 12:49:26 -0800 Subject: [PATCH 12/12] SECURITY PATCH TCBZ4540 - CVE-2023-45235 - Host Based Unit Test --- NetworkPkg/Test/NetworkPkgHostTest.dsc | 11 +- .../GoogleTest/PxeBcDhcp6GoogleTest.cpp | 871 ++++++++++++------ .../GoogleTest/PxeBcDhcp6GoogleTest.h | 18 + .../GoogleTest/UefiPxeBcDxeGoogleTest.cpp | 42 +- .../GoogleTest/UefiPxeBcDxeGoogleTest.inf | 92 +- 5 files changed, 661 insertions(+), 373 deletions(-) diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc index b9d031a611..e6c55692a1 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -26,7 +26,10 @@ # 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/MockUefiRuntimeServicesTableLib/MockUefiRuntimeServicesTableLib.inf + } # Despite these library classes being listed in [LibraryClasses] below, they are not needed for the host-based unit tests. [LibraryClasses] @@ -88,11 +91,9 @@ # This library provides the instrinsic functions generated by a given compiler. # [LibraryClasses.ARM] and NULL mean link this library into all ARM images. # - # MU_CHANGE Start -!if $(TOOL_CHAIN_TAG) != VS2017 and $(TOOL_CHAIN_TAG) != VS2015 and $(TOOL_CHAIN_TAG) != VS2019 - NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf +!if $(TOOL_CHAIN_TAG) == VS2019 or $(TOOL_CHAIN_TAG) == VS2022 + NULL|MdePkg/Library/CompilerIntrinsicsLib/ArmCompilerIntrinsicsLib.inf !endif - # MU_CHANGE End NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf [LibraryClasses.ARM] RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp index 9ee805a284..665aff7d72 100644 --- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp @@ -1,301 +1,570 @@ -/** @file - Host based unit test for PxeBcDhcp6.c. - - Copyright (c) Microsoft Corporation - SPDX-License-Identifier: BSD-2-Clause-Patent -**/ -#include - -extern "C" { -#include -#include -#include -#include "../PxeBcImpl.h" -#include "../PxeBcDhcp6.h" -#include "PxeBcDhcp6GoogleTest.h" -} - -/////////////////////////////////////////////////////////////////////////////// -// Definitions -/////////////////////////////////////////////////////////////////////////////// - -#define PACKET_SIZE (1500) - -typedef struct { - UINT16 OptionCode; // The option code for DHCP6_OPT_SERVER_ID (e.g., 0x03) - UINT16 OptionLen; // The length of the option (e.g., 16 bytes) - UINT8 ServerId[16]; // The 16-byte DHCPv6 Server Identifier -} DHCP6_OPTION_SERVER_ID; - -/////////////////////////////////////////////////////////////////////////////// -/// Symbol Definitions -/////////////////////////////////////////////////////////////////////////////// - -EFI_STATUS -MockUdpWrite ( - IN EFI_PXE_BASE_CODE_PROTOCOL *This, - IN UINT16 OpFlags, - IN EFI_IP_ADDRESS *DestIp, - IN EFI_PXE_BASE_CODE_UDP_PORT *DestPort, - IN EFI_IP_ADDRESS *GatewayIp OPTIONAL, - IN EFI_IP_ADDRESS *SrcIp OPTIONAL, - IN OUT EFI_PXE_BASE_CODE_UDP_PORT *SrcPort OPTIONAL, - IN UINTN *HeaderSize OPTIONAL, - IN VOID *HeaderPtr OPTIONAL, - IN UINTN *BufferSize, - IN VOID *BufferPtr - ) -{ - return EFI_SUCCESS; -} - -EFI_STATUS -MockUdpRead ( - IN EFI_PXE_BASE_CODE_PROTOCOL *This, - IN UINT16 OpFlags, - IN OUT EFI_IP_ADDRESS *DestIp OPTIONAL, - IN OUT EFI_PXE_BASE_CODE_UDP_PORT *DestPort OPTIONAL, - IN OUT EFI_IP_ADDRESS *SrcIp OPTIONAL, - IN OUT EFI_PXE_BASE_CODE_UDP_PORT *SrcPort OPTIONAL, - IN UINTN *HeaderSize OPTIONAL, - IN VOID *HeaderPtr OPTIONAL, - IN OUT UINTN *BufferSize, - IN VOID *BufferPtr - ) -{ - return EFI_SUCCESS; -} - -EFI_STATUS -MockConfigure ( - IN EFI_UDP6_PROTOCOL *This, - IN EFI_UDP6_CONFIG_DATA *UdpConfigData OPTIONAL - ) -{ - return EFI_SUCCESS; -} - -// Needed by PxeBcSupport -EFI_STATUS -EFIAPI -QueueDpc ( - IN EFI_TPL DpcTpl, - IN EFI_DPC_PROCEDURE DpcProcedure, - IN VOID *DpcContext OPTIONAL - ) -{ - return EFI_SUCCESS; -} - -/////////////////////////////////////////////////////////////////////////////// -// PxeBcHandleDhcp6OfferTest Tests -/////////////////////////////////////////////////////////////////////////////// - -class PxeBcHandleDhcp6OfferTest : public ::testing::Test { -public: -PXEBC_PRIVATE_DATA Private = { 0 }; -EFI_UDP6_PROTOCOL Udp6Read; -EFI_PXE_BASE_CODE_MODE Mode = { 0 }; - -protected: -// Add any setup code if needed -virtual void -SetUp ( - ) -{ - Private.Dhcp6Request = (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 = (EFI_PXE_BASE_CODE_UDP_WRITE)MockUdpWrite; - Private.PxeBc.UdpRead = (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 = (EFI_UDP6_CONFIGURE)MockConfigure; - Private.Udp6Read = &Udp6Read; - - // Need to setup the EFI_PXE_BASE_CODE_MODE - Private.PxeBc.Mode = &Mode; - - // for this test it doesn't really matter what the Dhcpv6 ack is set to -} - -// Add any cleanup code if needed -virtual void -TearDown ( - ) -{ - if (Private.Dhcp6Request != NULL) { - FreePool (Private.Dhcp6Request); - } - - // Clean up any resources or variables -} -}; - -// Note: -// Testing PxeBcHandleDhcp6Offer() is difficult because it depends on a -// properly setup Private structure. Attempting to properly test this function -// without a signficant refactor is a fools errand. Instead, we will test -// that we can prevent an overflow in the function. -TEST_F (PxeBcHandleDhcp6OfferTest, BasicUsageTest) { - PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; - EFI_DHCP6_PACKET_OPTION Option = { 0 }; - - Private.SelectIndex = 1; // SelectIndex is 1-based - Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; - - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; - // Setup the DHCPv6 offer packet - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (1337); - - ASSERT_EQ (PxeBcHandleDhcp6Offer (&(PxeBcHandleDhcp6OfferTest::Private)), EFI_DEVICE_ERROR); -} - -class PxeBcCacheDnsServerAddressesTest : public ::testing::Test { -public: -PXEBC_PRIVATE_DATA Private = { 0 }; - -protected: -// Add any setup code if needed -virtual void -SetUp ( - ) -{ -} - -// Add any cleanup code if needed -virtual void -TearDown ( - ) -{ -} -}; - -// Test Description -// Test that we cache the DNS server address from the DHCPv6 offer packet -TEST_F (PxeBcCacheDnsServerAddressesTest, BasicUsageTest) { - UINT8 SearchPattern[16] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF }; - EFI_DHCP6_PACKET_OPTION *Option; - PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; - - Option = (EFI_DHCP6_PACKET_OPTION *)AllocateZeroPool (sizeof (EFI_DHCP6_PACKET_OPTION) + sizeof (SearchPattern)); - ASSERT_NE (Option, nullptr); - - Option->OpCode = DHCP6_OPT_SERVER_ID; - Option->OpLen = NTOHS (sizeof (SearchPattern)); - CopyMem (Option->Data, SearchPattern, sizeof (SearchPattern)); - - Private.SelectIndex = 1; // SelectIndex is 1-based - Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = Option; - - Private.DnsServer = nullptr; - - ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_SUCCESS); - ASSERT_NE (Private.DnsServer, nullptr); - ASSERT_EQ (CompareMem (Private.DnsServer, SearchPattern, sizeof (SearchPattern)), 0); - - if (Private.DnsServer) { - FreePool (Private.DnsServer); - } - - if (Option) { - FreePool (Option); - } -} -// Test Description -// Test that we can prevent an overflow in the function -TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptOverflowTest) { - EFI_DHCP6_PACKET_OPTION Option = { 0 }; - PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; - - Private.SelectIndex = 1; // SelectIndex is 1-based - Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; - // Setup the DHCPv6 offer packet - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (1337); - - Private.DnsServer = NULL; - - ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_DEVICE_ERROR); - ASSERT_EQ (Private.DnsServer, nullptr); - - if (Private.DnsServer) { - FreePool (Private.DnsServer); - } -} - -// Test Description -// Test that we can prevent an underflow in the function -TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptUnderflowTest) { - EFI_DHCP6_PACKET_OPTION Option = { 0 }; - PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; - - Private.SelectIndex = 1; // SelectIndex is 1-based - Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; - // Setup the DHCPv6 offer packet - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (2); - - Private.DnsServer = NULL; - - ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_DEVICE_ERROR); - ASSERT_EQ (Private.DnsServer, nullptr); - - if (Private.DnsServer) { - FreePool (Private.DnsServer); - } -} - - -// Test Description -// Test that we can handle recursive dns (multiple dns entries) -TEST_F (PxeBcCacheDnsServerAddressesTest, MultipleDnsEntries) { - EFI_DHCP6_PACKET_OPTION Option = { 0 }; - PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; - - Private.SelectIndex = 1; // SelectIndex is 1-based - Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; - // Setup the DHCPv6 offer packet - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; - - EFI_IPv6_ADDRESS addresses[2] = { - // 2001:db8:85a3::8a2e:370:7334 - {0x20, 0x01, 0x0d, 0xb8, 0x85, 0xa3, 0x00, 0x00, 0x00, 0x00, 0x8a, 0x2e, 0x03, 0x70, 0x73, 0x34}, - // fe80::d478:91c3:ecd7:4ff9 - {0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xd4, 0x78, 0x91, 0xc3, 0xec, 0xd7, 0x4f, 0xf9} - }; - - - CopyMem(Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, &addresses, sizeof(addresses)); - - Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (sizeof(addresses)); - - Private.DnsServer = NULL; - - ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_SUCCESS); - - ASSERT_NE (Private.DnsServer, nullptr); - - // - // This is expected to fail until DnsServer supports multiple DNS servers - // - // This is tracked in https://bugzilla.tianocore.org/show_bug.cgi?id=1886 - // - ASSERT_EQ (CompareMem(Private.DnsServer, &addresses, sizeof(addresses)), 0); - - if (Private.DnsServer) { - FreePool (Private.DnsServer); - } -} +/** @file + Host based unit test for PxeBcDhcp6.c. + + Copyright (c) Microsoft Corporation + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ +#include +#include +#include +#include + +extern "C" { +#include +#include +#include +#include "../PxeBcImpl.h" +#include "../PxeBcDhcp6.h" +#include "PxeBcDhcp6GoogleTest.h" +} + +/////////////////////////////////////////////////////////////////////////////// +// Definitions +/////////////////////////////////////////////////////////////////////////////// + +#define PACKET_SIZE (1500) + +typedef struct { + UINT16 OptionCode; // The option code for DHCP6_OPT_SERVER_ID (e.g., 0x03) + UINT16 OptionLen; // The length of the option (e.g., 16 bytes) + UINT8 ServerId[16]; // The 16-byte DHCPv6 Server Identifier +} DHCP6_OPTION_SERVER_ID; + +/////////////////////////////////////////////////////////////////////////////// +/// Symbol Definitions +/////////////////////////////////////////////////////////////////////////////// + +EFI_STATUS +MockUdpWrite ( + IN EFI_PXE_BASE_CODE_PROTOCOL *This, + IN UINT16 OpFlags, + IN EFI_IP_ADDRESS *DestIp, + IN EFI_PXE_BASE_CODE_UDP_PORT *DestPort, + IN EFI_IP_ADDRESS *GatewayIp OPTIONAL, + IN EFI_IP_ADDRESS *SrcIp OPTIONAL, + IN OUT EFI_PXE_BASE_CODE_UDP_PORT *SrcPort OPTIONAL, + IN UINTN *HeaderSize OPTIONAL, + IN VOID *HeaderPtr OPTIONAL, + IN UINTN *BufferSize, + IN VOID *BufferPtr + ) +{ + return EFI_SUCCESS; +} + +EFI_STATUS +MockUdpRead ( + IN EFI_PXE_BASE_CODE_PROTOCOL *This, + IN UINT16 OpFlags, + IN OUT EFI_IP_ADDRESS *DestIp OPTIONAL, + IN OUT EFI_PXE_BASE_CODE_UDP_PORT *DestPort OPTIONAL, + IN OUT EFI_IP_ADDRESS *SrcIp OPTIONAL, + IN OUT EFI_PXE_BASE_CODE_UDP_PORT *SrcPort OPTIONAL, + IN UINTN *HeaderSize OPTIONAL, + IN VOID *HeaderPtr OPTIONAL, + IN OUT UINTN *BufferSize, + IN VOID *BufferPtr + ) +{ + return EFI_SUCCESS; +} + +EFI_STATUS +MockConfigure ( + IN EFI_UDP6_PROTOCOL *This, + IN EFI_UDP6_CONFIG_DATA *UdpConfigData OPTIONAL + ) +{ + return EFI_SUCCESS; +} + +// 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 ( + IN EFI_TPL DpcTpl, + IN EFI_DPC_PROCEDURE DpcProcedure, + IN VOID *DpcContext OPTIONAL + ) +{ + return EFI_SUCCESS; +} + +/////////////////////////////////////////////////////////////////////////////// +// PxeBcHandleDhcp6OfferTest Tests +/////////////////////////////////////////////////////////////////////////////// + +class PxeBcHandleDhcp6OfferTest : public ::testing::Test { +public: +PXEBC_PRIVATE_DATA Private = { 0 }; +EFI_UDP6_PROTOCOL Udp6Read; +EFI_PXE_BASE_CODE_MODE Mode = { 0 }; + +protected: +// Add any setup code if needed +virtual void +SetUp ( + ) +{ + Private.Dhcp6Request = (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 = (EFI_PXE_BASE_CODE_UDP_WRITE)MockUdpWrite; + Private.PxeBc.UdpRead = (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 = (EFI_UDP6_CONFIGURE)MockConfigure; + Private.Udp6Read = &Udp6Read; + + // Need to setup the EFI_PXE_BASE_CODE_MODE + Private.PxeBc.Mode = &Mode; + + // for this test it doesn't really matter what the Dhcpv6 ack is set to +} + +// Add any cleanup code if needed +virtual void +TearDown ( + ) +{ + if (Private.Dhcp6Request != NULL) { + FreePool (Private.Dhcp6Request); + } + + // Clean up any resources or variables +} +}; + +// Note: +// Testing PxeBcHandleDhcp6Offer() is difficult because it depends on a +// properly setup Private structure. Attempting to properly test this function +// without a signficant refactor is a fools errand. Instead, we will test +// that we can prevent an overflow in the function. +TEST_F (PxeBcHandleDhcp6OfferTest, BasicUsageTest) { + PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; + EFI_DHCP6_PACKET_OPTION Option = { 0 }; + + Private.SelectIndex = 1; // SelectIndex is 1-based + Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; + + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; + // Setup the DHCPv6 offer packet + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (1337); + + ASSERT_EQ (PxeBcHandleDhcp6Offer (&(PxeBcHandleDhcp6OfferTest::Private)), EFI_DEVICE_ERROR); +} + +class PxeBcCacheDnsServerAddressesTest : public ::testing::Test { +public: +PXEBC_PRIVATE_DATA Private = { 0 }; + +protected: +// Add any setup code if needed +virtual void +SetUp ( + ) +{ +} + +// Add any cleanup code if needed +virtual void +TearDown ( + ) +{ +} +}; + +// Test Description +// Test that we cache the DNS server address from the DHCPv6 offer packet +TEST_F (PxeBcCacheDnsServerAddressesTest, BasicUsageTest) { + UINT8 SearchPattern[16] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF }; + EFI_DHCP6_PACKET_OPTION *Option; + PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; + + Option = (EFI_DHCP6_PACKET_OPTION *)AllocateZeroPool (sizeof (EFI_DHCP6_PACKET_OPTION) + sizeof (SearchPattern)); + ASSERT_NE (Option, nullptr); + + Option->OpCode = DHCP6_OPT_SERVER_ID; + Option->OpLen = NTOHS (sizeof (SearchPattern)); + CopyMem (Option->Data, SearchPattern, sizeof (SearchPattern)); + + Private.SelectIndex = 1; // SelectIndex is 1-based + Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = Option; + + Private.DnsServer = nullptr; + + ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_SUCCESS); + ASSERT_NE (Private.DnsServer, nullptr); + ASSERT_EQ (CompareMem (Private.DnsServer, SearchPattern, sizeof (SearchPattern)), 0); + + if (Private.DnsServer) { + FreePool (Private.DnsServer); + } + + if (Option) { + FreePool (Option); + } +} +// Test Description +// Test that we can prevent an overflow in the function +TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptOverflowTest) { + EFI_DHCP6_PACKET_OPTION Option = { 0 }; + PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; + + Private.SelectIndex = 1; // SelectIndex is 1-based + Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; + // Setup the DHCPv6 offer packet + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (1337); + + Private.DnsServer = NULL; + + ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_DEVICE_ERROR); + ASSERT_EQ (Private.DnsServer, nullptr); + + if (Private.DnsServer) { + FreePool (Private.DnsServer); + } +} + +// Test Description +// Test that we can prevent an underflow in the function +TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptUnderflowTest) { + EFI_DHCP6_PACKET_OPTION Option = { 0 }; + PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; + + Private.SelectIndex = 1; // SelectIndex is 1-based + Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; + // Setup the DHCPv6 offer packet + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (2); + + Private.DnsServer = NULL; + + ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_DEVICE_ERROR); + ASSERT_EQ (Private.DnsServer, nullptr); + + if (Private.DnsServer) { + FreePool (Private.DnsServer); + } +} + + +// Test Description +// Test that we can handle recursive dns (multiple dns entries) +TEST_F (PxeBcCacheDnsServerAddressesTest, MultipleDnsEntries) { + EFI_DHCP6_PACKET_OPTION Option = { 0 }; + PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; + + Private.SelectIndex = 1; // SelectIndex is 1-based + Cache6 = &Private.OfferBuffer[Private.SelectIndex - 1].Dhcp6; + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] = &Option; + // Setup the DHCPv6 offer packet + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpCode = DHCP6_OPT_SERVER_ID; + + EFI_IPv6_ADDRESS addresses[2] = { + // 2001:db8:85a3::8a2e:370:7334 + {0x20, 0x01, 0x0d, 0xb8, 0x85, 0xa3, 0x00, 0x00, 0x00, 0x00, 0x8a, 0x2e, 0x03, 0x70, 0x73, 0x34}, + // fe80::d478:91c3:ecd7:4ff9 + {0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xd4, 0x78, 0x91, 0xc3, 0xec, 0xd7, 0x4f, 0xf9} + }; + + + CopyMem(Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, &addresses, sizeof(addresses)); + + Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen = NTOHS (sizeof(addresses)); + + Private.DnsServer = NULL; + + ASSERT_EQ (PxeBcCacheDnsServerAddresses (&(PxeBcCacheDnsServerAddressesTest::Private), Cache6), EFI_SUCCESS); + + ASSERT_NE (Private.DnsServer, nullptr); + + // + // This is expected to fail until DnsServer supports multiple DNS servers + // + // This is tracked in https://bugzilla.tianocore.org/show_bug.cgi?id=1886 + // + ASSERT_EQ (CompareMem(Private.DnsServer, &addresses, sizeof(addresses)), 0); + + if (Private.DnsServer) { + FreePool (Private.DnsServer); + } +} + +/////////////////////////////////////////////////////////////////////////////// +// PxeBcRequestBootServiceTest Test Cases +/////////////////////////////////////////////////////////////////////////////// + +class PxeBcRequestBootServiceTest : public ::testing::Test { +public: +PXEBC_PRIVATE_DATA Private = { 0 }; +EFI_UDP6_PROTOCOL Udp6Read; + +protected: +// Add any setup code if needed +virtual void +SetUp ( + ) +{ + Private.Dhcp6Request = (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 = (EFI_PXE_BASE_CODE_UDP_WRITE)MockUdpWrite; + Private.PxeBc.UdpRead = (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 = (EFI_UDP6_CONFIGURE)MockConfigure; + Private.Udp6Read = &Udp6Read; +} + +// Add any cleanup code if needed +virtual void +TearDown ( + ) +{ + if (Private.Dhcp6Request != NULL) { + FreePool (Private.Dhcp6Request); + } + + // Clean up any resources or variables +} +}; + +TEST_F (PxeBcRequestBootServiceTest, ServerDiscoverBasicUsageTest) { + PxeBcRequestBootServiceTest::Private.OfferBuffer[0].Dhcp6.OfferType = PxeOfferTypeProxyBinl; + + DHCP6_OPTION_SERVER_ID Server = { 0 }; + + Server.OptionCode = HTONS (DHCP6_OPT_SERVER_ID); + Server.OptionLen = HTONS (16); // valid length + UINT8 Index = 0; + + EFI_DHCP6_PACKET *Packet = (EFI_DHCP6_PACKET *)&Private.OfferBuffer[Index].Dhcp6.Packet.Offer; + + UINT8 *Cursor = (UINT8 *)(Packet->Dhcp6.Option); + + CopyMem (Cursor, &Server, sizeof (Server)); + Cursor += sizeof (Server); + + // Update the packet length + Packet->Length = (UINT16)(Cursor - (UINT8 *)Packet); + Packet->Size = PACKET_SIZE; + + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Private), Index), EFI_SUCCESS); +} + +TEST_F (PxeBcRequestBootServiceTest, AttemptDiscoverOverFlowExpectFailure) { + PxeBcRequestBootServiceTest::Private.OfferBuffer[0].Dhcp6.OfferType = PxeOfferTypeProxyBinl; + + DHCP6_OPTION_SERVER_ID Server = { 0 }; + + Server.OptionCode = HTONS (DHCP6_OPT_SERVER_ID); + Server.OptionLen = HTONS (1500); // This length would overflow without a check + UINT8 Index = 0; + + EFI_DHCP6_PACKET *Packet = (EFI_DHCP6_PACKET *)&Private.OfferBuffer[Index].Dhcp6.Packet.Offer; + + UINT8 *Cursor = (UINT8 *)(Packet->Dhcp6.Option); + + CopyMem (Cursor, &Server, sizeof (Server)); + Cursor += sizeof (Server); + + // Update the packet length + Packet->Length = (UINT16)(Cursor - (UINT8 *)Packet); + Packet->Size = PACKET_SIZE; + + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Private), Index), EFI_OUT_OF_RESOURCES); +} + +TEST_F (PxeBcRequestBootServiceTest, RequestBasicUsageTest) { + EFI_DHCP6_PACKET_OPTION RequestOpt = { 0 }; // the data section doesn't really matter + + RequestOpt.OpCode = HTONS (0x1337); + RequestOpt.OpLen = 0; // valid length + + UINT8 Index = 0; + + EFI_DHCP6_PACKET *Packet = (EFI_DHCP6_PACKET *)&Private.Dhcp6Request[Index]; + + UINT8 *Cursor = (UINT8 *)(Packet->Dhcp6.Option); + + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt)); + Cursor += sizeof (RequestOpt); + + // Update the packet length + Packet->Length = (UINT16)(Cursor - (UINT8 *)Packet); + Packet->Size = PACKET_SIZE; + + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Private), Index), EFI_SUCCESS); +} + +TEST_F (PxeBcRequestBootServiceTest, AttemptRequestOverFlowExpectFailure) { + EFI_DHCP6_PACKET_OPTION RequestOpt = { 0 }; // the data section doesn't really matter + + RequestOpt.OpCode = HTONS (0x1337); + RequestOpt.OpLen = 1500; // this length would overflow without a check + + UINT8 Index = 0; + + EFI_DHCP6_PACKET *Packet = (EFI_DHCP6_PACKET *)&Private.Dhcp6Request[Index]; + + UINT8 *Cursor = (UINT8 *)(Packet->Dhcp6.Option); + + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt)); + Cursor += sizeof (RequestOpt); + + // Update the packet length + Packet->Length = (UINT16)(Cursor - (UINT8 *)Packet); + Packet->Size = PACKET_SIZE; + + ASSERT_EQ (PxeBcRequestBootService (&(PxeBcRequestBootServiceTest::Private), Index), EFI_OUT_OF_RESOURCES); +} + +/////////////////////////////////////////////////////////////////////////////// +// PxeBcDhcp6Discover Test +/////////////////////////////////////////////////////////////////////////////// + +class PxeBcDhcp6DiscoverTest : public ::testing::Test { +public: +PXEBC_PRIVATE_DATA Private = { 0 }; +EFI_UDP6_PROTOCOL Udp6Read; + +protected: +MockUefiRuntimeServicesTableLib RtServicesMock; + +// Add any setup code if needed +virtual void +SetUp ( + ) +{ + Private.Dhcp6Request = (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 = (EFI_PXE_BASE_CODE_UDP_WRITE)MockUdpWrite; + Private.PxeBc.UdpRead = (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 = (EFI_UDP6_CONFIGURE)MockConfigure; + Private.Udp6Read = &Udp6Read; +} + +// Add any cleanup code if needed +virtual void +TearDown ( + ) +{ + if (Private.Dhcp6Request != NULL) { + FreePool (Private.Dhcp6Request); + } + + // Clean up any resources or variables +} +}; + +// Test Description +// This will cause an overflow by an untrusted packet during the option parsing +TEST_F (PxeBcDhcp6DiscoverTest, BasicOverflowTest) { + EFI_IPv6_ADDRESS DestIp = { 0 }; + EFI_DHCP6_PACKET_OPTION RequestOpt = { 0 }; // the data section doesn't really matter + + RequestOpt.OpCode = HTONS (0x1337); + RequestOpt.OpLen = HTONS (0xFFFF); // overflow + + UINT8 *Cursor = (UINT8 *)(Private.Dhcp6Request->Dhcp6.Option); + + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt)); + Cursor += sizeof (RequestOpt); + + Private.Dhcp6Request->Length = (UINT16)(Cursor - (UINT8 *)Private.Dhcp6Request); + + 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 = { 0 }; + EFI_DHCP6_PACKET_OPTION RequestOpt = { 0 }; // the data section doesn't really matter + + RequestOpt.OpCode = HTONS (0x1337); + RequestOpt.OpLen = HTONS (0x30); + + UINT8 *Cursor = (UINT8 *)(Private.Dhcp6Request->Dhcp6.Option); + + CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt)); + Cursor += sizeof (RequestOpt); + + Private.Dhcp6Request->Length = (UINT16)(Cursor - (UINT8 *)Private.Dhcp6Request); + + EXPECT_CALL (RtServicesMock, gRT_GetTime) + .WillOnce (::testing::Return (0)); + + ASSERT_EQ ( + PxeBcDhcp6Discover ( + &(PxeBcDhcp6DiscoverTest::Private), + 0, + NULL, + FALSE, + (EFI_IP_ADDRESS *)&DestIp + ), + EFI_SUCCESS + ); +} diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h index 724ee468cb..4d40594d51 100644 --- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.h @@ -45,4 +45,22 @@ PxeBcCacheDnsServerAddresses ( IN PXEBC_DHCP6_PACKET_CACHE *Cache6 ); +/** + Build and send out the request packet for the bootfile, and parse the reply. + + @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__ \ No newline at end of file diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp index cc295f5c88..32270d0dcb 100644 --- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.cpp @@ -1,21 +1,21 @@ -/** @file - Acts as the main entry point for the tests for the UefiPxeBcDxe module. - Copyright (c) Microsoft Corporation - SPDX-License-Identifier: BSD-2-Clause-Patent -**/ -#include - -//////////////////////////////////////////////////////////////////////////////// -// Add test files here -// Google Test will only pick up the tests from the files that are included -// here. -//////////////////////////////////////////////////////////////////////////////// -#include "PxeBcDhcp6GoogleTest.cpp" - -//////////////////////////////////////////////////////////////////////////////// -// Run the tests -//////////////////////////////////////////////////////////////////////////////// -int main(int argc, char* argv[]) { - testing::InitGoogleTest (&argc, argv); - return RUN_ALL_TESTS (); -} \ No newline at end of file +/** @file + Acts as the main entry point for the tests for the UefiPxeBcDxe module. + Copyright (c) Microsoft Corporation + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ +#include + +//////////////////////////////////////////////////////////////////////////////// +// Add test files here +// Google Test will only pick up the tests from the files that are included +// here. +//////////////////////////////////////////////////////////////////////////////// +#include "PxeBcDhcp6GoogleTest.cpp" + +//////////////////////////////////////////////////////////////////////////////// +// Run the tests +//////////////////////////////////////////////////////////////////////////////// +int main(int argc, char* argv[]) { + testing::InitGoogleTest (&argc, argv); + return RUN_ALL_TESTS (); +} diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf index c2fd7ae7a9..2d23db0e46 100644 --- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf @@ -1,47 +1,47 @@ -## @file -# Unit test suite for the UefiPxeBcDxe using Google Test -# -# Copyright (c) Microsoft Corporation.
-# SPDX-License-Identifier: BSD-2-Clause-Patent -## -[Defines] -INF_VERSION = 0x00010005 -BASE_NAME = UefiPxeBcDxeGoogleTest -FILE_GUID = 77D45C64-EC1E-4174-887B-886E89FD1EDF -MODULE_TYPE = HOST_APPLICATION -VERSION_STRING = 1.0 - -# -# The following information is for reference only and not required by the build tools. -# -# VALID_ARCHITECTURES = IA32 X64 -# - -[Sources] - UefiPxeBcDxeGoogleTest.cpp - PxeBcDhcp6GoogleTest.cpp - ../PxeBcDhcp6.c - ../PxeBcSupport.c - -[Packages] - MdePkg/MdePkg.dec - MdeModulePkg/MdeModulePkg.dec - UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec - NetworkPkg/NetworkPkg.dec - -[LibraryClasses] - GoogleTestLib - DebugLib - NetLib - PcdLib - -[Protocols] - gEfiDhcp6ServiceBindingProtocolGuid - gEfiDns6ServiceBindingProtocolGuid - gEfiDns6ProtocolGuid - -[Pcd] - gEfiNetworkPkgTokenSpaceGuid.PcdDhcp6UidType - -[Guids] +## @file +# Unit test suite for the UefiPxeBcDxe using Google Test +# +# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent +## +[Defines] +INF_VERSION = 0x00010005 +BASE_NAME = UefiPxeBcDxeGoogleTest +FILE_GUID = 77D45C64-EC1E-4174-887B-886E89FD1EDF +MODULE_TYPE = HOST_APPLICATION +VERSION_STRING = 1.0 + +# +# The following information is for reference only and not required by the build tools. +# +# VALID_ARCHITECTURES = IA32 X64 +# + +[Sources] + UefiPxeBcDxeGoogleTest.cpp + PxeBcDhcp6GoogleTest.cpp + ../PxeBcDhcp6.c + ../PxeBcSupport.c + +[Packages] + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec + UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec + NetworkPkg/NetworkPkg.dec + +[LibraryClasses] + GoogleTestLib + DebugLib + NetLib + PcdLib + +[Protocols] + gEfiDhcp6ServiceBindingProtocolGuid + gEfiDns6ServiceBindingProtocolGuid + gEfiDns6ProtocolGuid + +[Pcd] + gEfiNetworkPkgTokenSpaceGuid.PcdDhcp6UidType + +[Guids] gZeroGuid \ No newline at end of file -- 2.41.0