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>

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;