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

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

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,
> >