[v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).

[edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).

Posted by Wu, Jiaxin 4 years, 2 months ago

This patch is to check the received package length to make sure the package
has a valid length field.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
---
 NetworkPkg/Ip4Dxe/Ip4Input.c | 46 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/NetworkPkg/Ip4Dxe/Ip4Input.c b/NetworkPkg/Ip4Dxe/Ip4Input.c
index fec242c71f..95fbd01d05 100644
--- a/NetworkPkg/Ip4Dxe/Ip4Input.c
+++ b/NetworkPkg/Ip4Dxe/Ip4Input.c
@@ -1,9 +1,9 @@
 /** @file
   IP4 input process.
 
-Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2020, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -709,14 +709,10 @@ Ip4PreProcessPacket (
   UINT16                    Checksum;
 
   //
   // Check if the IP4 header is correctly formatted.
   //
-  if ((*Packet)->TotalSize < IP4_MIN_HEADLEN) {
-    return EFI_INVALID_PARAMETER;
-  }
-
   HeadLen  = (Head->HeadLen << 2);
   TotalLen = NTOHS (Head->TotalLen);
 
   //
   // Mnp may deliver frame trailer sequence up, trim it off.
@@ -806,10 +802,34 @@ Ip4PreProcessPacket (
   }
 
   return EFI_SUCCESS;
 }
 
+/**
+  This function checks the IPv4 packet length.
+
+  @param[in]       Packet          Pointer to the IPv4 Packet to be checked.
+
+  @retval TRUE                   The input IPv4 packet length is valid.
+  @retval FALSE                  The input IPv4 packet length is invalid.
+
+**/
+BOOLEAN
+Ip4IsValidPacketLength (
+  IN NET_BUF        *Packet
+  )
+{
+  //
+  // Check the IP4 packet length.
+  //
+  if (Packet->TotalSize < IP4_MIN_HEADLEN) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
 /**
   The IP4 input routine. It is called by the IP4_INTERFACE when a
   IP4 fragment is received from MNP.
 
   @param[in]  Ip4Instance        The IP4 child that request the receive, most like
@@ -842,10 +862,14 @@ Ip4AccpetFrame (
 
   if (EFI_ERROR (IoStatus) || (IpSb->State == IP4_SERVICE_DESTROY)) {
     goto DROP;
   }
 
+  if (!Ip4IsValidPacketLength (Packet)) {
+    goto RESTART;
+  }
+
   Head      = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL);
   ASSERT (Head != NULL);
   OptionLen = (Head->HeadLen << 2) - IP4_MIN_HEADLEN;
   if (OptionLen > 0) {
     Option = (UINT8 *) (Head + 1);
@@ -888,14 +912,18 @@ Ip4AccpetFrame (
   //
   // If the packet is protected by tunnel mode, parse the inner Ip Packet.
   //
   ZeroMem (&ZeroHead, sizeof (IP4_HEAD));
   if (0 == CompareMem (Head, &ZeroHead, sizeof (IP4_HEAD))) {
-  // Packet may have been changed. Head, HeadLen, TotalLen, and
-  // info must be reloaded before use. The ownership of the packet
-  // is transferred to the packet process logic.
-  //
+    // Packet may have been changed. Head, HeadLen, TotalLen, and
+    // info must be reloaded before use. The ownership of the packet
+    // is transferred to the packet process logic.
+    //
+    if (!Ip4IsValidPacketLength (Packet)) {
+      goto RESTART;
+    }
+    
     Head = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL);
     ASSERT (Head != NULL);
     Status = Ip4PreProcessPacket (
                IpSb,
                &Packet,
-- 
2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54538): https://edk2.groups.io/g/devel/message/54538
Mute This Topic: https://groups.io/mt/71345472/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).

Posted by Laszlo Ersek 4 years, 2 months ago

On 02/17/20 08:43, Wu, Jiaxin wrote:
> This patch is to check the received package length to make sure the package
> has a valid length field.
> 
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
> ---
>  NetworkPkg/Ip4Dxe/Ip4Input.c | 46 +++++++++++++++++++++++++++++++++++---------
>  1 file changed, 37 insertions(+), 9 deletions(-)

There are two patches on the list for CVE-2019-14559:

- [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).
- [edk2-devel] [PATCH 1/1] NetworkPkg/ArpDxe: Recycle invalid ARP packets(CVE-2019-14559).

sent by different submitters.

How do they relate to each other?

Also, while Nick's patch mentions TianoCore#2031, the current patch doesn't include a BZ link. Is the current patch for TianoCore#2032? (Per <https://bugzilla.tianocore.org/show_bug.cgi?id=2032#c8>, both BZs share the same CVE ID.)

Also, I remain confused (with comment 11 being the latest one, as of this time, in TianoCore#2032), whether the issue affects IPv4 only, IPv6 only, or both. This patch is only for IPv4, apparently.

If the present patch is related to TianoCore#2032, then please add a mailing list archive link to the BZ, and move the BZ to IN_PROGRESS status.

Laszlo

> 
> diff --git a/NetworkPkg/Ip4Dxe/Ip4Input.c b/NetworkPkg/Ip4Dxe/Ip4Input.c
> index fec242c71f..95fbd01d05 100644
> --- a/NetworkPkg/Ip4Dxe/Ip4Input.c
> +++ b/NetworkPkg/Ip4Dxe/Ip4Input.c
> @@ -1,9 +1,9 @@
>  /** @file
>    IP4 input process.
>  
> -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2020, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
>  
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -709,14 +709,10 @@ Ip4PreProcessPacket (
>    UINT16                    Checksum;
>  
>    //
>    // Check if the IP4 header is correctly formatted.
>    //
> -  if ((*Packet)->TotalSize < IP4_MIN_HEADLEN) {
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
>    HeadLen  = (Head->HeadLen << 2);
>    TotalLen = NTOHS (Head->TotalLen);
>  
>    //
>    // Mnp may deliver frame trailer sequence up, trim it off.
> @@ -806,10 +802,34 @@ Ip4PreProcessPacket (
>    }
>  
>    return EFI_SUCCESS;
>  }
>  
> +/**
> +  This function checks the IPv4 packet length.
> +
> +  @param[in]       Packet          Pointer to the IPv4 Packet to be checked.
> +
> +  @retval TRUE                   The input IPv4 packet length is valid.
> +  @retval FALSE                  The input IPv4 packet length is invalid.
> +
> +**/
> +BOOLEAN
> +Ip4IsValidPacketLength (
> +  IN NET_BUF        *Packet
> +  )
> +{
> +  //
> +  // Check the IP4 packet length.
> +  //
> +  if (Packet->TotalSize < IP4_MIN_HEADLEN) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
>  /**
>    The IP4 input routine. It is called by the IP4_INTERFACE when a
>    IP4 fragment is received from MNP.
>  
>    @param[in]  Ip4Instance        The IP4 child that request the receive, most like
> @@ -842,10 +862,14 @@ Ip4AccpetFrame (
>  
>    if (EFI_ERROR (IoStatus) || (IpSb->State == IP4_SERVICE_DESTROY)) {
>      goto DROP;
>    }
>  
> +  if (!Ip4IsValidPacketLength (Packet)) {
> +    goto RESTART;
> +  }
> +
>    Head      = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL);
>    ASSERT (Head != NULL);
>    OptionLen = (Head->HeadLen << 2) - IP4_MIN_HEADLEN;
>    if (OptionLen > 0) {
>      Option = (UINT8 *) (Head + 1);
> @@ -888,14 +912,18 @@ Ip4AccpetFrame (
>    //
>    // If the packet is protected by tunnel mode, parse the inner Ip Packet.
>    //
>    ZeroMem (&ZeroHead, sizeof (IP4_HEAD));
>    if (0 == CompareMem (Head, &ZeroHead, sizeof (IP4_HEAD))) {
> -  // Packet may have been changed. Head, HeadLen, TotalLen, and
> -  // info must be reloaded before use. The ownership of the packet
> -  // is transferred to the packet process logic.
> -  //
> +    // Packet may have been changed. Head, HeadLen, TotalLen, and
> +    // info must be reloaded before use. The ownership of the packet
> +    // is transferred to the packet process logic.
> +    //
> +    if (!Ip4IsValidPacketLength (Packet)) {
> +      goto RESTART;
> +    }
> +    
>      Head = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL);
>      ASSERT (Head != NULL);
>      Status = Ip4PreProcessPacket (
>                 IpSb,
>                 &Packet,
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54552): https://edk2.groups.io/g/devel/message/54552
Mute This Topic: https://groups.io/mt/71345472/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).

Posted by Liming Gao 4 years, 2 months ago

Another minor comment. Ip4Dxe is moved into NetworkPkg. So, the patch title should be NetworkPkg/Ip4Dxe.

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Monday, February 17, 2020 6:40 PM
> To: devel@edk2.groups.io; Wu, Jiaxin <jiaxin.wu@intel.com>
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Maciej Rabeda <maciej.rabeda@linux.intel.com>; Armour, Nicholas <nicholas.armour@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).
> 
> On 02/17/20 08:43, Wu, Jiaxin wrote:
> > This patch is to check the received package length to make sure the package
> > has a valid length field.
> >
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
> > ---
> >  NetworkPkg/Ip4Dxe/Ip4Input.c | 46 +++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 37 insertions(+), 9 deletions(-)
> 
> There are two patches on the list for CVE-2019-14559:
> 
> - [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).
> - [edk2-devel] [PATCH 1/1] NetworkPkg/ArpDxe: Recycle invalid ARP packets(CVE-2019-14559).
> 
> sent by different submitters.
> 
> How do they relate to each other?
> 
> Also, while Nick's patch mentions TianoCore#2031, the current patch doesn't include a BZ link. Is the current patch for TianoCore#2032?
> (Per <https://bugzilla.tianocore.org/show_bug.cgi?id=2032#c8>, both BZs share the same CVE ID.)
> 
> Also, I remain confused (with comment 11 being the latest one, as of this time, in TianoCore#2032), whether the issue affects IPv4 only,
> IPv6 only, or both. This patch is only for IPv4, apparently.
> 
> If the present patch is related to TianoCore#2032, then please add a mailing list archive link to the BZ, and move the BZ to IN_PROGRESS
> status.
> 
> Laszlo
> 
> >
> > diff --git a/NetworkPkg/Ip4Dxe/Ip4Input.c b/NetworkPkg/Ip4Dxe/Ip4Input.c
> > index fec242c71f..95fbd01d05 100644
> > --- a/NetworkPkg/Ip4Dxe/Ip4Input.c
> > +++ b/NetworkPkg/Ip4Dxe/Ip4Input.c
> > @@ -1,9 +1,9 @@
> >  /** @file
> >    IP4 input process.
> >
> > -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
> > +Copyright (c) 2005 - 2020, Intel Corporation. All rights reserved.<BR>
> >  (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
> >
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -709,14 +709,10 @@ Ip4PreProcessPacket (
> >    UINT16                    Checksum;
> >
> >    //
> >    // Check if the IP4 header is correctly formatted.
> >    //
> > -  if ((*Packet)->TotalSize < IP4_MIN_HEADLEN) {
> > -    return EFI_INVALID_PARAMETER;
> > -  }
> > -
> >    HeadLen  = (Head->HeadLen << 2);
> >    TotalLen = NTOHS (Head->TotalLen);
> >
> >    //
> >    // Mnp may deliver frame trailer sequence up, trim it off.
> > @@ -806,10 +802,34 @@ Ip4PreProcessPacket (
> >    }
> >
> >    return EFI_SUCCESS;
> >  }
> >
> > +/**
> > +  This function checks the IPv4 packet length.
> > +
> > +  @param[in]       Packet          Pointer to the IPv4 Packet to be checked.
> > +
> > +  @retval TRUE                   The input IPv4 packet length is valid.
> > +  @retval FALSE                  The input IPv4 packet length is invalid.
> > +
> > +**/
> > +BOOLEAN
> > +Ip4IsValidPacketLength (
> > +  IN NET_BUF        *Packet
> > +  )
> > +{
> > +  //
> > +  // Check the IP4 packet length.
> > +  //
> > +  if (Packet->TotalSize < IP4_MIN_HEADLEN) {
> > +    return FALSE;
> > +  }
> > +
> > +  return TRUE;
> > +}
> > +
> >  /**
> >    The IP4 input routine. It is called by the IP4_INTERFACE when a
> >    IP4 fragment is received from MNP.
> >
> >    @param[in]  Ip4Instance        The IP4 child that request the receive, most like
> > @@ -842,10 +862,14 @@ Ip4AccpetFrame (
> >
> >    if (EFI_ERROR (IoStatus) || (IpSb->State == IP4_SERVICE_DESTROY)) {
> >      goto DROP;
> >    }
> >
> > +  if (!Ip4IsValidPacketLength (Packet)) {
> > +    goto RESTART;
> > +  }
> > +
> >    Head      = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL);
> >    ASSERT (Head != NULL);
> >    OptionLen = (Head->HeadLen << 2) - IP4_MIN_HEADLEN;
> >    if (OptionLen > 0) {
> >      Option = (UINT8 *) (Head + 1);
> > @@ -888,14 +912,18 @@ Ip4AccpetFrame (
> >    //
> >    // If the packet is protected by tunnel mode, parse the inner Ip Packet.
> >    //
> >    ZeroMem (&ZeroHead, sizeof (IP4_HEAD));
> >    if (0 == CompareMem (Head, &ZeroHead, sizeof (IP4_HEAD))) {
> > -  // Packet may have been changed. Head, HeadLen, TotalLen, and
> > -  // info must be reloaded before use. The ownership of the packet
> > -  // is transferred to the packet process logic.
> > -  //
> > +    // Packet may have been changed. Head, HeadLen, TotalLen, and
> > +    // info must be reloaded before use. The ownership of the packet
> > +    // is transferred to the packet process logic.
> > +    //
> > +    if (!Ip4IsValidPacketLength (Packet)) {
> > +      goto RESTART;
> > +    }
> > +
> >      Head = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL);
> >      ASSERT (Head != NULL);
> >      Status = Ip4PreProcessPacket (
> >                 IpSb,
> >                 &Packet,
> >
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54554): https://edk2.groups.io/g/devel/message/54554
Mute This Topic: https://groups.io/mt/71345472/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).

Posted by Wu, Jiaxin 4 years, 2 months ago

Thanks liming, already corrected in V2.

> -----Original Message-----
> From: Gao, Liming <liming.gao@intel.com>
> Sent: Monday, February 17, 2020 10:27 PM
> To: devel@edk2.groups.io; lersek@redhat.com; Wu, Jiaxin
> <jiaxin.wu@intel.com>
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Maciej Rabeda
> <maciej.rabeda@linux.intel.com>; Armour, Nicholas
> <nicholas.armour@intel.com>
> Subject: RE: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the
> received package length (CVE-2019-14559).
> 
> Another minor comment. Ip4Dxe is moved into NetworkPkg. So, the patch
> title should be NetworkPkg/Ip4Dxe.
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> > Sent: Monday, February 17, 2020 6:40 PM
> > To: devel@edk2.groups.io; Wu, Jiaxin <jiaxin.wu@intel.com>
> > Cc: Fu, Siyuan <siyuan.fu@intel.com>; Maciej Rabeda
> <maciej.rabeda@linux.intel.com>; Armour, Nicholas
> <nicholas.armour@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the
> received package length (CVE-2019-14559).
> >
> > On 02/17/20 08:43, Wu, Jiaxin wrote:
> > > This patch is to check the received package length to make sure the
> package
> > > has a valid length field.
> > >
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
> > > ---
> > >  NetworkPkg/Ip4Dxe/Ip4Input.c | 46
> +++++++++++++++++++++++++++++++++++---------
> > >  1 file changed, 37 insertions(+), 9 deletions(-)
> >
> > There are two patches on the list for CVE-2019-14559:
> >
> > - [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received
> package length (CVE-2019-14559).
> > - [edk2-devel] [PATCH 1/1] NetworkPkg/ArpDxe: Recycle invalid ARP
> packets(CVE-2019-14559).
> >
> > sent by different submitters.
> >
> > How do they relate to each other?
> >
> > Also, while Nick's patch mentions TianoCore#2031, the current patch
> doesn't include a BZ link. Is the current patch for TianoCore#2032?
> > (Per <https://bugzilla.tianocore.org/show_bug.cgi?id=2032#c8>, both BZs
> share the same CVE ID.)
> >
> > Also, I remain confused (with comment 11 being the latest one, as of this
> time, in TianoCore#2032), whether the issue affects IPv4 only,
> > IPv6 only, or both. This patch is only for IPv4, apparently.
> >
> > If the present patch is related to TianoCore#2032, then please add a mailing
> list archive link to the BZ, and move the BZ to IN_PROGRESS
> > status.
> >
> > Laszlo
> >
> > >
> > > diff --git a/NetworkPkg/Ip4Dxe/Ip4Input.c
> b/NetworkPkg/Ip4Dxe/Ip4Input.c
> > > index fec242c71f..95fbd01d05 100644
> > > --- a/NetworkPkg/Ip4Dxe/Ip4Input.c
> > > +++ b/NetworkPkg/Ip4Dxe/Ip4Input.c
> > > @@ -1,9 +1,9 @@
> > >  /** @file
> > >    IP4 input process.
> > >
> > > -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
> > > +Copyright (c) 2005 - 2020, Intel Corporation. All rights reserved.<BR>
> > >  (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
> > >
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > > @@ -709,14 +709,10 @@ Ip4PreProcessPacket (
> > >    UINT16                    Checksum;
> > >
> > >    //
> > >    // Check if the IP4 header is correctly formatted.
> > >    //
> > > -  if ((*Packet)->TotalSize < IP4_MIN_HEADLEN) {
> > > -    return EFI_INVALID_PARAMETER;
> > > -  }
> > > -
> > >    HeadLen  = (Head->HeadLen << 2);
> > >    TotalLen = NTOHS (Head->TotalLen);
> > >
> > >    //
> > >    // Mnp may deliver frame trailer sequence up, trim it off.
> > > @@ -806,10 +802,34 @@ Ip4PreProcessPacket (
> > >    }
> > >
> > >    return EFI_SUCCESS;
> > >  }
> > >
> > > +/**
> > > +  This function checks the IPv4 packet length.
> > > +
> > > +  @param[in]       Packet          Pointer to the IPv4 Packet to be checked.
> > > +
> > > +  @retval TRUE                   The input IPv4 packet length is valid.
> > > +  @retval FALSE                  The input IPv4 packet length is invalid.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +Ip4IsValidPacketLength (
> > > +  IN NET_BUF        *Packet
> > > +  )
> > > +{
> > > +  //
> > > +  // Check the IP4 packet length.
> > > +  //
> > > +  if (Packet->TotalSize < IP4_MIN_HEADLEN) {
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  return TRUE;
> > > +}
> > > +
> > >  /**
> > >    The IP4 input routine. It is called by the IP4_INTERFACE when a
> > >    IP4 fragment is received from MNP.
> > >
> > >    @param[in]  Ip4Instance        The IP4 child that request the receive, most
> like
> > > @@ -842,10 +862,14 @@ Ip4AccpetFrame (
> > >
> > >    if (EFI_ERROR (IoStatus) || (IpSb->State == IP4_SERVICE_DESTROY)) {
> > >      goto DROP;
> > >    }
> > >
> > > +  if (!Ip4IsValidPacketLength (Packet)) {
> > > +    goto RESTART;
> > > +  }
> > > +
> > >    Head      = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL);
> > >    ASSERT (Head != NULL);
> > >    OptionLen = (Head->HeadLen << 2) - IP4_MIN_HEADLEN;
> > >    if (OptionLen > 0) {
> > >      Option = (UINT8 *) (Head + 1);
> > > @@ -888,14 +912,18 @@ Ip4AccpetFrame (
> > >    //
> > >    // If the packet is protected by tunnel mode, parse the inner Ip Packet.
> > >    //
> > >    ZeroMem (&ZeroHead, sizeof (IP4_HEAD));
> > >    if (0 == CompareMem (Head, &ZeroHead, sizeof (IP4_HEAD))) {
> > > -  // Packet may have been changed. Head, HeadLen, TotalLen, and
> > > -  // info must be reloaded before use. The ownership of the packet
> > > -  // is transferred to the packet process logic.
> > > -  //
> > > +    // Packet may have been changed. Head, HeadLen, TotalLen, and
> > > +    // info must be reloaded before use. The ownership of the packet
> > > +    // is transferred to the packet process logic.
> > > +    //
> > > +    if (!Ip4IsValidPacketLength (Packet)) {
> > > +      goto RESTART;
> > > +    }
> > > +
> > >      Head = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL);
> > >      ASSERT (Head != NULL);
> > >      Status = Ip4PreProcessPacket (
> > >                 IpSb,
> > >                 &Packet,
> > >
> >
> >
> > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54564): https://edk2.groups.io/g/devel/message/54564
Mute This Topic: https://groups.io/mt/71345472/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).

Posted by Wu, Jiaxin 4 years, 2 months ago

Hi Laszlo,

Thanks the comments. I have updated the patch to v2.

BZ link is https://bugzilla.tianocore.org/show_bug.cgi?id=1610. According comment 7, the CVE number is CVE-2019-14559 (share with BZ2031).

I guess they are produced by the same test environment. So, they are sharing the same CVE number. It's the same case for BZ2032.

Jian, please correct me if not right.

Thanks,
Jiaxin

> 
> There are two patches on the list for CVE-2019-14559:
> 
> - [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received
> package length (CVE-2019-14559).
> - [edk2-devel] [PATCH 1/1] NetworkPkg/ArpDxe: Recycle invalid ARP
> packets(CVE-2019-14559).
> 
> sent by different submitters.
> 
> How do they relate to each other?
> 
> Also, while Nick's patch mentions TianoCore#2031, the current patch doesn't
> include a BZ link. Is the current patch for TianoCore#2032? (Per
> <https://bugzilla.tianocore.org/show_bug.cgi?id=2032#c8>, both BZs share
> the same CVE ID.)
> 
> Also, I remain confused (with comment 11 being the latest one, as of this
> time, in TianoCore#2032), whether the issue affects IPv4 only, IPv6 only, or
> both. This patch is only for IPv4, apparently.
> 
> If the present patch is related to TianoCore#2032, then please add a mailing
> list archive link to the BZ, and move the BZ to IN_PROGRESS status.
> 
> Laszlo
> 
> >
> > diff --git a/NetworkPkg/Ip4Dxe/Ip4Input.c
> b/NetworkPkg/Ip4Dxe/Ip4Input.c
> > index fec242c71f..95fbd01d05 100644
> > --- a/NetworkPkg/Ip4Dxe/Ip4Input.c
> > +++ b/NetworkPkg/Ip4Dxe/Ip4Input.c
> > @@ -1,9 +1,9 @@
> >  /** @file
> >    IP4 input process.
> >
> > -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
> > +Copyright (c) 2005 - 2020, Intel Corporation. All rights reserved.<BR>
> >  (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
> >
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -709,14 +709,10 @@ Ip4PreProcessPacket (
> >    UINT16                    Checksum;
> >
> >    //
> >    // Check if the IP4 header is correctly formatted.
> >    //
> > -  if ((*Packet)->TotalSize < IP4_MIN_HEADLEN) {
> > -    return EFI_INVALID_PARAMETER;
> > -  }
> > -
> >    HeadLen  = (Head->HeadLen << 2);
> >    TotalLen = NTOHS (Head->TotalLen);
> >
> >    //
> >    // Mnp may deliver frame trailer sequence up, trim it off.
> > @@ -806,10 +802,34 @@ Ip4PreProcessPacket (
> >    }
> >
> >    return EFI_SUCCESS;
> >  }
> >
> > +/**
> > +  This function checks the IPv4 packet length.
> > +
> > +  @param[in]       Packet          Pointer to the IPv4 Packet to be checked.
> > +
> > +  @retval TRUE                   The input IPv4 packet length is valid.
> > +  @retval FALSE                  The input IPv4 packet length is invalid.
> > +
> > +**/
> > +BOOLEAN
> > +Ip4IsValidPacketLength (
> > +  IN NET_BUF        *Packet
> > +  )
> > +{
> > +  //
> > +  // Check the IP4 packet length.
> > +  //
> > +  if (Packet->TotalSize < IP4_MIN_HEADLEN) {
> > +    return FALSE;
> > +  }
> > +
> > +  return TRUE;
> > +}
> > +
> >  /**
> >    The IP4 input routine. It is called by the IP4_INTERFACE when a
> >    IP4 fragment is received from MNP.
> >
> >    @param[in]  Ip4Instance        The IP4 child that request the receive, most
> like
> > @@ -842,10 +862,14 @@ Ip4AccpetFrame (
> >
> >    if (EFI_ERROR (IoStatus) || (IpSb->State == IP4_SERVICE_DESTROY)) {
> >      goto DROP;
> >    }
> >
> > +  if (!Ip4IsValidPacketLength (Packet)) {
> > +    goto RESTART;
> > +  }
> > +
> >    Head      = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL);
> >    ASSERT (Head != NULL);
> >    OptionLen = (Head->HeadLen << 2) - IP4_MIN_HEADLEN;
> >    if (OptionLen > 0) {
> >      Option = (UINT8 *) (Head + 1);
> > @@ -888,14 +912,18 @@ Ip4AccpetFrame (
> >    //
> >    // If the packet is protected by tunnel mode, parse the inner Ip Packet.
> >    //
> >    ZeroMem (&ZeroHead, sizeof (IP4_HEAD));
> >    if (0 == CompareMem (Head, &ZeroHead, sizeof (IP4_HEAD))) {
> > -  // Packet may have been changed. Head, HeadLen, TotalLen, and
> > -  // info must be reloaded before use. The ownership of the packet
> > -  // is transferred to the packet process logic.
> > -  //
> > +    // Packet may have been changed. Head, HeadLen, TotalLen, and
> > +    // info must be reloaded before use. The ownership of the packet
> > +    // is transferred to the packet process logic.
> > +    //
> > +    if (!Ip4IsValidPacketLength (Packet)) {
> > +      goto RESTART;
> > +    }
> > +
> >      Head = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL);
> >      ASSERT (Head != NULL);
> >      Status = Ip4PreProcessPacket (
> >                 IpSb,
> >                 &Packet,
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54563): https://edk2.groups.io/g/devel/message/54563
Mute This Topic: https://groups.io/mt/71345472/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-