Re: [edk2-devel] [PATCH 1/3] [edk2-stable202402] NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Related Patch

["Saloni Kasbekar" <saloni.kasbekar@intel.com>]

Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>

-----Original Message-----
From: Douglas Flick [MSFT] <doug.edk2@gmail.com> 
Sent: Friday, February 9, 2024 7:05 PM
To: devel@edk2.groups.io
Cc: Doug Flick <dougflick@microsoft.com>; Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary <zachary.clark-williams@intel.com>; Andrew Fish <afish@apple.com>; Leif Lindholm <quic_llindhol@quicinc.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Doug Flick [MSFT] <doug.edk2@gmail.com>
Subject: [PATCH 1/3] [edk2-stable202402] NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Related Patch

From: Doug Flick <dougflick@microsoft.com>

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

This was not part of the Quarkslab bugs however the same pattern as CVE-2023-45229 exists in Dhcp6UpdateIaInfo.

This patch replaces the code in question with the safe function created to patch CVE-2023-45229

>
>   if (EFI_ERROR (
>        Dhcp6SeekInnerOptionSafe (
>          Instance->Config->IaDescriptor.Type,
>          Option,
>          OptionLen,
>          &IaInnerOpt,
>          &IaInnerLen
>          )
>        ))
>  {
>    return EFI_DEVICE_ERROR;
>  }
>

Additionally corrects incorrect usage of macro to read the status

> - StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)DHCP6_OFFSET_OF_OPT_LEN
 (Option)));
> + StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)
DHCP6_OFFSET_OF_STATUS_CODE (Option));

Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
Cc: Zachary Clark-williams <zachary.clark-williams@intel.com>

Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>

Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
---
 NetworkPkg/Dhcp6Dxe/Dhcp6Io.c | 232 ++++++++++++++++++++--------------
 1 file changed, 134 insertions(+), 98 deletions(-)

diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c index 3b8feb4a2032..6000e885afaf 100644
--- a/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c
+++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Io.c
@@ -510,6 +510,97 @@ Dhcp6CallbackUser (
   return Status; } +/**+  Seeks the Inner Options from a DHCP6 Option++  @param[in]  IaType          The type of the IA option.+  @param[in]  Option          The pointer to the DHCP6 Option.+  @param[in]  OptionLen       The length of the DHCP6 Option.+  @param[out] IaInnerOpt      The pointer to the IA inner option.+  @param[out] IaInnerLen      The length of the IA inner option.++  @retval EFI_SUCCESS         Seek the inner option successfully.+  @retval EFI_DEVICE_ERROR    The OptionLen is invalid. On Error,+                              the pointers are not modified+**/+EFI_STATUS+Dhcp6SeekInnerOptionSafe (+  IN  UINT16  IaType,+  IN  UINT8   *Option,+  IN  UINT32  OptionLen,+  OUT UINT8   **IaInnerOpt,+  OUT UINT16  *IaInnerLen+  )+{+  UINT16  IaInnerLenTmp;+  UINT8   *IaInnerOptTmp;++  if (Option == NULL) {+    ASSERT (Option != NULL);+    return EFI_DEVICE_ERROR;+  }++  if (IaInnerOpt == NULL) {+    ASSERT (IaInnerOpt != NULL);+    return EFI_DEVICE_ERROR;+  }++  if (IaInnerLen == NULL) {+    ASSERT (IaInnerLen != NULL);+    return EFI_DEVICE_ERROR;+  }++  if (IaType == Dhcp6OptIana) {+    //+    // Verify we have a fully formed IA_NA+    //+    if (OptionLen < DHCP6_MIN_SIZE_OF_IA_NA) {+      return EFI_DEVICE_ERROR;+    }++    //+    // Get the IA Inner Option and Length+    //+    IaInnerOptTmp = DHCP6_OFFSET_OF_IA_NA_INNER_OPT (Option);++    //+    // Verify the IaInnerLen is valid.+    //+    IaInnerLenTmp = (UINT16)NTOHS (ReadUnaligned16 ((UINT16 *)DHCP6_OFFSET_OF_OPT_LEN (Option)));+    if (IaInnerLenTmp < DHCP6_SIZE_OF_COMBINED_IAID_T1_T2) {+      return EFI_DEVICE_ERROR;+    }++    IaInnerLenTmp -= DHCP6_SIZE_OF_COMBINED_IAID_T1_T2;+  } else if (IaType == Dhcp6OptIata) {+    //+    // Verify the OptionLen is valid.+    //+    if (OptionLen < DHCP6_MIN_SIZE_OF_IA_TA) {+      return EFI_DEVICE_ERROR;+    }++    IaInnerOptTmp = DHCP6_OFFSET_OF_IA_TA_INNER_OPT (Option);++    //+    // Verify the IaInnerLen is valid.+    //+    IaInnerLenTmp = (UINT16)NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN (Option))));+    if (IaInnerLenTmp < DHCP6_SIZE_OF_IAID) {+      return EFI_DEVICE_ERROR;+    }++    IaInnerLenTmp -= DHCP6_SIZE_OF_IAID;+  } else {+    return EFI_DEVICE_ERROR;+  }++  *IaInnerOpt = IaInnerOptTmp;+  *IaInnerLen = IaInnerLenTmp;++  return EFI_SUCCESS;+}+ /**   Update Ia according to the new reply message. @@ -528,13 +619,23 @@ Dhcp6UpdateIaInfo (
 {   EFI_STATUS  Status;   UINT8       *Option;+  UINT32      OptionLen;   UINT8       *IaInnerOpt;   UINT16      IaInnerLen;   UINT16      StsCode;   UINT32      T1;   UINT32      T2; +  T1 = 0;+  T2 = 0;+   ASSERT (Instance->Config != NULL);++  // OptionLen is the length of the Options excluding the DHCP header.+  // Length of the EFI_DHCP6_PACKET from the first byte of the Header field to the last+  // byte of the Option[] field.+  OptionLen = Packet->Length - sizeof (Packet->Dhcp6.Header);+   //   // If the reply was received in response to a solicit with rapid commit option,   // request, renew or rebind message, the client updates the information it has@@ -549,13 +650,29 @@ Dhcp6UpdateIaInfo (
   //   Option = Dhcp6SeekIaOption (              Packet->Dhcp6.Option,-             Packet->Length - sizeof (EFI_DHCP6_HEADER),+             OptionLen,              &Instance->Config->IaDescriptor              );   if (Option == NULL) {     return EFI_DEVICE_ERROR;   } +  //+  // Calculate the distance from Packet->Dhcp6.Option to the IA option.+  //+  // Packet->Size and Packet->Length are both UINT32 type, and Packet->Size is+  // the size of the whole packet, including the DHCP header, and Packet->Length+  // is the length of the DHCP message body, excluding the DHCP header.+  //+  // (*Option - Packet->Dhcp6.Option) is the number of bytes from the start of+  // DHCP6 option area to the start of the IA option.+  //+  // Dhcp6SeekInnerOptionSafe() is searching starting from the start of the+  // IA option to the end of the DHCP6 option area, thus subtract the space+  // up until this option+  //+  OptionLen = OptionLen - (UINT32)(Option - Packet->Dhcp6.Option);+   //   // The format of the IA_NA option is:   //@@ -591,32 +708,32 @@ Dhcp6UpdateIaInfo (
   //    //-  // sizeof (option-code + option-len + IaId)           = 8-  // sizeof (option-code + option-len + IaId + T1)      = 12-  // sizeof (option-code + option-len + IaId + T1 + T2) = 16-  //-  // The inner options still start with 2 bytes option-code and 2 bytes option-len.+  // Seek the inner option   //+  if (EFI_ERROR (+        Dhcp6SeekInnerOptionSafe (+          Instance->Config->IaDescriptor.Type,+          Option,+          OptionLen,+          &IaInnerOpt,+          &IaInnerLen+          )+        ))+  {+    return EFI_DEVICE_ERROR;+  }+   if (Instance->Config->IaDescriptor.Type == Dhcp6OptIana) {     T1 = NTOHL (ReadUnaligned32 ((UINT32 *)(DHCP6_OFFSET_OF_IA_NA_T1 (Option))));     T2 = NTOHL (ReadUnaligned32 ((UINT32 *)(DHCP6_OFFSET_OF_IA_NA_T2 (Option))));     //     // Refer to RFC3155 Chapter 22.4. If a client receives an IA_NA with T1 greater than T2,     // and both T1 and T2 are greater than 0, the client discards the IA_NA option and processes-    // the remainder of the message as though the server had not  included the invalid IA_NA option.+    // the remainder of the message as though the server had not included the invalid IA_NA option.     //     if ((T1 > T2) && (T2 > 0)) {       return EFI_DEVICE_ERROR;     }--    IaInnerOpt = DHCP6_OFFSET_OF_IA_NA_INNER_OPT (Option);-    IaInnerLen = (UINT16)(NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN (Option)))) - DHCP6_SIZE_OF_COMBINED_IAID_T1_T2);-  } else {-    T1 = 0;-    T2 = 0;--    IaInnerOpt = DHCP6_OFFSET_OF_IA_TA_INNER_OPT (Option);-    IaInnerLen = (UINT16)(NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN (Option)))) - DHCP6_SIZE_OF_IAID);   }    //@@ -642,7 +759,7 @@ Dhcp6UpdateIaInfo (
   Option  = Dhcp6SeekOption (IaInnerOpt, IaInnerLen, Dhcp6OptStatusCode);    if (Option != NULL) {-    StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN (Option))));+    StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_STATUS_CODE (Option))));     if (StsCode != Dhcp6StsSuccess) {       return EFI_DEVICE_ERROR;     }@@ -662,87 +779,6 @@ Dhcp6UpdateIaInfo (
   return Status; } -/**-  Seeks the Inner Options from a DHCP6 Option--  @param[in]  IaType          The type of the IA option.-  @param[in]  Option          The pointer to the DHCP6 Option.-  @param[in]  OptionLen       The length of the DHCP6 Option.-  @param[out] IaInnerOpt      The pointer to the IA inner option.-  @param[out] IaInnerLen      The length of the IA inner option.--  @retval EFI_SUCCESS         Seek the inner option successfully.-  @retval EFI_DEVICE_ERROR    The OptionLen is invalid. On Error,-                              the pointers are not modified-**/-EFI_STATUS-Dhcp6SeekInnerOptionSafe (-  IN  UINT16  IaType,-  IN  UINT8   *Option,-  IN  UINT32  OptionLen,-  OUT UINT8   **IaInnerOpt,-  OUT UINT16  *IaInnerLen-  )-{-  UINT16  IaInnerLenTmp;-  UINT8   *IaInnerOptTmp;--  if (Option == NULL) {-    ASSERT (Option != NULL);-    return EFI_DEVICE_ERROR;-  }--  if (IaInnerOpt == NULL) {-    ASSERT (IaInnerOpt != NULL);-    return EFI_DEVICE_ERROR;-  }--  if (IaInnerLen == NULL) {-    ASSERT (IaInnerLen != NULL);-    return EFI_DEVICE_ERROR;-  }--  if (IaType == Dhcp6OptIana) {-    // Verify we have a fully formed IA_NA-    if (OptionLen < DHCP6_MIN_SIZE_OF_IA_NA) {-      return EFI_DEVICE_ERROR;-    }--    //-    IaInnerOptTmp = DHCP6_OFFSET_OF_IA_NA_INNER_OPT (Option);--    // Verify the IaInnerLen is valid.-    IaInnerLenTmp = (UINT16)NTOHS (ReadUnaligned16 ((UINT16 *)DHCP6_OFFSET_OF_OPT_LEN (Option)));-    if (IaInnerLenTmp < DHCP6_SIZE_OF_COMBINED_IAID_T1_T2) {-      return EFI_DEVICE_ERROR;-    }--    IaInnerLenTmp -= DHCP6_SIZE_OF_COMBINED_IAID_T1_T2;-  } else if (IaType == Dhcp6OptIata) {-    // Verify the OptionLen is valid.-    if (OptionLen < DHCP6_MIN_SIZE_OF_IA_TA) {-      return EFI_DEVICE_ERROR;-    }--    IaInnerOptTmp = DHCP6_OFFSET_OF_IA_TA_INNER_OPT (Option);--    // Verify the IaInnerLen is valid.-    IaInnerLenTmp = (UINT16)NTOHS (ReadUnaligned16 ((UINT16 *)(DHCP6_OFFSET_OF_OPT_LEN (Option))));-    if (IaInnerLenTmp < DHCP6_SIZE_OF_IAID) {-      return EFI_DEVICE_ERROR;-    }--    IaInnerLenTmp -= DHCP6_SIZE_OF_IAID;-  } else {-    return EFI_DEVICE_ERROR;-  }--  *IaInnerOpt = IaInnerOptTmp;-  *IaInnerLen = IaInnerLenTmp;--  return EFI_SUCCESS;-}- /**   Seek StatusCode Option in package. A Status Code option may appear in the   options field of a DHCP message and/or in the options field of another option.-- 
2.43.0



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