ovmf: Fix CVE-2023-45235

EDK2's Network Package is susceptible to a buffer overflow vulnerability when handling Server ID option from a DHCPv6 proxy Advertise message. This vulnerability can be exploited by an attacker to gain unauthorized access and potentially lead to a loss of Confidentiality, Integrity and/or Availability. References: https://nvd.nist.gov/vuln/detail/CVE-2023-45235 Upstream-patches: tianocore/edk2@fac2977 tianocore/edk2@ff29863 Signed-off-by: Soumya Sambu <[email protected]>
openembedded · Dec 4, 2024 · dd26902 · dd26902
1 parent d9d9e66
commit dd26902
Show file tree

Hide file tree

Showing 3 changed files with 624 additions and 0 deletions.
diff --git a/meta/recipes-core/ovmf/ovmf/CVE-2023-45235-0001.patch b/meta/recipes-core/ovmf/ovmf/CVE-2023-45235-0001.patch
@@ -0,0 +1,243 @@
+From fac297724e6cc343430cd0104e55cd7a96d1151e Mon Sep 17 00:00:00 2001
+From: Doug Flick <[email protected]>
+Date: Fri, 26 Jan 2024 05:54:55 +0800
+Subject: [PATCH] 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 <[email protected]>
+Cc: Zachary Clark-williams <[email protected]>
+
+Signed-off-by: Doug Flick [MSFT] <[email protected]>
+Reviewed-by: Saloni Kasbekar <[email protected]>
+
+CVE: CVE-2023-45235
+
+Upstream-Status: Backport [https://github.com/tianocore/edk2/commit/fac297724e6cc343430cd0104e55cd7a96d1151e]
+
+Signed-off-by: Soumya Sambu <[email protected]>
+---
+ NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 77 ++++++++++++++++++++++------
+ NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h | 17 ++++++
+ 2 files changed, 78 insertions(+), 16 deletions(-)
+
+diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
+index 2b2d372889..7fd1281c11 100644
+--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
++++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
+@@ -887,6 +887,7 @@ PxeBcRequestBootService (
+   EFI_STATUS                       Status;
+   EFI_DHCP6_PACKET                 *IndexOffer;
+   UINT8                            *Option;
++  UINTN                            DiscoverLenNeeded;
+
+   PxeBc      = &Private->PxeBc;
+   Request    = Private->Dhcp6Request;
+@@ -899,7 +900,8 @@ PxeBcRequestBootService (
+     return EFI_DEVICE_ERROR;
+   }
+
+-  Discover = AllocateZeroPool (sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET));
++  DiscoverLenNeeded = sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET);
++  Discover          = AllocateZeroPool (DiscoverLenNeeded);
+   if (Discover == NULL) {
+     return EFI_OUT_OF_RESOURCES;
+   }
+@@ -924,16 +926,34 @@ PxeBcRequestBootService (
+                DHCP6_OPT_SERVER_ID
+                );
+     if (Option == NULL) {
+-      return EFI_NOT_FOUND;
++      Status = EFI_NOT_FOUND;
++      goto ON_ERROR;
+     }
+
+     //
+     // Add Server ID Option.
+     //
+     OpLen = NTOHS (((EFI_DHCP6_PACKET_OPTION *)Option)->OpLen);
+-    CopyMem (DiscoverOpt, Option, OpLen + 4);
+-    DiscoverOpt += (OpLen + 4);
+-    DiscoverLen += (OpLen + 4);
++
++    //
++    // 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;
++    }
++
++    //
++    // 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;
++    }
++
++    CopyMem (DiscoverOpt, Option, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
++    DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
++    DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
+   }
+
+   while (RequestLen < Request->Length) {
+@@ -944,16 +964,24 @@ PxeBcRequestBootService (
+         (OpCode != DHCP6_OPT_SERVER_ID)
+         )
+     {
++      //
++      // 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;
++      }
++
+       //
+       // Copy all the options except IA option and Server ID
+       //
+-      CopyMem (DiscoverOpt, RequestOpt, OpLen + 4);
+-      DiscoverOpt += (OpLen + 4);
+-      DiscoverLen += (OpLen + 4);
++      CopyMem (DiscoverOpt, RequestOpt, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
++      DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
++      DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
+     }
+
+-    RequestOpt += (OpLen + 4);
+-    RequestLen += (OpLen + 4);
++    RequestOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
++    RequestLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
+   }
+
+   //
+@@ -2154,6 +2182,7 @@ PxeBcDhcp6Discover (
+   UINT16                           OpLen;
+   UINT32                           Xid;
+   EFI_STATUS                       Status;
++  UINTN                            DiscoverLenNeeded;
+
+   PxeBc    = &Private->PxeBc;
+   Mode     = PxeBc->Mode;
+@@ -2169,7 +2198,8 @@ PxeBcDhcp6Discover (
+     return EFI_DEVICE_ERROR;
+   }
+
+-  Discover = AllocateZeroPool (sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET));
++  DiscoverLenNeeded = sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET);
++  Discover          = AllocateZeroPool (DiscoverLenNeeded);
+   if (Discover == NULL) {
+     return EFI_OUT_OF_RESOURCES;
+   }
+@@ -2185,22 +2215,37 @@ PxeBcDhcp6Discover (
+   DiscoverLen             = sizeof (EFI_DHCP6_HEADER);
+   RequestLen              = DiscoverLen;
+
++  //
++  // The request packet is generated by the UEFI network stack. In the DHCP4 DORA and DHCP6 SARR sequence,
++  // the first (discover in DHCP4 and solicit in DHCP6) and third (request in both DHCP4 and DHCP6) are
++  // generated by the DHCP client (the UEFI network stack in this case). By the time this function executes,
++  // the DHCP sequence already has been executed once (see UEFI Specification Figures 24.2 and 24.3), with
++  // Private->Dhcp6Request being a cached copy of the DHCP6 request packet that UEFI network stack previously
++  // generated and sent.
++  //
++  // Therefore while this code looks like it could overflow, in practice it's not possible.
++  //
+   while (RequestLen < Request->Length) {
+     OpCode = NTOHS (((EFI_DHCP6_PACKET_OPTION *)RequestOpt)->OpCode);
+     OpLen  = NTOHS (((EFI_DHCP6_PACKET_OPTION *)RequestOpt)->OpLen);
+     if ((OpCode != EFI_DHCP6_IA_TYPE_NA) &&
+         (OpCode != EFI_DHCP6_IA_TYPE_TA))
+     {
++      if (DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN > DiscoverLenNeeded) {
++        Status = EFI_OUT_OF_RESOURCES;
++        goto ON_ERROR;
++      }
++
+       //
+       // Copy all the options except IA option.
+       //
+-      CopyMem (DiscoverOpt, RequestOpt, OpLen + 4);
+-      DiscoverOpt += (OpLen + 4);
+-      DiscoverLen += (OpLen + 4);
++      CopyMem (DiscoverOpt, RequestOpt, OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
++      DiscoverOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
++      DiscoverLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
+     }
+
+-    RequestOpt += (OpLen + 4);
+-    RequestLen += (OpLen + 4);
++    RequestOpt += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
++    RequestLen += (OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN);
+   }
+
+   Status = PxeBc->UdpWrite (
+diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
+index c86f6d391b..6357d27fae 100644
+--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
++++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
+@@ -34,6 +34,23 @@
+ #define PXEBC_ADDR_START_DELIMITER        '['
+ #define PXEBC_ADDR_END_DELIMITER          ']'
+
++//
++// A DUID consists of a 2-octet type code represented in network byte
++// order, followed by a variable number of octets that make up the
++// actual identifier.  The length of the DUID (not including the type
++// code) is at least 1 octet and at most 128 octets.
++//
++#define PXEBC_MIN_SIZE_OF_DUID  (sizeof(UINT16) + 1)
++#define PXEBC_MAX_SIZE_OF_DUID  (sizeof(UINT16) + 128)
++
++//
++// This define represents the combineds code and length field from
++// https://datatracker.ietf.org/doc/html/rfc3315#section-22.1
++//
++#define PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN  \
++      (sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpCode) + \
++      sizeof (((EFI_DHCP6_PACKET_OPTION *)0)->OpLen))
++
+ #define GET_NEXT_DHCP6_OPTION(Opt) \
+   (EFI_DHCP6_PACKET_OPTION *) ((UINT8 *) (Opt) + \
+   sizeof (EFI_DHCP6_PACKET_OPTION) + (NTOHS ((Opt)->OpLen)) - 1)
+-- 
+2.40.0
+