From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=siyuan.fu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0889721F833A9 for ; Thu, 11 Jan 2018 16:45:12 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jan 2018 16:50:26 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,346,1511856000"; d="scan'208";a="193155152" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga005.jf.intel.com with ESMTP; 11 Jan 2018 16:50:26 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 11 Jan 2018 16:50:26 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.159]) with mapi id 14.03.0319.002; Fri, 12 Jan 2018 08:50:24 +0800 From: "Fu, Siyuan" To: "Wang, Fan" , "edk2-devel@lists.01.org" CC: "Wu, Jiaxin" , "Wu, Hao A" Thread-Topic: [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in NetIp6IsNetEqual(). Thread-Index: AQHTisW7E912J3aBJUyq18lnTIt5tKNvaRQA Date: Fri, 12 Jan 2018 00:50:24 +0000 Message-ID: References: <1515665669-6404-1-git-send-email-fan.wang@intel.com> In-Reply-To: <1515665669-6404-1-git-send-email-fan.wang@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiM2FiM2ZhZjAtOTJhZi00ZDNmLTlmODEtZWUyMWQ2YjM5NWVmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJvWlRcL2JRVWlsd0JOR2s1OEVRV3hWV3ZoNlROUDRXZEt3MDFMSUd3blp5a3liaEFjSzI2Q3FZTUgyeENRR3UrSSJ9 dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in NetIp6IsNetEqual(). X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Jan 2018 00:45:13 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Fu Siyuan > -----Original Message----- > From: Wang, Fan > Sent: Thursday, January 11, 2018 6:14 PM > To: edk2-devel@lists.01.org > Cc: Fu, Siyuan ; Wu, Jiaxin ; W= u, > Hao A > Subject: [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in > NetIp6IsNetEqual(). >=20 > V2 > * Added an ASSERT check for the case PrefixLength equals to IP6_PREFIX_MA= X. > * Synced some code descriptions to head file. >=20 > Cc: Fu Siyuan > Cc: Jiaxin Wu > Cc: Hao Wu > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Wang Fan > --- > MdeModulePkg/Include/Library/NetLib.h | 50 > +++++++++++++++++++++++++++--- > MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 8 +++-- > 2 files changed, 52 insertions(+), 6 deletions(-) >=20 > diff --git a/MdeModulePkg/Include/Library/NetLib.h > b/MdeModulePkg/Include/Library/NetLib.h > index 7862df9..b0bbaf2 100644 > --- a/MdeModulePkg/Include/Library/NetLib.h > +++ b/MdeModulePkg/Include/Library/NetLib.h > @@ -1,10 +1,10 @@ > /** @file > This library is only intended to be used by UEFI network stack modules= . > It provides basic functions for the UEFI network stack. >=20 > -Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BS= D > License > which accompanies this distribution. The full text of the license may b= e > found at
> http://opensource.org/licenses/bsd-license.php >=20 > @@ -438,10 +438,12 @@ NetIp4IsUnicast ( > ); >=20 > /** > Check whether the incoming IPv6 address is a valid unicast address. >=20 > + ASSERT if Ip6 is NULL. > + > If the address is a multicast address has binary 0xFF at the start, it > is not > a valid unicast address. If the address is unspecified ::, it is not a > valid > unicast address to be assigned to any node. If the address is loopback > address > ::1, it is also not a valid unicast address to be assigned to any > physical > interface. > @@ -459,10 +461,12 @@ NetIp6IsValidUnicast ( >=20 >=20 > /** > Check whether the incoming Ipv6 address is the unspecified address or > not. >=20 > + ASSERT if Ip6 is NULL. > + > @param[in] Ip6 - Ip6 address, in network order. >=20 > @retval TRUE - Yes, incoming Ipv6 address is the unspecified > address. > @retval FALSE - The incoming Ipv6 address is not the unspecified > address >=20 > @@ -474,10 +478,12 @@ NetIp6IsUnspecifiedAddr ( > ); >=20 > /** > Check whether the incoming Ipv6 address is a link-local address. >=20 > + ASSERT if Ip6 is NULL. > + > @param[in] Ip6 - Ip6 address, in network order. >=20 > @retval TRUE - The incoming Ipv6 address is a link-local address. > @retval FALSE - The incoming Ipv6 address is not a link-local address. >=20 > @@ -489,10 +495,13 @@ NetIp6IsLinkLocalAddr ( > ); >=20 > /** > Check whether the Ipv6 address1 and address2 are on the connected > network. >=20 > + ASSERT if Ip1 or Ip2 is NULL. > + ASSERT if PrefixLength exceeds or equals to IP6_PREFIX_MAX. > + > @param[in] Ip1 - Ip6 address1, in network order. > @param[in] Ip2 - Ip6 address2, in network order. > @param[in] PrefixLength - The prefix length of the checking net. >=20 > @retval TRUE - Yes, the Ipv6 address1 and address2 are > connected. > @@ -508,10 +517,12 @@ NetIp6IsNetEqual ( > ); >=20 > /** > Switches the endianess of an IPv6 address. >=20 > + ASSERT if Ip6 is NULL. > + > This function swaps the bytes in a 128-bit IPv6 address to switch the > value > from little endian to big endian or vice versa. The byte swapped value > is > returned. >=20 > @param Ip6 Points to an IPv6 address. > @@ -542,10 +553,12 @@ extern EFI_IPv4_ADDRESS mZeroIp4Addr; > #define NET_RANDOM(Seed) ((UINT32) ((UINT32) (Seed) * 1103515245U= L > + 12345) % 4294967295UL) >=20 > /** > Extract a UINT32 from a byte stream. >=20 > + ASSERT if Buf is NULL. > + > This function copies a UINT32 from a byte stream, and then converts it > from Network > byte order to host byte order. Use this function to avoid alignment > error. >=20 > @param[in] Buf The buffer to extract the UINT32. >=20 > @@ -559,10 +572,12 @@ NetGetUint32 ( > ); >=20 > /** > Puts a UINT32 into the byte stream in network byte order. >=20 > + ASSERT if Buf is NULL. > + > Converts a UINT32 from host byte order to network byte order, then > copies it to the > byte stream. >=20 > @param[in, out] Buf The buffer in which to put the UINT32. > @param[in] Data The data to be converted and put into th= e > byte stream. > @@ -675,10 +690,12 @@ NetListRemoveTail ( > ); >=20 > /** > Insert a new node entry after a designated node entry of a doubly > linked list. >=20 > + ASSERT if PrevEntry or NewEntry is NULL. > + > Inserts a new node entry designated by NewEntry after the node entry > designated by PrevEntry > of the doubly linked list. >=20 > @param[in, out] PrevEntry The entry after which to insert= . > @param[in, out] NewEntry The new entry to insert. > @@ -692,10 +709,12 @@ NetListInsertAfter ( > ); >=20 > /** > Insert a new node entry before a designated node entry of a doubly > linked list. >=20 > + ASSERT if PostEntry or NewEntry is NULL. > + > Inserts a new node entry designated by NewEntry before the node entry > designated by PostEntry > of the doubly linked list. >=20 > @param[in, out] PostEntry The entry to insert before. > @param[in, out] NewEntry The new entry to insert. > @@ -837,11 +856,10 @@ NetMapClean ( >=20 > If the number of the pairs in the netmap is zero, return > TRUE. >=20 > If Map is NULL, then ASSERT(). >=20 > - > @param[in] Map The net map to test. >=20 > @return TRUE if the netmap is empty, otherwise FALSE. >=20 > **/ > @@ -852,10 +870,12 @@ NetMapIsEmpty ( > ); >=20 > /** > Return the number of the pairs in the netmap. >=20 > + If Map is NULL, then ASSERT(). > + > @param[in] Map The netmap to get the entry number. >=20 > @return The entry number in the netmap. >=20 > **/ > @@ -871,10 +891,11 @@ NetMapGetCount ( > Allocate an item to save the pair and add corresponding > node entry > to the beginning of the Used doubly linked list. The number of the Value> > pairs in the netmap increase by 1. >=20 > If Map is NULL, then ASSERT(). > + If Key is NULL, then ASSERT(). >=20 > @param[in, out] Map The netmap to insert into. > @param[in] Key The user's key. > @param[in] Value The user's value for the key. >=20 > @@ -896,10 +917,11 @@ NetMapInsertHead ( > Allocate an item to save the pair and add corresponding > node entry > to the tail of the Used doubly linked list. The number of the Value> > pairs in the netmap increase by 1. >=20 > If Map is NULL, then ASSERT(). > + If Key is NULL, then ASSERT(). >=20 > @param[in, out] Map The netmap to insert into. > @param[in] Key The user's key. > @param[in] Value The user's value for the key. >=20 > @@ -920,10 +942,11 @@ NetMapInsertTail ( >=20 > Iterate the Used doubly linked list of the netmap to get every item. > Compare the key of every > item with the key to search. It returns the point to the item contains > the Key if found. >=20 > If Map is NULL, then ASSERT(). > + If Key is NULL, then ASSERT(). >=20 > @param[in] Map The netmap to search within. > @param[in] Key The key to search. >=20 > @return The point to the item contains the Key, or NULL if Key isn't i= n > the map. > @@ -1165,10 +1188,13 @@ NetLibGetVlanHandle ( > ); >=20 > /** > Get MAC address associated with the network service handle. >=20 > + If MacAddress is NULL, then ASSERT(). > + If AddressSize is NULL, then ASSERT(). > + > There should be MNP Service Binding Protocol installed on the input > ServiceHandle. > If SNP is installed on the ServiceHandle or its parent handle, MAC > address will > be retrieved from SNP. If no SNP found, try to get SNP mode data use > MNP. >=20 > @param[in] ServiceHandle The handle where network service binding > protocols are > @@ -1190,10 +1216,12 @@ NetLibGetMacAddress ( >=20 > /** > Convert MAC address of the NIC associated with specified Service > Binding Handle > to a unicode string. Callers are responsible for freeing the string > storage. >=20 > + If MacString is NULL, then ASSERT(). > + > Locate simple network protocol associated with the Service Binding > Handle and > get the mac address from SNP. Then convert the mac address into a > unicode > string. It takes 2 unicode characters to represent a 1 byte binary > buffer. > Plus one unicode character for the null-terminator. >=20 > @@ -1219,10 +1247,12 @@ NetLibGetMacString ( > ); >=20 > /** > Detect media status for specified network device. >=20 > + If MediaPresent is NULL, then ASSERT(). > + > The underlying UNDI driver may or may not support reporting media > status from > GET_STATUS command (PXE_STATFLAGS_GET_STATUS_NO_MEDIA_SUPPORTED). This > routine > will try to invoke Snp->GetStatus() to get the media status. If media > is already > present, it returns directly. If media is not present, it will stop SN= P > and then > restart SNP to get the latest media status. This provides an > opportunity to get > @@ -1252,11 +1282,10 @@ NetLibDetectMedia ( > IN EFI_HANDLE ServiceHandle, > OUT BOOLEAN *MediaPresent > ); >=20 > /** > - > Detect media state for a network device. This routine will wait for a > period of time at > a specified checking interval when a certain network is under > connecting until connection > process finishes or timeout. If Aip protocol is supported by low layer > drivers, three kinds > of media states can be detected: EFI_SUCCESS, EFI_NOT_READY and > EFI_NO_MEDIA, represents > connected state, connecting state and no media state respectively. Whe= n > function detects > @@ -1288,10 +1317,12 @@ NetLibDetectMediaWaitTimeout ( >=20 >=20 > /** > Create an IPv4 device path node. >=20 > + If Node is NULL, then ASSERT(). > + > The header type of IPv4 device path node is MESSAGING_DEVICE_PATH. > The header subtype of IPv4 device path node is MSG_IPv4_DP. > The length of the IPv4 device path node in bytes is 19. > Get other information from parameters to make up the whole IPv4 device > path node. >=20 > @@ -1319,10 +1350,14 @@ NetLibCreateIPv4DPathNode ( > ); >=20 > /** > Create an IPv6 device path node. >=20 > + If Node is NULL, then ASSERT(). > + If LocalIp is NULL, then ASSERT(). > + If RemoteIp is NULL, then ASSERT(). > + > The header type of IPv6 device path node is MESSAGING_DEVICE_PATH. > The header subtype of IPv6 device path node is MSG_IPv6_DP. > The length of the IPv6 device path node in bytes is 43. > Get other information from parameters to make up the whole IPv6 device > path node. >=20 > @@ -1349,10 +1384,12 @@ NetLibCreateIPv6DPathNode ( >=20 >=20 > /** > Find the UNDI/SNP handle from controller and protocol GUID. >=20 > + If ProtocolGuid is NULL, then ASSERT(). > + > For example, IP will open an MNP child to transmit/receive > packets. When MNP is stopped, IP should also be stopped. IP > needs to find its own private data that is related the IP's > service binding instance that is installed on the UNDI/SNP handle. > The controller is then either an MNP or an ARP child handle. Note that > @@ -2192,10 +2229,12 @@ NetIpSecNetbufFree ( > ); >=20 > /** > This function obtains the system guid from the smbios table. >=20 > + If SystemGuid is NULL, then ASSERT(). > + > @param[out] SystemGuid The pointer of the returned system guid. >=20 > @retval EFI_SUCCESS Successfully obtained the system guid. > @retval EFI_NOT_FOUND Did not find the SMBIOS table. >=20 > @@ -2205,11 +2244,14 @@ EFIAPI > NetLibGetSystemGuid ( > OUT EFI_GUID *SystemGuid > ); >=20 > /** > - Create Dns QName according the queried domain name. > + Create Dns QName according the queried domain name. > + > + If DomainName is NULL, then ASSERT(). > + > QName is a domain name represented as a sequence of labels, > where each label consists of a length octet followed by that > number of octets. The QName terminates with the zero > length octet for the null label of the root. Caller should > take responsibility to free the buffer in returned pointer. > diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c > b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c > index cbce28f..90d2e3e 100644 > --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c > +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c > @@ -802,11 +802,11 @@ NetIp6IsLinkLocalAddr ( >=20 > /** > Check whether the Ipv6 address1 and address2 are on the connected > network. >=20 > ASSERT if Ip1 or Ip2 is NULL. > - ASSERT if PrefixLength exceeds IP6_PREFIX_MAX. > + ASSERT if PrefixLength exceeds or equals to IP6_PREFIX_MAX. >=20 > @param[in] Ip1 - Ip6 address1, in network order. > @param[in] Ip2 - Ip6 address2, in network order. > @param[in] PrefixLength - The prefix length of the checking net. >=20 > @@ -824,11 +824,11 @@ NetIp6IsNetEqual ( > { > UINT8 Byte; > UINT8 Bit; > UINT8 Mask; >=20 > - ASSERT ((Ip1 !=3D NULL) && (Ip2 !=3D NULL) && (PrefixLength <=3D > IP6_PREFIX_MAX)); > + ASSERT ((Ip1 !=3D NULL) && (Ip2 !=3D NULL) && (PrefixLength < > IP6_PREFIX_MAX)); >=20 > if (PrefixLength =3D=3D 0) { > return TRUE; > } >=20 > @@ -840,10 +840,14 @@ NetIp6IsNetEqual ( > } >=20 > if (Bit > 0) { > Mask =3D (UINT8) (0xFF << (8 - Bit)); >=20 > + ASSERT (Byte < 16); > + if (Byte >=3D 16) { > + return FALSE; > + } > if ((Ip1->Addr[Byte] & Mask) !=3D (Ip2->Addr[Byte] & Mask)) { > return FALSE; > } > } >=20 > -- > 1.9.5.msysgit.1