NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45235 Patch

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

Bug Details:
PixieFail Bug #7
CVE-2023-45235
CVSS 8.3 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:L/A:H
CWE-119 Improper Restriction of Operations within the Bounds of
 a Memory Buffer

Buffer overflow when handling Server ID option from a DHCPv6 proxy
Advertise message

Change Overview:

Performs two checks

1. Checks that the length of the duid is accurate
> + //
> + // Check that the minimum and maximum requirements are met
> + //
> + if ((OpLen < PXEBC_MIN_SIZE_OF_DUID) ||
(OpLen > PXEBC_MAX_SIZE_OF_DUID)) {
> +  Status = EFI_INVALID_PARAMETER;
> +  goto ON_ERROR;
> + }

2. Ensures that the amount of data written to the buffer is tracked and
never exceeds that
> + //
> + // Check that the option length is valid.
> + //
> + if ((DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN)
 > DiscoverLenNeeded) {
> +     Status = EFI_OUT_OF_RESOURCES;
> +     goto ON_ERROR;
> + }

Additional code clean up and fix for memory leak in case Option was NULL

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>

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
index 2b2d372889a3541a9632b26e83d625038e0d14e6..7fd1281c11840cfeaad5efbb5de96c4b33cf93b5 100644 (file)
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
@@ -887,6 +887,7 @@ PxeBcRequestBootService (
   EFI_STATUS                       Status;\r
   EFI_DHCP6_PACKET                 *IndexOffer;\r
   UINT8                            *Option;\r
+  UINTN                            DiscoverLenNeeded;\r
 \r
   PxeBc      = &Private->PxeBc;\r
   Request    = Private->Dhcp6Request;\r
@@ -899,7 +900,8 @@ PxeBcRequestBootService (
     return EFI_DEVICE_ERROR;\r
   }\r
 \r
-  Discover = AllocateZeroPool (sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET));\r
+  DiscoverLenNeeded = sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET);\r
+  Discover          = AllocateZeroPool (DiscoverLenNeeded);\r
   if (Discover == NULL) {\r
     return EFI_OUT_OF_RESOURCES;\r
   }\r
@@ -924,16 +926,34 @@ PxeBcRequestBootService (
                DHCP6_OPT_SERVER_ID\r
                );\r
     if (Option == NULL) {\r
-      return EFI_NOT_FOUND;\r
+      Status = EFI_NOT_FOUND;\r
+      goto ON_ERROR;\r
     }\r
 \r
     //\r
     // Add Server ID Option.\r
     //\r
     OpLen = NTOHS (((EFI_DHCP6_PACKET_OPTION *)Option)->OpLen);\r
-    CopyMem (DiscoverOpt, Option, OpLen + 4);\r
-    DiscoverOpt += (OpLen + 4);\r
-    DiscoverLen += (OpLen + 4);\r
+\r
+    //\r
+    // Check that the minimum and maximum requirements are met\r
+    //\r
+    if ((OpLen < PXEBC_MIN_SIZE_OF_DUID) || (OpLen > PXEBC_MAX_SIZE_OF_DUID)) {\r
+      Status = EFI_INVALID_PARAMETER;\r
+      goto ON_ERROR;\r
+    }\r
+\r
+    //\r
+    // Check that the option length is valid.\r
+    //\r
+    if ((DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN) > DiscoverLenNeeded) {\r
+      Status = EFI_OUT_OF_RESOURCES;\r
+      goto ON_ERROR;\r
+    }\r
+\r
+    CopyMem (DiscoverOpt, Option, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
+    DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
+    DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
   }\r
 \r
   while (RequestLen < Request->Length) {\r
@@ -944,16 +964,24 @@ PxeBcRequestBootService (
         (OpCode != DHCP6_OPT_SERVER_ID)\r
         )\r
     {\r
+      //\r
+      // Check that the option length is valid.\r
+      //\r
+      if (DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN > DiscoverLenNeeded) {\r
+        Status = EFI_OUT_OF_RESOURCES;\r
+        goto ON_ERROR;\r
+      }\r
+\r
       //\r
       // Copy all the options except IA option and Server ID\r
       //\r
-      CopyMem (DiscoverOpt, RequestOpt, OpLen + 4);\r
-      DiscoverOpt += (OpLen + 4);\r
-      DiscoverLen += (OpLen + 4);\r
+      CopyMem (DiscoverOpt, RequestOpt, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
+      DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
+      DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
     }\r
 \r
-    RequestOpt += (OpLen + 4);\r
-    RequestLen += (OpLen + 4);\r
+    RequestOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
+    RequestLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
   }\r
 \r
   //\r
@@ -2154,6 +2182,7 @@ PxeBcDhcp6Discover (
   UINT16                           OpLen;\r
   UINT32                           Xid;\r
   EFI_STATUS                       Status;\r
+  UINTN                            DiscoverLenNeeded;\r
 \r
   PxeBc    = &Private->PxeBc;\r
   Mode     = PxeBc->Mode;\r
@@ -2169,7 +2198,8 @@ PxeBcDhcp6Discover (
     return EFI_DEVICE_ERROR;\r
   }\r
 \r
-  Discover = AllocateZeroPool (sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET));\r
+  DiscoverLenNeeded = sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET);\r
+  Discover          = AllocateZeroPool (DiscoverLenNeeded);\r
   if (Discover == NULL) {\r
     return EFI_OUT_OF_RESOURCES;\r
   }\r
@@ -2185,22 +2215,37 @@ PxeBcDhcp6Discover (
   DiscoverLen             = sizeof (EFI_DHCP6_HEADER);\r
   RequestLen              = DiscoverLen;\r
 \r
+  //\r
+  // The request packet is generated by the UEFI network stack. In the DHCP4 DORA and DHCP6 SARR sequence,\r
+  // the first (discover in DHCP4 and solicit in DHCP6) and third (request in both DHCP4 and DHCP6) are\r
+  // generated by the DHCP client (the UEFI network stack in this case). By the time this function executes,\r
+  // the DHCP sequence already has been executed once (see UEFI Specification Figures 24.2 and 24.3), with\r
+  // Private->Dhcp6Request being a cached copy of the DHCP6 request packet that UEFI network stack previously\r
+  // generated and sent.\r
+  //\r
+  // Therefore while this code looks like it could overflow, in practice it's not possible.\r
+  //\r
   while (RequestLen < Request->Length) {\r
     OpCode = NTOHS (((EFI_DHCP6_PACKET_OPTION *)RequestOpt)->OpCode);\r
     OpLen  = NTOHS (((EFI_DHCP6_PACKET_OPTION *)RequestOpt)->OpLen);\r
     if ((OpCode != EFI_DHCP6_IA_TYPE_NA) &&\r
         (OpCode != EFI_DHCP6_IA_TYPE_TA))\r
     {\r
+      if (DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN > DiscoverLenNeeded) {\r
+        Status = EFI_OUT_OF_RESOURCES;\r
+        goto ON_ERROR;\r
+      }\r
+\r
       //\r
       // Copy all the options except IA option.\r
       //\r
-      CopyMem (DiscoverOpt, RequestOpt, OpLen + 4);\r
-      DiscoverOpt += (OpLen + 4);\r
-      DiscoverLen += (OpLen + 4);\r
+      CopyMem (DiscoverOpt, RequestOpt, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
+      DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
+      DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
     }\r
 \r
-    RequestOpt += (OpLen + 4);\r
-    RequestLen += (OpLen + 4);\r
+    RequestOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
+    RequestLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);\r
   }\r
 \r
   Status = PxeBc->UdpWrite (\r

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
index c86f6d391b801c41a80dee9119662f2135cdd11d..6357d27faefd6654e1254eb1ebe8ce67173c94b6 100644 (file)
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
@@ -34,6 +34,23 @@
 #define PXEBC_ADDR_START_DELIMITER        '['\r
 #define PXEBC_ADDR_END_DELIMITER          ']'\r
 \r
+//\r
+// A DUID consists of a 2-octet type code represented in network byte\r
+// order, followed by a variable number of octets that make up the\r
+// actual identifier.  The length of the DUID (not including the type\r
+// code) is at least 1 octet and at most 128 octets.\r
+//\r
+#define PXEBC_MIN_SIZE_OF_DUID  (sizeof(UINT16) + 1)\r
+#define PXEBC_MAX_SIZE_OF_DUID  (sizeof(UINT16) + 128)\r
+\r
+//\r
+// This define represents the combineds code and length field from\r
+// https://datatracker.ietf.org/doc/html/rfc3315#section-22.1\r
+//\r
+#define PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN  \\r
+      (sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpCode) + \\r
+      sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpLen))\r
+\r
 #define GET_NEXT_DHCP6_OPTION(Opt) \\r
   (EFI_DHCP6_PACKET_OPTION *) ((UINT8 *) (Opt) + \\r
   sizeof (EFI_DHCP6_PACKET_OPTION) + (NTOHS ((Opt)->OpLen)) - 1)\r