Re: [edk2-devel] [PATCH v2 04/15] NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Patch

["bryan-bt.tan via groups.io" <bryan-bt.tan=broadcom.com@groups.io>]

[-- Attachment #1: Type: text/plain, Size: 2527 bytes --]

On Thu, Jan 25, 2024 at 11:06 PM, Doug Flick wrote:

> 
> <snip>
> @@ -641,7 +642,7 @@ Dhcp6UpdateIaInfo (
> Option =3D Dhcp6SeekOption (IaInnerOpt, IaInnerLen, Dhcp6OptStatusCode)=
> ;=0D
> =0D
> if (Option !=3D NULL) {=0D
> - StsCode =3D NTOHS (ReadUnaligned16 ((UINT16 *)(Option + 4)));=0D
> + StsCode =3D NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN=
> (Option))));=0D

This should be DHCP6_OFFSET_OF_STATUS_CODE.

> 
> <snip>
> @@ -715,52 +803,35 @@ Dhcp6SeekStsOption (
> }=0D
> =0D
> //=0D
> - // The format of the IA_NA option is:=0D
> + // Calculate the distance from Packet->Dhcp6.Option to the IA option.=0D
> 
> //=0D
> - // 0 1 2 3=0D
> - // 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=0D
> - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+=
> =0D
> - // | OPTION_IA_NA | option-len |=
> =0D
> - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+=
> =0D
> - // | IAID (4 octets) |=
> =0D
> - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+=
> =0D
> - // | T1 |=
> =0D
> - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+=
> =0D
> - // | T2 |=
> =0D
> - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+=
> =0D
> - // | |=
> =0D
> - // . IA_NA-options .=
> =0D
> - // . .=
> =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
> - // The format of the IA_TA option is:=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

Probably just my lack of understanding on DHCP, but earlier you do
`OptionLen = Packet->Length - sizeof (Packet->Dhcp6.Header);`
which seems to imply Packet->Length includes the DHCP header.
Am I missing something here?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115121): https://edk2.groups.io/g/devel/message/115121
Mute This Topic: https://groups.io/mt/103964979/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 3121 bytes --]