From 23b31a16bbb789f4c251b1d2f23334210a9fb545 Mon Sep 17 00:00:00 2001 From: Jon Maloy Date: Thu, 15 Feb 2024 11:51:09 -0500 Subject: [PATCH 04/15] NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45232 Patch RH-Author: Jon Maloy RH-MergeRequest: 56: Pixiefail issues in NetworkPkg package RH-Jira: RHEL-21840 RHEL-21844 RHEL-21846 RHEL-21848 RHEL-21850 RHEL-21852 RH-Acked-by: Gerd Hoffmann RH-Acked-by: Oliver Steffen RH-Commit: [4/15] 48c273e43a6275c7eae3223c4ffa433f4d6531a4 JIRA: https://issues.redhat.com/browse/RHEL-21846 CVE: CVE-2022-45232 Upstream: Merged JIRA: https://issues.redhat.com/browse/RHEL-21848 CVE: CVE-2022-45233 Upstream: Merged commit 4df0229ef992d4f2721a8508787ebf9dc81fbd6e Author: Doug Flick Date: Fri Jan 26 05:54:50 2024 +0800 NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45232 Patch REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537 REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538 Bug Details: 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 Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] Reviewed-by: Saloni Kasbekar Signed-off-by: Jon Maloy --- NetworkPkg/Ip6Dxe/Ip6Nd.h | 35 ++++++++++++++++ NetworkPkg/Ip6Dxe/Ip6Option.c | 76 ++++++++++++++++++++++++++++++----- NetworkPkg/Ip6Dxe/Ip6Option.h | 71 ++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+), 11 deletions(-) diff --git a/NetworkPkg/Ip6Dxe/Ip6Nd.h b/NetworkPkg/Ip6Dxe/Ip6Nd.h index 860934a167..bf64e9114e 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Nd.h +++ b/NetworkPkg/Ip6Dxe/Ip6Nd.h @@ -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; diff --git a/NetworkPkg/Ip6Dxe/Ip6Option.c b/NetworkPkg/Ip6Dxe/Ip6Option.c index 8718d5d875..fd97ce116f 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Option.c +++ b/NetworkPkg/Ip6Dxe/Ip6Option.c @@ -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. @@ -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; @@ -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: // @@ -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; @@ -308,7 +332,7 @@ Ip6IsExtsValid ( UINT32 Pointer; UINT32 Offset; UINT8 *Option; - UINT8 OptionLen; + UINT16 OptionLen; BOOLEAN Flag; UINT8 CountD; UINT8 CountA; @@ -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++; } @@ -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++; @@ -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; @@ -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)) { @@ -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; diff --git a/NetworkPkg/Ip6Dxe/Ip6Option.h b/NetworkPkg/Ip6Dxe/Ip6Option.h index bd8e223c8a..fb07c28f5a 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Option.h +++ b/NetworkPkg/Ip6Dxe/Ip6Option.h @@ -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; -- 2.39.3