From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 6EF987803CE for ; Thu, 25 Jan 2024 23:06:43 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=qS/W3kB4B016HJmKzREZ4iYpkxqZfVr9Ql4BKDm9P98=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1706224002; v=1; b=U1/tv2CTbdOLXTdLzFfzv1TJ/OO6bt6w1IX6B7GdtMVHKkOUj+FVmjedUf6YqSBXvvgk1FPN 0NSwkiIyknt52joGSN7Ltngvl78PGprUt2Wt9pg1eoe9kbE3F+DhCHHN3fDhSMkvokD4T0PCqzt X5PjXtej5gotKGzjIsJ9W2bQ= X-Received: by 127.0.0.2 with SMTP id l3W8YY7687511xWbqkoezNFB; Thu, 25 Jan 2024 15:06:42 -0800 X-Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) by mx.groups.io with SMTP id smtpd.web11.791.1706223998545916722 for ; Thu, 25 Jan 2024 15:06:38 -0800 X-Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-1d73066880eso49439855ad.3 for ; Thu, 25 Jan 2024 15:06:38 -0800 (PST) X-Gm-Message-State: OZWYRvcZeJ69RHGWY2vaCE41x7686176AA= X-Google-Smtp-Source: AGHT+IHXT0Z2ylqWpXUTSI80Ih1jWyMD08TJTIwlxFpZXvq9CTrEGXNmXv3Qd2EFnmk12VNmr50x1g== X-Received: by 2002:a17:903:24d:b0:1d4:e0e:fa1b with SMTP id j13-20020a170903024d00b001d40e0efa1bmr571117plh.57.1706223997744; Thu, 25 Jan 2024 15:06:37 -0800 (PST) X-Received: from localhost.localdomain ([24.17.138.83]) by smtp.gmail.com with ESMTPSA id jh1-20020a170903328100b001d752c4f180sm16779plb.94.2024.01.25.15.06.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Jan 2024 15:06:37 -0800 (PST) From: "Doug Flick via groups.io" To: devel@edk2.groups.io Cc: Doug Flick , Saloni Kasbekar , Zachary Clark-williams , "Doug Flick [MSFT]" Subject: [edk2-devel] [PATCH v2 13/15] NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45235 Patch Date: Thu, 25 Jan 2024 13:54:55 -0800 Message-ID: In-Reply-To: References: MIME-Version: 1.0 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,dougflick@microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b="U1/tv2CT"; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io From: Doug Flick REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D4540 Bug Details: PixieFail Bug #7 CVE-2023-45235 CVSS 8.3 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:L/A:H CWE-119 Improper Restriction of Operations within the Bounds of a Memory Buffer Buffer overflow when handling Server ID option from a DHCPv6 proxy Advertise message Change Overview: Performs two checks 1. Checks that the length of the duid is accurate > + // > + // Check that the minimum and maximum requirements are met > + // > + if ((OpLen < PXEBC_MIN_SIZE_OF_DUID) || (OpLen > PXEBC_MAX_SIZE_OF_DUID)) { > + Status =3D EFI_INVALID_PARAMETER; > + goto ON_ERROR; > + } 2. Ensures that the amount of data written to the buffer is tracked and never exceeds that > + // > + // Check that the option length is valid. > + // > + if ((DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN) > DiscoverLenNeeded) { > + Status =3D EFI_OUT_OF_RESOURCES; > + goto ON_ERROR; > + } Additional code clean up and fix for memory leak in case Option was NULL Cc: Saloni Kasbekar Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] --- NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h | 17 ++++++ NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 77 ++++++++++++++++++++++------ 2 files changed, 78 insertions(+), 16 deletions(-) diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h b/NetworkPkg/UefiPxeBcDxe= /PxeBcDhcp6.h index c86f6d391b80..6357d27faefd 100644 --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h @@ -34,6 +34,23 @@ #define PXEBC_ADDR_START_DELIMITER '['=0D #define PXEBC_ADDR_END_DELIMITER ']'=0D =0D +//=0D +// A DUID consists of a 2-octet type code represented in network byte=0D +// order, followed by a variable number of octets that make up the=0D +// actual identifier. The length of the DUID (not including the type=0D +// code) is at least 1 octet and at most 128 octets.=0D +//=0D +#define PXEBC_MIN_SIZE_OF_DUID (sizeof(UINT16) + 1)=0D +#define PXEBC_MAX_SIZE_OF_DUID (sizeof(UINT16) + 128)=0D +=0D +//=0D +// This define represents the combineds code and length field from=0D +// https://datatracker.ietf.org/doc/html/rfc3315#section-22.1=0D +//=0D +#define PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN \=0D + (sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpCode) + \=0D + sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpLen))=0D +=0D #define GET_NEXT_DHCP6_OPTION(Opt) \=0D (EFI_DHCP6_PACKET_OPTION *) ((UINT8 *) (Opt) + \=0D sizeof (EFI_DHCP6_PACKET_OPTION) + (NTOHS ((Opt)->OpLen)) - 1)=0D diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c b/NetworkPkg/UefiPxeBcDxe= /PxeBcDhcp6.c index 2b2d372889a3..7fd1281c1184 100644 --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c @@ -887,6 +887,7 @@ PxeBcRequestBootService ( EFI_STATUS Status;=0D EFI_DHCP6_PACKET *IndexOffer;=0D UINT8 *Option;=0D + UINTN DiscoverLenNeeded;=0D =0D PxeBc =3D &Private->PxeBc;=0D Request =3D Private->Dhcp6Request;=0D @@ -899,7 +900,8 @@ PxeBcRequestBootService ( return EFI_DEVICE_ERROR;=0D }=0D =0D - Discover =3D AllocateZeroPool (sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET))= ;=0D + DiscoverLenNeeded =3D sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET);=0D + Discover =3D AllocateZeroPool (DiscoverLenNeeded);=0D if (Discover =3D=3D NULL) {=0D return EFI_OUT_OF_RESOURCES;=0D }=0D @@ -924,16 +926,34 @@ PxeBcRequestBootService ( DHCP6_OPT_SERVER_ID=0D );=0D if (Option =3D=3D NULL) {=0D - return EFI_NOT_FOUND;=0D + Status =3D EFI_NOT_FOUND;=0D + goto ON_ERROR;=0D }=0D =0D //=0D // Add Server ID Option.=0D //=0D OpLen =3D NTOHS (((EFI_DHCP6_PACKET_OPTION *)Option)->OpLen);=0D - CopyMem (DiscoverOpt, Option, OpLen + 4);=0D - DiscoverOpt +=3D (OpLen + 4);=0D - DiscoverLen +=3D (OpLen + 4);=0D +=0D + //=0D + // Check that the minimum and maximum requirements are met=0D + //=0D + if ((OpLen < PXEBC_MIN_SIZE_OF_DUID) || (OpLen > PXEBC_MAX_SIZE_OF_DUI= D)) {=0D + Status =3D EFI_INVALID_PARAMETER;=0D + goto ON_ERROR;=0D + }=0D +=0D + //=0D + // Check that the option length is valid.=0D + //=0D + if ((DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN) > = DiscoverLenNeeded) {=0D + Status =3D EFI_OUT_OF_RESOURCES;=0D + goto ON_ERROR;=0D + }=0D +=0D + CopyMem (DiscoverOpt, Option, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_= AND_LEN);=0D + DiscoverOpt +=3D (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);=0D + DiscoverLen +=3D (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);=0D }=0D =0D while (RequestLen < Request->Length) {=0D @@ -944,16 +964,24 @@ PxeBcRequestBootService ( (OpCode !=3D DHCP6_OPT_SERVER_ID)=0D )=0D {=0D + //=0D + // Check that the option length is valid.=0D + //=0D + if (DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN > = DiscoverLenNeeded) {=0D + Status =3D EFI_OUT_OF_RESOURCES;=0D + goto ON_ERROR;=0D + }=0D +=0D //=0D // Copy all the options except IA option and Server ID=0D //=0D - CopyMem (DiscoverOpt, RequestOpt, OpLen + 4);=0D - DiscoverOpt +=3D (OpLen + 4);=0D - DiscoverLen +=3D (OpLen + 4);=0D + CopyMem (DiscoverOpt, RequestOpt, OpLen + PXEBC_COMBINED_SIZE_OF_OPT= _CODE_AND_LEN);=0D + DiscoverOpt +=3D (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);= =0D + DiscoverLen +=3D (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);= =0D }=0D =0D - RequestOpt +=3D (OpLen + 4);=0D - RequestLen +=3D (OpLen + 4);=0D + RequestOpt +=3D (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);=0D + RequestLen +=3D (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);=0D }=0D =0D //=0D @@ -2154,6 +2182,7 @@ PxeBcDhcp6Discover ( UINT16 OpLen;=0D UINT32 Xid;=0D EFI_STATUS Status;=0D + UINTN DiscoverLenNeeded;=0D =0D PxeBc =3D &Private->PxeBc;=0D Mode =3D PxeBc->Mode;=0D @@ -2169,7 +2198,8 @@ PxeBcDhcp6Discover ( return EFI_DEVICE_ERROR;=0D }=0D =0D - Discover =3D AllocateZeroPool (sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET))= ;=0D + DiscoverLenNeeded =3D sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET);=0D + Discover =3D AllocateZeroPool (DiscoverLenNeeded);=0D if (Discover =3D=3D NULL) {=0D return EFI_OUT_OF_RESOURCES;=0D }=0D @@ -2185,22 +2215,37 @@ PxeBcDhcp6Discover ( DiscoverLen =3D sizeof (EFI_DHCP6_HEADER);=0D RequestLen =3D DiscoverLen;=0D =0D + //=0D + // The request packet is generated by the UEFI network stack. In the DHC= P4 DORA and DHCP6 SARR sequence,=0D + // the first (discover in DHCP4 and solicit in DHCP6) and third (request= in both DHCP4 and DHCP6) are=0D + // generated by the DHCP client (the UEFI network stack in this case). B= y the time this function executes,=0D + // the DHCP sequence already has been executed once (see UEFI Specificat= ion Figures 24.2 and 24.3), with=0D + // Private->Dhcp6Request being a cached copy of the DHCP6 request packet= that UEFI network stack previously=0D + // generated and sent.=0D + //=0D + // Therefore while this code looks like it could overflow, in practice i= t's not possible.=0D + //=0D while (RequestLen < Request->Length) {=0D OpCode =3D NTOHS (((EFI_DHCP6_PACKET_OPTION *)RequestOpt)->OpCode);=0D OpLen =3D NTOHS (((EFI_DHCP6_PACKET_OPTION *)RequestOpt)->OpLen);=0D if ((OpCode !=3D EFI_DHCP6_IA_TYPE_NA) &&=0D (OpCode !=3D EFI_DHCP6_IA_TYPE_TA))=0D {=0D + if (DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN > = DiscoverLenNeeded) {=0D + Status =3D EFI_OUT_OF_RESOURCES;=0D + goto ON_ERROR;=0D + }=0D +=0D //=0D // Copy all the options except IA option.=0D //=0D - CopyMem (DiscoverOpt, RequestOpt, OpLen + 4);=0D - DiscoverOpt +=3D (OpLen + 4);=0D - DiscoverLen +=3D (OpLen + 4);=0D + CopyMem (DiscoverOpt, RequestOpt, OpLen + PXEBC_COMBINED_SIZE_OF_OPT= _CODE_AND_LEN);=0D + DiscoverOpt +=3D (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);= =0D + DiscoverLen +=3D (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);= =0D }=0D =0D - RequestOpt +=3D (OpLen + 4);=0D - RequestLen +=3D (OpLen + 4);=0D + RequestOpt +=3D (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);=0D + RequestLen +=3D (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);=0D }=0D =0D Status =3D PxeBc->UdpWrite (=0D --=20 2.43.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114475): https://edk2.groups.io/g/devel/message/114475 Mute This Topic: https://groups.io/mt/103964991/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-