* [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in NetIp6IsNetEqual().
@ 2018-01-11 10:14 Wang Fan
2018-01-12 0:46 ` Wu, Jiaxin
2018-01-12 0:50 ` Fu, Siyuan
0 siblings, 2 replies; 3+ messages in thread
From: Wang Fan @ 2018-01-11 10:14 UTC (permalink / raw)
To: edk2-devel; +Cc: Fu Siyuan, Jiaxin Wu, Hao Wu
V2
* Added an ASSERT check for the case PrefixLength equals to IP6_PREFIX_MAX.
* Synced some code descriptions to head file.
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan <fan.wang@intel.com>
---
MdeModulePkg/Include/Library/NetLib.h | 50 +++++++++++++++++++++++++++---
MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 8 +++--
2 files changed, 52 insertions(+), 6 deletions(-)
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.
-Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at<BR>
http://opensource.org/licenses/bsd-license.php
@@ -438,10 +438,12 @@ NetIp4IsUnicast (
);
/**
Check whether the incoming IPv6 address is a valid unicast address.
+ 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 (
/**
Check whether the incoming Ipv6 address is the unspecified address or not.
+ ASSERT if Ip6 is NULL.
+
@param[in] Ip6 - Ip6 address, in network order.
@retval TRUE - Yes, incoming Ipv6 address is the unspecified address.
@retval FALSE - The incoming Ipv6 address is not the unspecified address
@@ -474,10 +478,12 @@ NetIp6IsUnspecifiedAddr (
);
/**
Check whether the incoming Ipv6 address is a link-local address.
+ ASSERT if Ip6 is NULL.
+
@param[in] Ip6 - Ip6 address, in network order.
@retval TRUE - The incoming Ipv6 address is a link-local address.
@retval FALSE - The incoming Ipv6 address is not a link-local address.
@@ -489,10 +495,13 @@ NetIp6IsLinkLocalAddr (
);
/**
Check whether the Ipv6 address1 and address2 are on the connected network.
+ 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.
@retval TRUE - Yes, the Ipv6 address1 and address2 are connected.
@@ -508,10 +517,12 @@ NetIp6IsNetEqual (
);
/**
Switches the endianess of an IPv6 address.
+ 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.
@param Ip6 Points to an IPv6 address.
@@ -542,10 +553,12 @@ extern EFI_IPv4_ADDRESS mZeroIp4Addr;
#define NET_RANDOM(Seed) ((UINT32) ((UINT32) (Seed) * 1103515245UL + 12345) % 4294967295UL)
/**
Extract a UINT32 from a byte stream.
+ 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.
@param[in] Buf The buffer to extract the UINT32.
@@ -559,10 +572,12 @@ NetGetUint32 (
);
/**
Puts a UINT32 into the byte stream in network byte order.
+ ASSERT if Buf is NULL.
+
Converts a UINT32 from host byte order to network byte order, then copies it to the
byte stream.
@param[in, out] Buf The buffer in which to put the UINT32.
@param[in] Data The data to be converted and put into the byte stream.
@@ -675,10 +690,12 @@ NetListRemoveTail (
);
/**
Insert a new node entry after a designated node entry of a doubly linked list.
+ 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.
@param[in, out] PrevEntry The entry after which to insert.
@param[in, out] NewEntry The new entry to insert.
@@ -692,10 +709,12 @@ NetListInsertAfter (
);
/**
Insert a new node entry before a designated node entry of a doubly linked list.
+ 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.
@param[in, out] PostEntry The entry to insert before.
@param[in, out] NewEntry The new entry to insert.
@@ -837,11 +856,10 @@ NetMapClean (
If the number of the <Key, Value> pairs in the netmap is zero, return TRUE.
If Map is NULL, then ASSERT().
-
@param[in] Map The net map to test.
@return TRUE if the netmap is empty, otherwise FALSE.
**/
@@ -852,10 +870,12 @@ NetMapIsEmpty (
);
/**
Return the number of the <Key, Value> pairs in the netmap.
+ If Map is NULL, then ASSERT().
+
@param[in] Map The netmap to get the entry number.
@return The entry number in the netmap.
**/
@@ -871,10 +891,11 @@ NetMapGetCount (
Allocate an item to save the <Key, Value> pair and add corresponding node entry
to the beginning of the Used doubly linked list. The number of the <Key, Value>
pairs in the netmap increase by 1.
If Map is NULL, then ASSERT().
+ If Key is NULL, then ASSERT().
@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.
@@ -896,10 +917,11 @@ NetMapInsertHead (
Allocate an item to save the <Key, Value> pair and add corresponding node entry
to the tail of the Used doubly linked list. The number of the <Key, Value>
pairs in the netmap increase by 1.
If Map is NULL, then ASSERT().
+ If Key is NULL, then ASSERT().
@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.
@@ -920,10 +942,11 @@ NetMapInsertTail (
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.
If Map is NULL, then ASSERT().
+ If Key is NULL, then ASSERT().
@param[in] Map The netmap to search within.
@param[in] Key The key to search.
@return The point to the item contains the Key, or NULL if Key isn't in the map.
@@ -1165,10 +1188,13 @@ NetLibGetVlanHandle (
);
/**
Get MAC address associated with the network service handle.
+ 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.
@param[in] ServiceHandle The handle where network service binding protocols are
@@ -1190,10 +1216,12 @@ NetLibGetMacAddress (
/**
Convert MAC address of the NIC associated with specified Service Binding Handle
to a unicode string. Callers are responsible for freeing the string storage.
+ 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.
@@ -1219,10 +1247,12 @@ NetLibGetMacString (
);
/**
Detect media status for specified network device.
+ 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 SNP 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
);
/**
-
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. When function detects
@@ -1288,10 +1317,12 @@ NetLibDetectMediaWaitTimeout (
/**
Create an IPv4 device path node.
+ 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.
@@ -1319,10 +1350,14 @@ NetLibCreateIPv4DPathNode (
);
/**
Create an IPv6 device path node.
+ 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.
@@ -1349,10 +1384,12 @@ NetLibCreateIPv6DPathNode (
/**
Find the UNDI/SNP handle from controller and protocol GUID.
+ 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 (
);
/**
This function obtains the system guid from the smbios table.
+ If SystemGuid is NULL, then ASSERT().
+
@param[out] SystemGuid The pointer of the returned system guid.
@retval EFI_SUCCESS Successfully obtained the system guid.
@retval EFI_NOT_FOUND Did not find the SMBIOS table.
@@ -2205,11 +2244,14 @@ EFIAPI
NetLibGetSystemGuid (
OUT EFI_GUID *SystemGuid
);
/**
- 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 (
/**
Check whether the Ipv6 address1 and address2 are on the connected network.
ASSERT if Ip1 or Ip2 is NULL.
- ASSERT if PrefixLength exceeds IP6_PREFIX_MAX.
+ 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.
@@ -824,11 +824,11 @@ NetIp6IsNetEqual (
{
UINT8 Byte;
UINT8 Bit;
UINT8 Mask;
- ASSERT ((Ip1 != NULL) && (Ip2 != NULL) && (PrefixLength <= IP6_PREFIX_MAX));
+ ASSERT ((Ip1 != NULL) && (Ip2 != NULL) && (PrefixLength < IP6_PREFIX_MAX));
if (PrefixLength == 0) {
return TRUE;
}
@@ -840,10 +840,14 @@ NetIp6IsNetEqual (
}
if (Bit > 0) {
Mask = (UINT8) (0xFF << (8 - Bit));
+ ASSERT (Byte < 16);
+ if (Byte >= 16) {
+ return FALSE;
+ }
if ((Ip1->Addr[Byte] & Mask) != (Ip2->Addr[Byte] & Mask)) {
return FALSE;
}
}
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in NetIp6IsNetEqual().
2018-01-11 10:14 [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in NetIp6IsNetEqual() Wang Fan
@ 2018-01-12 0:46 ` Wu, Jiaxin
2018-01-12 0:50 ` Fu, Siyuan
1 sibling, 0 replies; 3+ messages in thread
From: Wu, Jiaxin @ 2018-01-12 0:46 UTC (permalink / raw)
To: Wang, Fan, edk2-devel@lists.01.org; +Cc: Fu, Siyuan, Wu, Hao A
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
> -----Original Message-----
> From: Wang, Fan
> Sent: Thursday, January 11, 2018 6:14 PM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Wu,
> Hao A <hao.a.wu@intel.com>
> Subject: [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in
> NetIp6IsNetEqual().
>
> V2
> * Added an ASSERT check for the case PrefixLength equals to
> IP6_PREFIX_MAX.
> * Synced some code descriptions to head file.
>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
> MdeModulePkg/Include/Library/NetLib.h | 50
> +++++++++++++++++++++++++++---
> MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 8 +++--
> 2 files changed, 52 insertions(+), 6 deletions(-)
>
> 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.
>
> -Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at<BR>
> http://opensource.org/licenses/bsd-license.php
>
> @@ -438,10 +438,12 @@ NetIp4IsUnicast (
> );
>
> /**
> Check whether the incoming IPv6 address is a valid unicast address.
>
> + 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 (
>
>
> /**
> Check whether the incoming Ipv6 address is the unspecified address or not.
>
> + ASSERT if Ip6 is NULL.
> +
> @param[in] Ip6 - Ip6 address, in network order.
>
> @retval TRUE - Yes, incoming Ipv6 address is the unspecified address.
> @retval FALSE - The incoming Ipv6 address is not the unspecified address
>
> @@ -474,10 +478,12 @@ NetIp6IsUnspecifiedAddr (
> );
>
> /**
> Check whether the incoming Ipv6 address is a link-local address.
>
> + ASSERT if Ip6 is NULL.
> +
> @param[in] Ip6 - Ip6 address, in network order.
>
> @retval TRUE - The incoming Ipv6 address is a link-local address.
> @retval FALSE - The incoming Ipv6 address is not a link-local address.
>
> @@ -489,10 +495,13 @@ NetIp6IsLinkLocalAddr (
> );
>
> /**
> Check whether the Ipv6 address1 and address2 are on the connected
> network.
>
> + 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.
>
> @retval TRUE - Yes, the Ipv6 address1 and address2 are connected.
> @@ -508,10 +517,12 @@ NetIp6IsNetEqual (
> );
>
> /**
> Switches the endianess of an IPv6 address.
>
> + 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.
>
> @param Ip6 Points to an IPv6 address.
> @@ -542,10 +553,12 @@ extern EFI_IPv4_ADDRESS mZeroIp4Addr;
> #define NET_RANDOM(Seed) ((UINT32) ((UINT32) (Seed) *
> 1103515245UL + 12345) % 4294967295UL)
>
> /**
> Extract a UINT32 from a byte stream.
>
> + 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.
>
> @param[in] Buf The buffer to extract the UINT32.
>
> @@ -559,10 +572,12 @@ NetGetUint32 (
> );
>
> /**
> Puts a UINT32 into the byte stream in network byte order.
>
> + ASSERT if Buf is NULL.
> +
> Converts a UINT32 from host byte order to network byte order, then copies
> it to the
> byte stream.
>
> @param[in, out] Buf The buffer in which to put the UINT32.
> @param[in] Data The data to be converted and put into the byte
> stream.
> @@ -675,10 +690,12 @@ NetListRemoveTail (
> );
>
> /**
> Insert a new node entry after a designated node entry of a doubly linked
> list.
>
> + 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.
>
> @param[in, out] PrevEntry The entry after which to insert.
> @param[in, out] NewEntry The new entry to insert.
> @@ -692,10 +709,12 @@ NetListInsertAfter (
> );
>
> /**
> Insert a new node entry before a designated node entry of a doubly linked
> list.
>
> + 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.
>
> @param[in, out] PostEntry The entry to insert before.
> @param[in, out] NewEntry The new entry to insert.
> @@ -837,11 +856,10 @@ NetMapClean (
>
> If the number of the <Key, Value> pairs in the netmap is zero, return TRUE.
>
> If Map is NULL, then ASSERT().
>
> -
> @param[in] Map The net map to test.
>
> @return TRUE if the netmap is empty, otherwise FALSE.
>
> **/
> @@ -852,10 +870,12 @@ NetMapIsEmpty (
> );
>
> /**
> Return the number of the <Key, Value> pairs in the netmap.
>
> + If Map is NULL, then ASSERT().
> +
> @param[in] Map The netmap to get the entry number.
>
> @return The entry number in the netmap.
>
> **/
> @@ -871,10 +891,11 @@ NetMapGetCount (
> Allocate an item to save the <Key, Value> pair and add corresponding node
> entry
> to the beginning of the Used doubly linked list. The number of the <Key,
> Value>
> pairs in the netmap increase by 1.
>
> If Map is NULL, then ASSERT().
> + If Key is NULL, then ASSERT().
>
> @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.
>
> @@ -896,10 +917,11 @@ NetMapInsertHead (
> Allocate an item to save the <Key, Value> pair and add corresponding node
> entry
> to the tail of the Used doubly linked list. The number of the <Key, Value>
> pairs in the netmap increase by 1.
>
> If Map is NULL, then ASSERT().
> + If Key is NULL, then ASSERT().
>
> @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.
>
> @@ -920,10 +942,11 @@ NetMapInsertTail (
>
> 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.
>
> If Map is NULL, then ASSERT().
> + If Key is NULL, then ASSERT().
>
> @param[in] Map The netmap to search within.
> @param[in] Key The key to search.
>
> @return The point to the item contains the Key, or NULL if Key isn't in the
> map.
> @@ -1165,10 +1188,13 @@ NetLibGetVlanHandle (
> );
>
> /**
> Get MAC address associated with the network service handle.
>
> + 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.
>
> @param[in] ServiceHandle The handle where network service binding
> protocols are
> @@ -1190,10 +1216,12 @@ NetLibGetMacAddress (
>
> /**
> Convert MAC address of the NIC associated with specified Service Binding
> Handle
> to a unicode string. Callers are responsible for freeing the string storage.
>
> + 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.
>
> @@ -1219,10 +1247,12 @@ NetLibGetMacString (
> );
>
> /**
> Detect media status for specified network device.
>
> + 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 SNP 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
> );
>
> /**
> -
> 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. When
> function detects
> @@ -1288,10 +1317,12 @@ NetLibDetectMediaWaitTimeout (
>
>
> /**
> Create an IPv4 device path node.
>
> + 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.
>
> @@ -1319,10 +1350,14 @@ NetLibCreateIPv4DPathNode (
> );
>
> /**
> Create an IPv6 device path node.
>
> + 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.
>
> @@ -1349,10 +1384,12 @@ NetLibCreateIPv6DPathNode (
>
>
> /**
> Find the UNDI/SNP handle from controller and protocol GUID.
>
> + 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 (
> );
>
> /**
> This function obtains the system guid from the smbios table.
>
> + If SystemGuid is NULL, then ASSERT().
> +
> @param[out] SystemGuid The pointer of the returned system guid.
>
> @retval EFI_SUCCESS Successfully obtained the system guid.
> @retval EFI_NOT_FOUND Did not find the SMBIOS table.
>
> @@ -2205,11 +2244,14 @@ EFIAPI
> NetLibGetSystemGuid (
> OUT EFI_GUID *SystemGuid
> );
>
> /**
> - 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 (
>
> /**
> Check whether the Ipv6 address1 and address2 are on the connected
> network.
>
> ASSERT if Ip1 or Ip2 is NULL.
> - ASSERT if PrefixLength exceeds IP6_PREFIX_MAX.
> + 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.
>
> @@ -824,11 +824,11 @@ NetIp6IsNetEqual (
> {
> UINT8 Byte;
> UINT8 Bit;
> UINT8 Mask;
>
> - ASSERT ((Ip1 != NULL) && (Ip2 != NULL) && (PrefixLength <=
> IP6_PREFIX_MAX));
> + ASSERT ((Ip1 != NULL) && (Ip2 != NULL) && (PrefixLength <
> IP6_PREFIX_MAX));
>
> if (PrefixLength == 0) {
> return TRUE;
> }
>
> @@ -840,10 +840,14 @@ NetIp6IsNetEqual (
> }
>
> if (Bit > 0) {
> Mask = (UINT8) (0xFF << (8 - Bit));
>
> + ASSERT (Byte < 16);
> + if (Byte >= 16) {
> + return FALSE;
> + }
> if ((Ip1->Addr[Byte] & Mask) != (Ip2->Addr[Byte] & Mask)) {
> return FALSE;
> }
> }
>
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in NetIp6IsNetEqual().
2018-01-11 10:14 [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in NetIp6IsNetEqual() Wang Fan
2018-01-12 0:46 ` Wu, Jiaxin
@ 2018-01-12 0:50 ` Fu, Siyuan
1 sibling, 0 replies; 3+ messages in thread
From: Fu, Siyuan @ 2018-01-12 0:50 UTC (permalink / raw)
To: Wang, Fan, edk2-devel@lists.01.org; +Cc: Wu, Jiaxin, Wu, Hao A
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: Wang, Fan
> Sent: Thursday, January 11, 2018 6:14 PM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Wu,
> Hao A <hao.a.wu@intel.com>
> Subject: [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in
> NetIp6IsNetEqual().
>
> V2
> * Added an ASSERT check for the case PrefixLength equals to IP6_PREFIX_MAX.
> * Synced some code descriptions to head file.
>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
> MdeModulePkg/Include/Library/NetLib.h | 50
> +++++++++++++++++++++++++++---
> MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 8 +++--
> 2 files changed, 52 insertions(+), 6 deletions(-)
>
> 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.
>
> -Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at<BR>
> http://opensource.org/licenses/bsd-license.php
>
> @@ -438,10 +438,12 @@ NetIp4IsUnicast (
> );
>
> /**
> Check whether the incoming IPv6 address is a valid unicast address.
>
> + 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 (
>
>
> /**
> Check whether the incoming Ipv6 address is the unspecified address or
> not.
>
> + ASSERT if Ip6 is NULL.
> +
> @param[in] Ip6 - Ip6 address, in network order.
>
> @retval TRUE - Yes, incoming Ipv6 address is the unspecified
> address.
> @retval FALSE - The incoming Ipv6 address is not the unspecified
> address
>
> @@ -474,10 +478,12 @@ NetIp6IsUnspecifiedAddr (
> );
>
> /**
> Check whether the incoming Ipv6 address is a link-local address.
>
> + ASSERT if Ip6 is NULL.
> +
> @param[in] Ip6 - Ip6 address, in network order.
>
> @retval TRUE - The incoming Ipv6 address is a link-local address.
> @retval FALSE - The incoming Ipv6 address is not a link-local address.
>
> @@ -489,10 +495,13 @@ NetIp6IsLinkLocalAddr (
> );
>
> /**
> Check whether the Ipv6 address1 and address2 are on the connected
> network.
>
> + 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.
>
> @retval TRUE - Yes, the Ipv6 address1 and address2 are
> connected.
> @@ -508,10 +517,12 @@ NetIp6IsNetEqual (
> );
>
> /**
> Switches the endianess of an IPv6 address.
>
> + 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.
>
> @param Ip6 Points to an IPv6 address.
> @@ -542,10 +553,12 @@ extern EFI_IPv4_ADDRESS mZeroIp4Addr;
> #define NET_RANDOM(Seed) ((UINT32) ((UINT32) (Seed) * 1103515245UL
> + 12345) % 4294967295UL)
>
> /**
> Extract a UINT32 from a byte stream.
>
> + 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.
>
> @param[in] Buf The buffer to extract the UINT32.
>
> @@ -559,10 +572,12 @@ NetGetUint32 (
> );
>
> /**
> Puts a UINT32 into the byte stream in network byte order.
>
> + ASSERT if Buf is NULL.
> +
> Converts a UINT32 from host byte order to network byte order, then
> copies it to the
> byte stream.
>
> @param[in, out] Buf The buffer in which to put the UINT32.
> @param[in] Data The data to be converted and put into the
> byte stream.
> @@ -675,10 +690,12 @@ NetListRemoveTail (
> );
>
> /**
> Insert a new node entry after a designated node entry of a doubly
> linked list.
>
> + 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.
>
> @param[in, out] PrevEntry The entry after which to insert.
> @param[in, out] NewEntry The new entry to insert.
> @@ -692,10 +709,12 @@ NetListInsertAfter (
> );
>
> /**
> Insert a new node entry before a designated node entry of a doubly
> linked list.
>
> + 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.
>
> @param[in, out] PostEntry The entry to insert before.
> @param[in, out] NewEntry The new entry to insert.
> @@ -837,11 +856,10 @@ NetMapClean (
>
> If the number of the <Key, Value> pairs in the netmap is zero, return
> TRUE.
>
> If Map is NULL, then ASSERT().
>
> -
> @param[in] Map The net map to test.
>
> @return TRUE if the netmap is empty, otherwise FALSE.
>
> **/
> @@ -852,10 +870,12 @@ NetMapIsEmpty (
> );
>
> /**
> Return the number of the <Key, Value> pairs in the netmap.
>
> + If Map is NULL, then ASSERT().
> +
> @param[in] Map The netmap to get the entry number.
>
> @return The entry number in the netmap.
>
> **/
> @@ -871,10 +891,11 @@ NetMapGetCount (
> Allocate an item to save the <Key, Value> pair and add corresponding
> node entry
> to the beginning of the Used doubly linked list. The number of the <Key,
> Value>
> pairs in the netmap increase by 1.
>
> If Map is NULL, then ASSERT().
> + If Key is NULL, then ASSERT().
>
> @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.
>
> @@ -896,10 +917,11 @@ NetMapInsertHead (
> Allocate an item to save the <Key, Value> pair and add corresponding
> node entry
> to the tail of the Used doubly linked list. The number of the <Key,
> Value>
> pairs in the netmap increase by 1.
>
> If Map is NULL, then ASSERT().
> + If Key is NULL, then ASSERT().
>
> @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.
>
> @@ -920,10 +942,11 @@ NetMapInsertTail (
>
> 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.
>
> If Map is NULL, then ASSERT().
> + If Key is NULL, then ASSERT().
>
> @param[in] Map The netmap to search within.
> @param[in] Key The key to search.
>
> @return The point to the item contains the Key, or NULL if Key isn't in
> the map.
> @@ -1165,10 +1188,13 @@ NetLibGetVlanHandle (
> );
>
> /**
> Get MAC address associated with the network service handle.
>
> + 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.
>
> @param[in] ServiceHandle The handle where network service binding
> protocols are
> @@ -1190,10 +1216,12 @@ NetLibGetMacAddress (
>
> /**
> Convert MAC address of the NIC associated with specified Service
> Binding Handle
> to a unicode string. Callers are responsible for freeing the string
> storage.
>
> + 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.
>
> @@ -1219,10 +1247,12 @@ NetLibGetMacString (
> );
>
> /**
> Detect media status for specified network device.
>
> + 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 SNP
> 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
> );
>
> /**
> -
> 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. When
> function detects
> @@ -1288,10 +1317,12 @@ NetLibDetectMediaWaitTimeout (
>
>
> /**
> Create an IPv4 device path node.
>
> + 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.
>
> @@ -1319,10 +1350,14 @@ NetLibCreateIPv4DPathNode (
> );
>
> /**
> Create an IPv6 device path node.
>
> + 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.
>
> @@ -1349,10 +1384,12 @@ NetLibCreateIPv6DPathNode (
>
>
> /**
> Find the UNDI/SNP handle from controller and protocol GUID.
>
> + 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 (
> );
>
> /**
> This function obtains the system guid from the smbios table.
>
> + If SystemGuid is NULL, then ASSERT().
> +
> @param[out] SystemGuid The pointer of the returned system guid.
>
> @retval EFI_SUCCESS Successfully obtained the system guid.
> @retval EFI_NOT_FOUND Did not find the SMBIOS table.
>
> @@ -2205,11 +2244,14 @@ EFIAPI
> NetLibGetSystemGuid (
> OUT EFI_GUID *SystemGuid
> );
>
> /**
> - 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 (
>
> /**
> Check whether the Ipv6 address1 and address2 are on the connected
> network.
>
> ASSERT if Ip1 or Ip2 is NULL.
> - ASSERT if PrefixLength exceeds IP6_PREFIX_MAX.
> + 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.
>
> @@ -824,11 +824,11 @@ NetIp6IsNetEqual (
> {
> UINT8 Byte;
> UINT8 Bit;
> UINT8 Mask;
>
> - ASSERT ((Ip1 != NULL) && (Ip2 != NULL) && (PrefixLength <=
> IP6_PREFIX_MAX));
> + ASSERT ((Ip1 != NULL) && (Ip2 != NULL) && (PrefixLength <
> IP6_PREFIX_MAX));
>
> if (PrefixLength == 0) {
> return TRUE;
> }
>
> @@ -840,10 +840,14 @@ NetIp6IsNetEqual (
> }
>
> if (Bit > 0) {
> Mask = (UINT8) (0xFF << (8 - Bit));
>
> + ASSERT (Byte < 16);
> + if (Byte >= 16) {
> + return FALSE;
> + }
> if ((Ip1->Addr[Byte] & Mask) != (Ip2->Addr[Byte] & Mask)) {
> return FALSE;
> }
> }
>
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-01-12 0:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-11 10:14 [Patch v2] MdeModulePkg/DxeNetLib: Add array range check in NetIp6IsNetEqual() Wang Fan
2018-01-12 0:46 ` Wu, Jiaxin
2018-01-12 0:50 ` Fu, Siyuan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox