* Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).
[not found] <15F4205BB8F7C9F2.5373@groups.io>
@ 2020-02-17 7:39 ` Wu, Jiaxin
0 siblings, 0 replies; 5+ messages in thread
From: Wu, Jiaxin @ 2020-02-17 7:39 UTC (permalink / raw)
To: devel@edk2.groups.io, Wu, Jiaxin; +Cc: Fu, Siyuan
Sorry, please ignore this patch, I will correct the commit log later.
Thanks,
Jiaxin
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
> Jiaxin
> Sent: Monday, February 17, 2020 3:36 PM
> To: devel@edk2.groups.io
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the
> received package length (CVE-2019-14559).
>
> 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>
> Contributed-under: TianoCore Contribution Agreement 1.1
> 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..3fd08a5231 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 - 2019, 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
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).
2020-02-17 7:43 Wu, Jiaxin
@ 2020-02-17 10:39 ` Laszlo Ersek
2020-02-17 14:26 ` Liming Gao
2020-02-17 21:43 ` Wu, Jiaxin
0 siblings, 2 replies; 5+ messages in thread
From: Laszlo Ersek @ 2020-02-17 10:39 UTC (permalink / raw)
To: devel, jiaxin.wu; +Cc: Fu Siyuan, Maciej Rabeda, nicholas.armour
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,
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).
2020-02-17 10:39 ` [edk2-devel] " Laszlo Ersek
@ 2020-02-17 14:26 ` Liming Gao
2020-02-17 21:43 ` Wu, Jiaxin
2020-02-17 21:43 ` Wu, Jiaxin
1 sibling, 1 reply; 5+ messages in thread
From: Liming Gao @ 2020-02-17 14:26 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com, Wu, Jiaxin
Cc: Fu, Siyuan, Maciej Rabeda, Armour, Nicholas
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,
> >
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).
2020-02-17 10:39 ` [edk2-devel] " Laszlo Ersek
2020-02-17 14:26 ` Liming Gao
@ 2020-02-17 21:43 ` Wu, Jiaxin
1 sibling, 0 replies; 5+ messages in thread
From: Wu, Jiaxin @ 2020-02-17 21:43 UTC (permalink / raw)
To: Laszlo Ersek, devel@edk2.groups.io
Cc: Fu, Siyuan, Maciej Rabeda, Armour, Nicholas
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,
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559).
2020-02-17 14:26 ` Liming Gao
@ 2020-02-17 21:43 ` Wu, Jiaxin
0 siblings, 0 replies; 5+ messages in thread
From: Wu, Jiaxin @ 2020-02-17 21:43 UTC (permalink / raw)
To: Gao, Liming, devel@edk2.groups.io, lersek@redhat.com
Cc: Fu, Siyuan, Maciej Rabeda, Armour, Nicholas
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,
> > >
> >
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-17 21:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <15F4205BB8F7C9F2.5373@groups.io>
2020-02-17 7:39 ` [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559) Wu, Jiaxin
2020-02-17 7:43 Wu, Jiaxin
2020-02-17 10:39 ` [edk2-devel] " Laszlo Ersek
2020-02-17 14:26 ` Liming Gao
2020-02-17 21:43 ` Wu, Jiaxin
2020-02-17 21:43 ` Wu, Jiaxin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox