From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mx.groups.io with SMTP id smtpd.web11.16039.1581975836346227150 for ; Mon, 17 Feb 2020 13:43:56 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: jiaxin.wu@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Feb 2020 13:43:55 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,454,1574150400"; d="scan'208";a="223944807" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga007.jf.intel.com with ESMTP; 17 Feb 2020 13:43:55 -0800 Received: from fmsmsx119.amr.corp.intel.com (10.18.124.207) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 17 Feb 2020 13:43:55 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX119.amr.corp.intel.com (10.18.124.207) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 17 Feb 2020 13:43:54 -0800 Received: from shsmsx107.ccr.corp.intel.com ([169.254.9.46]) by shsmsx102.ccr.corp.intel.com ([169.254.2.126]) with mapi id 14.03.0439.000; Tue, 18 Feb 2020 05:43:51 +0800 From: "Wu, Jiaxin" To: "Gao, Liming" , "devel@edk2.groups.io" , "lersek@redhat.com" CC: "Fu, Siyuan" , Maciej Rabeda , "Armour, Nicholas" Subject: Re: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559). Thread-Topic: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the received package length (CVE-2019-14559). Thread-Index: AQHV5WYQOyNfWycig02XfFfs2xZy6qgerEuAgAA/aICAAQATUA== Date: Mon, 17 Feb 2020 21:43:51 +0000 Message-ID: <895558F6EA4E3B41AC93A00D163B72741701C84A@SHSMSX107.ccr.corp.intel.com> References: <20200217074349.8924-1-Jiaxin.wu@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTJjMzczM2ItNjcwMy00ZDExLWIxMDUtMTNiZTc5MzY4ZDExIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibklMVFwvM3BcL09VSHdlemxMcXYrdTRHTVRiMmhnUFwveWMxWjl2U1htQ1dkWEszc2lqdFRoM1p4Z2F3UFh2OWRQbyJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: jiaxin.wu@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks liming, already corrected in V2. > -----Original Message----- > From: Gao, Liming > Sent: Monday, February 17, 2020 10:27 PM > To: devel@edk2.groups.io; lersek@redhat.com; Wu, Jiaxin > > Cc: Fu, Siyuan ; Maciej Rabeda > ; Armour, Nicholas > > Subject: RE: [edk2-devel] [PATCH v1] MdeModulePkg/Ip4Dxe: Check the > received package length (CVE-2019-14559). >=20 > Another minor comment. Ip4Dxe is moved into NetworkPkg. So, the patch > title should be NetworkPkg/Ip4Dxe. >=20 > Thanks > Liming > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Laszlo > Ersek > > Sent: Monday, February 17, 2020 6:40 PM > > To: devel@edk2.groups.io; Wu, Jiaxin > > Cc: Fu, Siyuan ; Maciej Rabeda > ; Armour, Nicholas > > > 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 > > > Cc: Maciej Rabeda > > > Signed-off-by: Wu Jiaxin > > > Reviewed-by: Siyuan Fu > > > --- > > > 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 , both = BZs > share the same CVE ID.) > > > > Also, I remain confused (with comment 11 being the latest one, as of t= his > 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 m= ailing > 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.
> > > > > > 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 =3D (Head->HeadLen << 2); > > > TotalLen =3D 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 va= lid. > > > + @retval FALSE The input IPv4 packet length is in= valid. > > > + > > > +**/ > > > +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 rec= eive, most > like > > > @@ -842,10 +862,14 @@ Ip4AccpetFrame ( > > > > > > if (EFI_ERROR (IoStatus) || (IpSb->State =3D=3D IP4_SERVICE_DESTR= OY)) { > > > goto DROP; > > > } > > > > > > + if (!Ip4IsValidPacketLength (Packet)) { > > > + goto RESTART; > > > + } > > > + > > > Head =3D (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL); > > > ASSERT (Head !=3D NULL); > > > OptionLen =3D (Head->HeadLen << 2) - IP4_MIN_HEADLEN; > > > if (OptionLen > 0) { > > > Option =3D (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 =3D=3D 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 packe= t > > > + // is transferred to the packet process logic. > > > + // > > > + if (!Ip4IsValidPacketLength (Packet)) { > > > + goto RESTART; > > > + } > > > + > > > Head =3D (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL); > > > ASSERT (Head !=3D NULL); > > > Status =3D Ip4PreProcessPacket ( > > > IpSb, > > > &Packet, > > > > > > > > >=20