Skip to content

Commit

Permalink
NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45230 Patch
Browse files Browse the repository at this point in the history
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4535

Bug Details:
PixieFail Bug #2
CVE-2023-45230
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

Changes Overview:
> -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
>    );

Dhcp6AppendOption() and variants can return errors now.  All callsites
are adapted accordingly.

It gets passed in EFI_DHCP6_PACKET as additional 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;
> +  }

... so it can look at Packet->Size when checking buffer space.
Also to allow Packet->Length updates.

Lots of checks added.

Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
Cc: Zachary Clark-williams <zachary.clark-williams@intel.com>

Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
Reviewed-by: Jeff Brasen <jbrasen@nvidia.com>
Tested-by: Jeff Brasen <jbrasen@nvidia.com>
  • Loading branch information
Doug Flick via groups.io authored and jgarver committed Feb 29, 2024
1 parent 9c9d01e commit ed83194
Show file tree
Hide file tree
Showing 4 changed files with 668 additions and 239 deletions.
43 changes: 43 additions & 0 deletions NetworkPkg/Dhcp6Dxe/Dhcp6Impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,49 @@ 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
Expand Down
Loading

0 comments on commit ed83194

Please sign in to comment.