Skip to content

Commit

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

PixieFail Bug #4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug #5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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
@Flickdm @jgarver
Flickdm authored and jgarver committed Feb 9, 2024
1 parent 6d52cf0 commit e257327
Show file tree
Hide file tree
Showing 3 changed files with 171 additions and 11 deletions.
35 changes: 35 additions & 0 deletions NetworkPkg/Ip6Dxe/Ip6Nd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,48 @@ VOID
VOID *Context
);

//
// Per RFC8200 Section 4.2
//
// Two of the currently-defined extension headers -- the Hop-by-Hop
// Options header and the Destination Options header -- carry a variable
// number of type-length-value (TLV) encoded "options", of the following
// format:
//
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+- - - - - - - - -
// | Option Type | Opt Data Len | Option Data
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+- - - - - - - - -
//
// Option Type 8-bit identifier of the type of option.
//
// Opt Data Len 8-bit unsigned integer. Length of the Option
// Data field of this option, in octets.
//
// Option Data Variable-length field. Option-Type-specific
// data.
//
typedef struct _IP6_OPTION_HEADER {
///
/// identifier of the type of option.
///
UINT8 Type;
///
/// Length of the Option Data field of this option, in octets.
///
UINT8 Length;
///
/// Option-Type-specific data.
///
} IP6_OPTION_HEADER;

STATIC_ASSERT (sizeof (IP6_OPTION_HEADER) == 2, "IP6_OPTION_HEADER is expected to be exactly 2 bytes long.");

#define IP6_NEXT_OPTION_OFFSET(offset, length) (offset + sizeof(IP6_OPTION_HEADER) + length)
STATIC_ASSERT (
IP6_NEXT_OPTION_OFFSET (0, 0) == 2,
"The next option is minimally the combined size of the option tag and length"
);

typedef struct _IP6_ETHE_ADDR_OPTION {
UINT8 Type;
UINT8 Length;
Expand Down
76 changes: 65 additions & 11 deletions NetworkPkg/Ip6Dxe/Ip6Option.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
@param[in] IpSb The IP6 service data.
@param[in] Packet The to be validated packet.
@param[in] Option The first byte of the option.
@param[in] OptionLen The length of the whole option.
@param[in] OptionLen The length of all options, expressed in byte length of octets.
Maximum length is 2046 bytes or ((n + 1) * 8) - 2 where n is 255.
@param[in] Pointer Identifies the octet offset within
the invoking packet where the error was detected.
Expand All @@ -31,12 +32,33 @@ Ip6IsOptionValid (
IN IP6_SERVICE *IpSb,
IN NET_BUF *Packet,
IN UINT8 *Option,
IN UINT8 OptionLen,
IN UINT16 OptionLen,
IN UINT32 Pointer
)
{
UINT8 Offset;
UINT8 OptionType;
UINT16 Offset;
UINT8 OptionType;
UINT8 OptDataLen;

if (Option == NULL) {
ASSERT (Option != NULL);
return FALSE;
}

if ((OptionLen <= 0) || (OptionLen > IP6_MAX_EXT_DATA_LENGTH)) {
ASSERT (OptionLen > 0 && OptionLen <= IP6_MAX_EXT_DATA_LENGTH);
return FALSE;
}

if (Packet == NULL) {
ASSERT (Packet != NULL);
return FALSE;
}

if (IpSb == NULL) {
ASSERT (IpSb != NULL);
return FALSE;
}

Offset = 0;

Expand All @@ -54,7 +76,8 @@ Ip6IsOptionValid (
//
// It is a PadN option
//
Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
OptDataLen = ((IP6_OPTION_HEADER *)(Option + Offset))->Length;
Offset = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);
break;
case Ip6OptionRouterAlert:
//
Expand All @@ -69,7 +92,8 @@ Ip6IsOptionValid (
//
switch (OptionType & Ip6OptionMask) {
case Ip6OptionSkip:
Offset = (UINT8)(Offset + *(Option + Offset + 1));
OptDataLen = ((IP6_OPTION_HEADER *)(Option + Offset))->Length;
Offset = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);
break;
case Ip6OptionDiscard:
return FALSE;
Expand Down Expand Up @@ -308,7 +332,7 @@ Ip6IsExtsValid (
UINT32 Pointer;
UINT32 Offset;
UINT8 *Option;
UINT8 OptionLen;
UINT16 OptionLen;
BOOLEAN Flag;
UINT8 CountD;
UINT8 CountA;
Expand Down Expand Up @@ -385,6 +409,36 @@ Ip6IsExtsValid (
// Fall through
//
case IP6_DESTINATION:
//
// See https://www.rfc-editor.org/rfc/rfc2460#section-4.2 page 23
//
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
// | Next Header | Hdr Ext Len | |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +
// | |
// . .
// . Options .
// . .
// | |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
//
//
// Next Header 8-bit selector. Identifies the type of header
// immediately following the Destination Options
// header. Uses the same values as the IPv4
// Protocol field [RFC-1700 et seq.].
//
// Hdr Ext Len 8-bit unsigned integer. Length of the
// Destination Options header in 8-octet units, not
// including the first 8 octets.
//
// Options Variable-length field, of length such that the
// complete Destination Options header is an
// integer multiple of 8 octets long. Contains one
// or more TLV-encoded options, as described in
// section 4.2.
//

if (*NextHeader == IP6_DESTINATION) {
CountD++;
}
Expand All @@ -398,7 +452,7 @@ Ip6IsExtsValid (

Offset++;
Option = ExtHdrs + Offset;
OptionLen = (UINT8)((*Option + 1) * 8 - 2);
OptionLen = IP6_HDR_EXT_LEN (*Option) - sizeof (IP6_EXT_HDR);
Option++;
Offset++;

Expand Down Expand Up @@ -430,7 +484,7 @@ Ip6IsExtsValid (
//
// Ignore the routing header and proceed to process the next header.
//
Offset = Offset + (RoutingHead->HeaderLen + 1) * 8;
Offset = Offset + IP6_HDR_EXT_LEN (RoutingHead->HeaderLen);

if (UnFragmentLen != NULL) {
*UnFragmentLen = Offset;
Expand All @@ -441,7 +495,7 @@ Ip6IsExtsValid (
// to the packet's source address, pointing to the unrecognized routing
// type.
//
Pointer = Offset + 2 + sizeof (EFI_IP6_HEADER);
Pointer = Offset + sizeof (IP6_EXT_HDR) + sizeof (EFI_IP6_HEADER);
if ((IpSb != NULL) && (Packet != NULL) &&
!IP6_IS_MULTICAST (&Packet->Ip.Ip6->DestinationAddress))
{
Expand Down Expand Up @@ -527,7 +581,7 @@ Ip6IsExtsValid (
//
// RFC2402, Payload length is specified in 32-bit words, minus "2".
//
OptionLen = (UINT8)((*Option + 2) * 4);
OptionLen = ((UINT16)(*Option + 2) * 4);
Offset = Offset + OptionLen;
break;

Expand Down
71 changes: 71 additions & 0 deletions NetworkPkg/Ip6Dxe/Ip6Option.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,77 @@

#define IP6_FRAGMENT_OFFSET_MASK (~0x3)

//
// For more information see RFC 8200, Section 4.3, 4.4, and 4.6
//
// This example format is from section 4.6
// This does not apply to fragment headers
//
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
// | Next Header | Hdr Ext Len | |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +
// | |
// . .
// . Header-Specific Data .
// . .
// | |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
//
// Next Header 8-bit selector. Identifies the type of
// header immediately following the extension
// header. Uses the same values as the IPv4
// Protocol field [IANA-PN].
//
// Hdr Ext Len 8-bit unsigned integer. Length of the
// Destination Options header in 8-octet units,
// not including the first 8 octets.

//
// These defines apply to the following:
// 1. Hop by Hop
// 2. Routing
// 3. Destination
//
typedef struct _IP6_EXT_HDR {
///
/// The Next Header field identifies the type of header immediately
///
UINT8 NextHeader;
///
/// The Hdr Ext Len field specifies the length of the Hop-by-Hop Options
///
UINT8 HdrExtLen;
///
/// Header-Specific Data
///
} IP6_EXT_HDR;

STATIC_ASSERT (
sizeof (IP6_EXT_HDR) == 2,
"The combined size of Next Header and Len is two 8 bit fields"
);

//
// IPv6 extension headers contain an 8-bit length field which describes the size of
// the header. However, the length field only includes the size of the extension
// header options, not the size of the first 8 bytes of the header. Therefore, in
// order to calculate the full size of the extension header, we add 1 (to account
// for the first 8 bytes omitted by the length field reporting) and then multiply
// by 8 (since the size is represented in 8-byte units).
//
// a is the length field of the extension header (UINT8)
// The result may be up to 2046 octets (UINT16)
//
#define IP6_HDR_EXT_LEN(a) (((UINT16)((UINT8)(a)) + 1) * 8)

// This is the maxmimum length permissible by a extension header
// Length is UINT8 of 8 octets not including the first 8 octets
#define IP6_MAX_EXT_DATA_LENGTH (IP6_HDR_EXT_LEN (MAX_UINT8) - sizeof(IP6_EXT_HDR))
STATIC_ASSERT (
IP6_MAX_EXT_DATA_LENGTH == 2046,
"Maximum data length is ((MAX_UINT8 + 1) * 8) - 2"
);

typedef struct _IP6_FRAGMENT_HEADER {
UINT8 NextHeader;
UINT8 Reserved;
Expand Down

0 comments on commit e257327

Please sign in to comment.