[edk2-devel] [PATCH 07/14] NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45232 Patch

["Doug Flick via groups.io" <dougflick=microsoft.com@groups.io>]

From: Doug Flick <dougflick@microsoft.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

SECURITY PATCH - Patch

TCBZ4537
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')

TCBZ4538
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')

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>
---
 NetworkPkg/Ip6Dxe/Ip6Option.h | 89 +++++++++++++++++++++++++++++++++++
 NetworkPkg/Ip6Dxe/Ip6Option.c | 76 +++++++++++++++++++++++++-----
 2 files changed, 154 insertions(+), 11 deletions(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6Option.h b/NetworkPkg/Ip6Dxe/Ip6Option.h
index bd8e223c8a67..5d786073ebcb 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Option.h
+++ b/NetworkPkg/Ip6Dxe/Ip6Option.h
@@ -12,6 +12,95 @@
 
 #define IP6_FRAGMENT_OFFSET_MASK  (~0x3)
 
+//
+// 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.
+//
+#define IP6_SIZE_OF_OPT_TYPE                  (sizeof(UINT8))
+#define IP6_SIZE_OF_OPT_LEN                   (sizeof(UINT8))
+#define IP6_COMBINED_SIZE_OF_OPT_TAG_AND_LEN  (IP6_SIZE_OF_OPT_TYPE + IP6_SIZE_OF_OPT_LEN)
+#define IP6_OFFSET_OF_OPT_LEN(a)  (a + IP6_SIZE_OF_OPT_TYPE)
+STATIC_ASSERT (
+  IP6_OFFSET_OF_OPT_LEN (0) == 1,
+  "The Length field should be 1 octet (8 bits) past the start of the option"
+  );
+
+#define IP6_NEXT_OPTION_OFFSET(offset, length)  (offset + IP6_COMBINED_SIZE_OF_OPT_TAG_AND_LEN + length)
+STATIC_ASSERT (
+  IP6_NEXT_OPTION_OFFSET (0, 0) == 2,
+  "The next option is minimally the combined size of the option tag and length"
+  );
+
+//
+// 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
+//
+#define IP6_SIZE_OF_EXT_NEXT_HDR  (sizeof(UINT8))
+#define IP6_SIZE_OF_HDR_EXT_LEN   (sizeof(UINT8))
+
+#define IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN  (IP6_SIZE_OF_EXT_NEXT_HDR + IP6_SIZE_OF_HDR_EXT_LEN)
+STATIC_ASSERT (
+  IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN == 2,
+  "The combined size of Next Header and Len is two 8 bit fields"
+  );
+
+//
+// The "+ 1" in this calculation is because of the "not including the first 8 octets"
+// part of the definition (meaning the value of 0 represents 64 bits)
+//
+#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) - IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN)
+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;
diff --git a/NetworkPkg/Ip6Dxe/Ip6Option.c b/NetworkPkg/Ip6Dxe/Ip6Option.c
index 8718d5d8756a..144f8d34dead 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_OFFSET_OF_OPT_LEN (Option + Offset));
+        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_OFFSET_OF_OPT_LEN (Option + Offset));
+            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) - IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;
         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 + IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN + 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;
 
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114259): https://edk2.groups.io/g/devel/message/114259
Mute This Topic: https://groups.io/mt/103926738/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-