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 22ABB7803D2 for ; Tue, 13 Feb 2024 19:41:09 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=5MUCfQ/Dez5iyM3Z/N3aiB9aSO+v1uXJYZe/CK8l+ug=; 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=1707853268; v=1; b=ws6pW5ZXWCUjfNsh6vOFc6LvxrCmOaHxZQ2XDLzEBEhKoDk9VWjqEu7nPdtWO+AfP6a+/ay2 G0ZdUlZZJyGS7RuQFQfW1xyYxkKBF+pfniqAf5A3m5gS3SuHfcPcabn5/69j/8Tdc+E6niMcS0m TUw3BBqS/nq15ze/Ff9hZ4eY= X-Received: by 127.0.0.2 with SMTP id Re9RYY7687511xnCWXUbEQ4V; Tue, 13 Feb 2024 11:41:08 -0800 X-Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) by mx.groups.io with SMTP id smtpd.web11.23013.1707853268097937027 for ; Tue, 13 Feb 2024 11:41:08 -0800 X-Received: by mail-pl1-f174.google.com with SMTP id d9443c01a7336-1d73066880eso43609845ad.3 for ; Tue, 13 Feb 2024 11:41:08 -0800 (PST) X-Gm-Message-State: O0MmeaeWGG3hSuKC1VgrTygLx7686176AA= X-Google-Smtp-Source: AGHT+IHwnuv82DB7R86rUKd+LUJUrZECCuFYrX5Y/dCAWVkN/jvqYiUDa3/RedW6wvERGBkFrHKDzw== X-Received: by 2002:a17:902:d502:b0:1d9:a148:49c with SMTP id b2-20020a170902d50200b001d9a148049cmr466560plg.57.1707853267383; Tue, 13 Feb 2024 11:41:07 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCV1ObB67S6P20AtVJwjZX930JmnTagZqhUoBzqwmi6H5heXkWvZ74sH1pi9P0E9i2RhuP5IWyqfS15yCEhlUzcADWzhc+TtyPw0oXQ9WEyMsbvvtK1glCtYf2DyJkuBIQHFGVKtBcynakv7aDcvpA/TURfzyiK0d8CHZnTVPbxfVA== X-Received: from localhost.localdomain ([131.107.147.247]) by smtp.gmail.com with ESMTPSA id ks6-20020a170903084600b001d9588f0714sm2436189plb.177.2024.02.13.11.41.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Feb 2024 11:41:06 -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 1/4] NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Related Patch Date: Tue, 13 Feb 2024 10:46:00 -0800 Message-Id: <20240213184603.2985-2-doug.edk2@gmail.com> In-Reply-To: <20240213184603.2985-1-doug.edk2@gmail.com> References: <20240213184603.2985-1-doug.edk2@gmail.com> 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=ws6pW5ZX; dmarc=pass (policy=none) header.from=groups.io; 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=3D4673 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4534 This was not part of the Quarkslab bugs however the same pattern as CVE-2023-45229 exists in Dhcp6UpdateIaInfo. This patch replaces the code in question with the safe function created to patch CVE-2023-45229 > > if (EFI_ERROR ( > Dhcp6SeekInnerOptionSafe ( > Instance->Config->IaDescriptor.Type, > Option, > OptionLen, > &IaInnerOpt, > &IaInnerLen > ) > )) > { > return EFI_DEVICE_ERROR; > } > Additionally corrects incorrect usage of macro to read the status > - StsCode =3D NTOHS (ReadUnaligned16 ((UINT16 *)DHCP6_OFFSET_OF_OPT_LEN (Option))); > + StsCode =3D NTOHS (ReadUnaligned16 ((UINT16 *) DHCP6_OFFSET_OF_STATUS_CODE (Option)); Cc: Saloni Kasbekar Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] --- NetworkPkg/Dhcp6Dxe/Dhcp6Io.h | 22 ++++++ NetworkPkg/Dhcp6Dxe/Dhcp6Io.c | 70 +++++++++++++++----- 2 files changed, 75 insertions(+), 17 deletions(-) diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.h b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.h index 051a652f2b1f..ab0e1ac27f10 100644 --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.h +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.h @@ -217,4 +217,26 @@ Dhcp6OnTimerTick ( IN VOID *Context=0D );=0D =0D +/**=0D + Seeks the Inner Options from a DHCP6 Option=0D +=0D + @param[in] IaType The type of the IA option.=0D + @param[in] Option The pointer to the DHCP6 Option.=0D + @param[in] OptionLen The length of the DHCP6 Option.=0D + @param[out] IaInnerOpt The pointer to the IA inner option.=0D + @param[out] IaInnerLen The length of the IA inner option.=0D +=0D + @retval EFI_SUCCESS Seek the inner option successfully.=0D + @retval EFI_DEVICE_ERROR The OptionLen is invalid. On Error,=0D + the pointers are not modified=0D +**/=0D +EFI_STATUS=0D +Dhcp6SeekInnerOptionSafe (=0D + IN UINT16 IaType,=0D + IN UINT8 *Option,=0D + IN UINT32 OptionLen,=0D + OUT UINT8 **IaInnerOpt,=0D + OUT UINT16 *IaInnerLen=0D + );=0D +=0D #endif=0D diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c index 3b8feb4a2032..a9bffae353d7 100644 --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c @@ -528,13 +528,23 @@ Dhcp6UpdateIaInfo ( {=0D EFI_STATUS Status;=0D UINT8 *Option;=0D + UINT32 OptionLen;=0D UINT8 *IaInnerOpt;=0D UINT16 IaInnerLen;=0D UINT16 StsCode;=0D UINT32 T1;=0D UINT32 T2;=0D =0D + T1 =3D 0;=0D + T2 =3D 0;=0D +=0D ASSERT (Instance->Config !=3D NULL);=0D +=0D + // OptionLen is the length of the Options excluding the DHCP header.=0D + // Length of the EFI_DHCP6_PACKET from the first byte of the Header fiel= d to the last=0D + // byte of the Option[] field.=0D + OptionLen =3D Packet->Length - sizeof (Packet->Dhcp6.Header);=0D +=0D //=0D // If the reply was received in response to a solicit with rapid commit = option,=0D // request, renew or rebind message, the client updates the information = it has=0D @@ -549,13 +559,29 @@ Dhcp6UpdateIaInfo ( //=0D Option =3D Dhcp6SeekIaOption (=0D Packet->Dhcp6.Option,=0D - Packet->Length - sizeof (EFI_DHCP6_HEADER),=0D + OptionLen,=0D &Instance->Config->IaDescriptor=0D );=0D if (Option =3D=3D NULL) {=0D return EFI_DEVICE_ERROR;=0D }=0D =0D + //=0D + // Calculate the distance from Packet->Dhcp6.Option to the IA option.=0D + //=0D + // Packet->Size and Packet->Length are both UINT32 type, and Packet->Siz= e is=0D + // the size of the whole packet, including the DHCP header, and Packet->= Length=0D + // is the length of the DHCP message body, excluding the DHCP header.=0D + //=0D + // (*Option - Packet->Dhcp6.Option) is the number of bytes from the star= t of=0D + // DHCP6 option area to the start of the IA option.=0D + //=0D + // Dhcp6SeekInnerOptionSafe() is searching starting from the start of th= e=0D + // IA option to the end of the DHCP6 option area, thus subtract the spac= e=0D + // up until this option=0D + //=0D + OptionLen =3D OptionLen - (UINT32)(Option - Packet->Dhcp6.Option);=0D +=0D //=0D // The format of the IA_NA option is:=0D //=0D @@ -591,32 +617,32 @@ Dhcp6UpdateIaInfo ( //=0D =0D //=0D - // sizeof (option-code + option-len + IaId) =3D 8=0D - // sizeof (option-code + option-len + IaId + T1) =3D 12=0D - // sizeof (option-code + option-len + IaId + T1 + T2) =3D 16=0D - //=0D - // The inner options still start with 2 bytes option-code and 2 bytes op= tion-len.=0D + // Seek the inner option=0D //=0D + if (EFI_ERROR (=0D + Dhcp6SeekInnerOptionSafe (=0D + Instance->Config->IaDescriptor.Type,=0D + Option,=0D + OptionLen,=0D + &IaInnerOpt,=0D + &IaInnerLen=0D + )=0D + ))=0D + {=0D + return EFI_DEVICE_ERROR;=0D + }=0D +=0D if (Instance->Config->IaDescriptor.Type =3D=3D Dhcp6OptIana) {=0D T1 =3D NTOHL (ReadUnaligned32 ((UINT32 *)(DHCP6_OFFSET_OF_IA_NA_T1 (Op= tion))));=0D T2 =3D NTOHL (ReadUnaligned32 ((UINT32 *)(DHCP6_OFFSET_OF_IA_NA_T2 (Op= tion))));=0D //=0D // Refer to RFC3155 Chapter 22.4. If a client receives an IA_NA with T= 1 greater than T2,=0D // and both T1 and T2 are greater than 0, the client discards the IA_N= A option and processes=0D - // the remainder of the message as though the server had not included= the invalid IA_NA option.=0D + // the remainder of the message as though the server had not included = the invalid IA_NA option.=0D //=0D if ((T1 > T2) && (T2 > 0)) {=0D return EFI_DEVICE_ERROR;=0D }=0D -=0D - IaInnerOpt =3D DHCP6_OFFSET_OF_IA_NA_INNER_OPT (Option);=0D - IaInnerLen =3D (UINT16)(NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSE= T_OF_OPT_LEN (Option)))) - DHCP6_SIZE_OF_COMBINED_IAID_T1_T2);=0D - } else {=0D - T1 =3D 0;=0D - T2 =3D 0;=0D -=0D - IaInnerOpt =3D DHCP6_OFFSET_OF_IA_TA_INNER_OPT (Option);=0D - IaInnerLen =3D (UINT16)(NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSE= T_OF_OPT_LEN (Option)))) - DHCP6_SIZE_OF_IAID);=0D }=0D =0D //=0D @@ -642,7 +668,7 @@ Dhcp6UpdateIaInfo ( Option =3D Dhcp6SeekOption (IaInnerOpt, IaInnerLen, Dhcp6OptStatusCode)= ;=0D =0D if (Option !=3D NULL) {=0D - StsCode =3D NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN= (Option))));=0D + StsCode =3D NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_STATUS_= CODE (Option))));=0D if (StsCode !=3D Dhcp6StsSuccess) {=0D return EFI_DEVICE_ERROR;=0D }=0D @@ -703,15 +729,21 @@ Dhcp6SeekInnerOptionSafe ( }=0D =0D if (IaType =3D=3D Dhcp6OptIana) {=0D + //=0D // Verify we have a fully formed IA_NA=0D + //=0D if (OptionLen < DHCP6_MIN_SIZE_OF_IA_NA) {=0D return EFI_DEVICE_ERROR;=0D }=0D =0D + //=0D + // Get the IA Inner Option and Length=0D //=0D IaInnerOptTmp =3D DHCP6_OFFSET_OF_IA_NA_INNER_OPT (Option);=0D =0D + //=0D // Verify the IaInnerLen is valid.=0D + //=0D IaInnerLenTmp =3D (UINT16)NTOHS (ReadUnaligned16 ((UINT16 *)DHCP6_OFFS= ET_OF_OPT_LEN (Option)));=0D if (IaInnerLenTmp < DHCP6_SIZE_OF_COMBINED_IAID_T1_T2) {=0D return EFI_DEVICE_ERROR;=0D @@ -719,14 +751,18 @@ Dhcp6SeekInnerOptionSafe ( =0D IaInnerLenTmp -=3D DHCP6_SIZE_OF_COMBINED_IAID_T1_T2;=0D } else if (IaType =3D=3D Dhcp6OptIata) {=0D + //=0D // Verify the OptionLen is valid.=0D + //=0D if (OptionLen < DHCP6_MIN_SIZE_OF_IA_TA) {=0D return EFI_DEVICE_ERROR;=0D }=0D =0D IaInnerOptTmp =3D DHCP6_OFFSET_OF_IA_TA_INNER_OPT (Option);=0D =0D + //=0D // Verify the IaInnerLen is valid.=0D + //=0D IaInnerLenTmp =3D (UINT16)NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFF= SET_OF_OPT_LEN (Option))));=0D if (IaInnerLenTmp < DHCP6_SIZE_OF_IAID) {=0D return EFI_DEVICE_ERROR;=0D --=20 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115407): https://edk2.groups.io/g/devel/message/115407 Mute This Topic: https://groups.io/mt/104339706/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-