378 lines
14 KiB
Diff
378 lines
14 KiB
Diff
|
From ff4f1d8227c6c4c89060e24df37defec6d7a07e2 Mon Sep 17 00:00:00 2001
|
||
|
From: Jon Maloy <jmaloy@redhat.com>
|
||
|
Date: Thu, 15 Feb 2024 11:51:09 -0500
|
||
|
Subject: [PATCH 08/18] NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45232 Patch
|
||
|
|
||
|
RH-Author: Jon Maloy <jmaloy@redhat.com>
|
||
|
RH-MergeRequest: 54: NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45230 Patch
|
||
|
RH-Jira: RHEL-21841 RHEL-21843 RHEL-21845 RHEL-21847 RHEL-21849 RHEL-21851 RHEL-21853
|
||
|
RH-Acked-by: Gerd Hoffmann <None>
|
||
|
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
|
||
|
RH-Commit: [8/18] c7bf831954da5b678450f1ba8e34371645959c81
|
||
|
|
||
|
JIRA: https://issues.redhat.com/browse/RHEL-21847
|
||
|
CVE: CVE-2022-45232
|
||
|
Upstream: Merged
|
||
|
|
||
|
JIRA: https://issues.redhat.com/browse/RHEL-21849
|
||
|
CVE: CVE-2022-45233
|
||
|
Upstream: Merged
|
||
|
|
||
|
commit 4df0229ef992d4f2721a8508787ebf9dc81fbd6e
|
||
|
Author: Doug Flick <dougflick@microsoft.com>
|
||
|
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 <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>
|
||
|
|
||
|
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
|
||
|
---
|
||
|
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
|
||
|
|