[patch] MdePkg: check Length para before use in DevPathToTextUsbWWID

[patch] MdePkg: check Length para before use in DevPathToTextUsbWWID

[Dandan Bi]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1336

In function DevPathToTextUsbWWID, the Length parameter is used
without check. This patch is to add check before using it.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdePkg/Library/UefiDevicePathLib/DevicePathToText.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c b/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
index 97d279eeb2..678f3d0a92 100644
--- a/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
+++ b/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
@@ -970,11 +970,11 @@ DevPathToTextUsbWWID (
 
   UsbWWId = DevPath;
 
   SerialNumberStr = (CHAR16 *) ((UINT8 *) UsbWWId + sizeof (USB_WWID_DEVICE_PATH));
   Length = (UINT16) ((DevicePathNodeLength ((EFI_DEVICE_PATH_PROTOCOL *) UsbWWId) - sizeof (USB_WWID_DEVICE_PATH)) / sizeof (CHAR16));
-  if (SerialNumberStr [Length - 1] != 0) {
+  if (Length >= 1 && SerialNumberStr [Length - 1] != 0) {
     //
     // In case no NULL terminator in SerialNumber, create a new one with NULL terminator
     //
     NewStr = AllocateCopyPool ((Length + 1) * sizeof (CHAR16), SerialNumberStr);
     ASSERT (NewStr != NULL);
-- 
2.18.0.windows.1

[patch] MdePkg: Check input Ptrs in GetSectionFromAnyFvByFileType

[Dandan Bi]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1337

In function GetSectionFromAnyFvByFileType, the input parameter "Buffer"
and "size" should not be NULL, so add ASSERT here to avoid any checker
report that the NULL pointer may be used.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdePkg/Library/DxeServicesLib/DxeServicesLib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
index d4f366425f..e78d51cb92 100644
--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
@@ -251,10 +251,13 @@ GetSectionFromAnyFvByFileType  (
   UINTN                         Key;
   EFI_GUID                      NameGuid;
   EFI_FV_FILE_ATTRIBUTES        Attributes;
   EFI_FIRMWARE_VOLUME2_PROTOCOL *Fv;
 
+  ASSERT (Buffer != NULL);
+  ASSERT (Size != NULL);
+
   //
   // Locate all available FVs.
   //
   HandleBuffer = NULL;
   Status = gBS->LocateHandleBuffer (
-- 
2.18.0.windows.1

Re: [patch] MdePkg: Check input Ptrs in GetSectionFromAnyFvByFileType

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Monday, November 19, 2018 10:09 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [patch] MdePkg: Check input Ptrs in GetSectionFromAnyFvByFileType
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1337
> 
> In function GetSectionFromAnyFvByFileType, the input parameter "Buffer"
> and "size" should not be NULL, so add ASSERT here to avoid any checker
> report that the NULL pointer may be used.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> index d4f366425f..e78d51cb92 100644
> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> @@ -251,10 +251,13 @@ GetSectionFromAnyFvByFileType  (
>    UINTN                         Key;
>    EFI_GUID                      NameGuid;
>    EFI_FV_FILE_ATTRIBUTES        Attributes;
>    EFI_FIRMWARE_VOLUME2_PROTOCOL *Fv;
> 
> +  ASSERT (Buffer != NULL);
> +  ASSERT (Size != NULL);
> +
>    //
>    // Locate all available FVs.
>    //
>    HandleBuffer = NULL;
>    Status = gBS->LocateHandleBuffer (
> --
> 2.18.0.windows.1

Re: [patch] MdePkg: check Length para before use in DevPathToTextUsbWWID

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Monday, November 19, 2018 10:09 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [patch] MdePkg: check Length para before use in DevPathToTextUsbWWID
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1336
> 
> In function DevPathToTextUsbWWID, the Length parameter is used
> without check. This patch is to add check before using it.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  MdePkg/Library/UefiDevicePathLib/DevicePathToText.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c b/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> index 97d279eeb2..678f3d0a92 100644
> --- a/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> @@ -970,11 +970,11 @@ DevPathToTextUsbWWID (
> 
>    UsbWWId = DevPath;
> 
>    SerialNumberStr = (CHAR16 *) ((UINT8 *) UsbWWId + sizeof (USB_WWID_DEVICE_PATH));
>    Length = (UINT16) ((DevicePathNodeLength ((EFI_DEVICE_PATH_PROTOCOL *) UsbWWId) - sizeof (USB_WWID_DEVICE_PATH)) /
> sizeof (CHAR16));
> -  if (SerialNumberStr [Length - 1] != 0) {
> +  if (Length >= 1 && SerialNumberStr [Length - 1] != 0) {
>      //
>      // In case no NULL terminator in SerialNumber, create a new one with NULL terminator
>      //
>      NewStr = AllocateCopyPool ((Length + 1) * sizeof (CHAR16), SerialNumberStr);
>      ASSERT (NewStr != NULL);
> --
> 2.18.0.windows.1