From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "Gao, Liming" <liming.gao@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"lersek@redhat.com" <lersek@redhat.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).
Date: Mon, 17 Feb 2020 21:43:51 +0000 [thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B72741701C84A@SHSMSX107.ccr.corp.intel.com> (raw)
In-Reply-To: <bf608603ff3741d296934b27ed8e8f69@intel.com>
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,
> > >
> >
> >
> >
next prev parent reply other threads:[~2020-02-17 21:43 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-17 7:43 [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559) 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 [this message]
2020-02-17 21:43 ` Wu, Jiaxin
[not found] <15F4205BB8F7C9F2.5373@groups.io>
2020-02-17 7:39 ` Wu, Jiaxin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=895558F6EA4E3B41AC93A00D163B72741701C84A@SHSMSX107.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox