Skip to content

Commit 20b8878

Browse files
FlickdmDouglas Flick [MSFT]
authored and
Douglas Flick [MSFT]
committed
NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45234 Patch
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4539 Bug Details: PixieFail Bug tianocore#6 CVE-2023-45234 CVSS 8.3 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:L/A:H CWE-119 Improper Restriction of Operations within the Bounds of a Memory Buffer Buffer overflow when processing DNS Servers option in a DHCPv6 Advertise message Change Overview: Introduces a function to cache the Dns Server and perform sanitizing on the incoming DnsServerLen to ensure that the length is valid > + EFI_STATUS > + PxeBcCacheDnsServerAddresses ( > + IN PXEBC_PRIVATE_DATA *Private, > + IN PXEBC_DHCP6_PACKET_CACHE *Cache6 > + ) Additional code cleanup Cc: Saloni Kasbekar <[email protected]> Cc: Zachary Clark-williams <[email protected]> Signed-off-by: Doug Flick [MSFT] <[email protected]>
1 parent 775863c commit 20b8878

File tree

1 file changed

+65
-6
lines changed

1 file changed

+65
-6
lines changed

NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c

+65-6
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
44
(C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
55
Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
6+
Copyright (c) Microsoft Corporation
67
78
SPDX-License-Identifier: BSD-2-Clause-Patent
89
@@ -1312,6 +1313,65 @@ PxeBcSelectDhcp6Offer (
13121313
}
13131314
}
13141315

1316+
/**
1317+
Cache the DHCPv6 DNS Server addresses
1318+
1319+
@param[in] Private The pointer to PXEBC_PRIVATE_DATA.
1320+
@param[in] Cache6 The pointer to PXEBC_DHCP6_PACKET_CACHE.
1321+
1322+
@retval EFI_SUCCESS Cache the DHCPv6 DNS Server address successfully.
1323+
@retval EFI_OUT_OF_RESOURCES Failed to allocate resources.
1324+
@retval EFI_DEVICE_ERROR The DNS Server Address Length provided by a untrusted
1325+
option is not a multiple of 16 bytes (sizeof (EFI_IPv6_ADDRESS)).
1326+
**/
1327+
EFI_STATUS
1328+
PxeBcCacheDnsServerAddresses (
1329+
IN PXEBC_PRIVATE_DATA *Private,
1330+
IN PXEBC_DHCP6_PACKET_CACHE *Cache6
1331+
)
1332+
{
1333+
UINT16 DnsServerLen;
1334+
1335+
DnsServerLen = NTOHS (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen);
1336+
//
1337+
// Make sure that the number is nonzero
1338+
//
1339+
if (DnsServerLen == 0) {
1340+
return EFI_DEVICE_ERROR;
1341+
}
1342+
1343+
//
1344+
// Make sure the DnsServerlen is a multiple of EFI_IPv6_ADDRESS (16)
1345+
//
1346+
if (DnsServerLen % sizeof (EFI_IPv6_ADDRESS) != 0) {
1347+
return EFI_DEVICE_ERROR;
1348+
}
1349+
1350+
//
1351+
// This code is currently written to only support a single DNS Server instead
1352+
// of multiple such as is spec defined (RFC3646, Section 3). The proper behavior
1353+
// would be to allocate the full space requested, CopyMem all of the data,
1354+
// and then add a DnsServerCount field to Private and update additional code
1355+
// that depends on this.
1356+
//
1357+
// To support multiple DNS servers the `AllocationSize` would need to be changed to DnsServerLen
1358+
//
1359+
// This is tracked in https://bugzilla.tianocore.org/show_bug.cgi?id=1886
1360+
//
1361+
Private->DnsServer = AllocateZeroPool (sizeof (EFI_IPv6_ADDRESS));
1362+
if (Private->DnsServer == NULL) {
1363+
return EFI_OUT_OF_RESOURCES;
1364+
}
1365+
1366+
//
1367+
// Intentionally only copy over the first server address.
1368+
// To support multiple DNS servers, the `Length` would need to be changed to DnsServerLen
1369+
//
1370+
CopyMem (Private->DnsServer, Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, sizeof (EFI_IPv6_ADDRESS));
1371+
1372+
return EFI_SUCCESS;
1373+
}
1374+
13151375
/**
13161376
Handle the DHCPv6 offer packet.
13171377
@@ -1335,22 +1395,21 @@ PxeBcHandleDhcp6Offer (
13351395
UINT32 SelectIndex;
13361396
UINT32 Index;
13371397

1398+
ASSERT (Private != NULL);
13381399
ASSERT (Private->SelectIndex > 0);
13391400
SelectIndex = (UINT32)(Private->SelectIndex - 1);
13401401
ASSERT (SelectIndex < PXEBC_OFFER_MAX_NUM);
13411402
Cache6 = &Private->OfferBuffer[SelectIndex].Dhcp6;
13421403
Status = EFI_SUCCESS;
13431404

13441405
//
1345-
// First try to cache DNS server address if DHCP6 offer provides.
1406+
// First try to cache DNS server addresses if DHCP6 offer provides.
13461407
//
13471408
if (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER] != NULL) {
1348-
Private->DnsServer = AllocateZeroPool (NTOHS (Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->OpLen));
1349-
if (Private->DnsServer == NULL) {
1350-
return EFI_OUT_OF_RESOURCES;
1409+
Status = PxeBcCacheDnsServerAddresses (Private, Cache6);
1410+
if (EFI_ERROR (Status)) {
1411+
return Status;
13511412
}
1352-
1353-
CopyMem (Private->DnsServer, Cache6->OptList[PXEBC_DHCP6_IDX_DNS_SERVER]->Data, sizeof (EFI_IPv6_ADDRESS));
13541413
}
13551414

13561415
if (Cache6->OfferType == PxeOfferTypeDhcpBinl) {

0 commit comments

Comments
 (0)