[edk2-devel] [PATCH v1 07/12] NetworkPkg: Fix conditionally uninitialized variables

Michael Kubacki posted 12 patches 3 years, 1 month ago
There is a newer version of this series
[edk2-devel] [PATCH v1 07/12] NetworkPkg: Fix conditionally uninitialized variables
Posted by Michael Kubacki 3 years, 1 month ago
From: Michael Kubacki <michael.kubacki@microsoft.com>

Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c | 2 +-
 NetworkPkg/TcpDxe/TcpInput.c               | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c b/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c
index 6a5d78629bb3..71c98abc820e 100644
--- a/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -753,7 +753,7 @@ HttpUrlGetPort (
 
   Status =  AsciiStrDecimalToUintnS (Url + Parser->FieldData[HTTP_URI_FIELD_PORT].Offset, (CHAR8 **)NULL, &Data);
 
-  if (Data > HTTP_URI_PORT_MAX_NUM) {
+  if (!EFI_ERROR (Status) && (Data > HTTP_URI_PORT_MAX_NUM)) {
     Status = EFI_INVALID_PARAMETER;
     goto ON_EXIT;
   }
diff --git a/NetworkPkg/TcpDxe/TcpInput.c b/NetworkPkg/TcpDxe/TcpInput.c
index fb1aa827f8ba..7b329be64dfe 100644
--- a/NetworkPkg/TcpDxe/TcpInput.c
+++ b/NetworkPkg/TcpDxe/TcpInput.c
@@ -1570,6 +1570,9 @@ TcpIcmpInput (
   BOOLEAN     IcmpErrIsHard;
   BOOLEAN     IcmpErrNotify;
 
+  IcmpErrIsHard = FALSE;
+  IcmpErrNotify = FALSE;
+
   if (Nbuf->TotalSize < sizeof (TCP_HEAD)) {
     goto CLEAN_EXIT;
   }
-- 
2.28.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96153): https://edk2.groups.io/g/devel/message/96153
Mute This Topic: https://groups.io/mt/94918098/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 07/12] NetworkPkg: Fix conditionally uninitialized variables
Posted by Michael D Kinney 3 years ago
Hi Michael,

Comment below.

Mike

> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Wednesday, November 9, 2022 9:33 AM
> To: devel@edk2.groups.io
> Cc: Erich McMillan <emcmillan@microsoft.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Maciej Rabeda
> <maciej.rabeda@linux.intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Michael Kubacki
> <mikuback@linux.microsoft.com>; Siyuan Fu <siyuan.fu@intel.com>
> Subject: [PATCH v1 07/12] NetworkPkg: Fix conditionally uninitialized variables
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Fixes CodeQL alerts for CWE-457:
> https://cwe.mitre.org/data/definitions/457.html
> 
> Cc: Erich McMillan <emcmillan@microsoft.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c | 2 +-
>  NetworkPkg/TcpDxe/TcpInput.c               | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c b/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c
> index 6a5d78629bb3..71c98abc820e 100644
> --- a/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c
> +++ b/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c
> @@ -753,7 +753,7 @@ HttpUrlGetPort (
> 
>    Status =  AsciiStrDecimalToUintnS (Url + Parser->FieldData[HTTP_URI_FIELD_PORT].Offset, (CHAR8 **)NULL, &Data);
> 
> -  if (Data > HTTP_URI_PORT_MAX_NUM) {
> +  if (!EFI_ERROR (Status) && (Data > HTTP_URI_PORT_MAX_NUM)) {

I do not think this logic change is correct.  If the string can not be
converted to  a value, then Status will be an error.  If that happens,
then the value of Data is undefined.  An error should be returned if
Status is an error or Data is out of range.

if (EFI_ERROR (Status) || (Data > HTTP_URI_PORT_MAX_NUM)) {

>      Status = EFI_INVALID_PARAMETER;
>      goto ON_EXIT;
>    }
> diff --git a/NetworkPkg/TcpDxe/TcpInput.c b/NetworkPkg/TcpDxe/TcpInput.c
> index fb1aa827f8ba..7b329be64dfe 100644
> --- a/NetworkPkg/TcpDxe/TcpInput.c
> +++ b/NetworkPkg/TcpDxe/TcpInput.c
> @@ -1570,6 +1570,9 @@ TcpIcmpInput (
>    BOOLEAN     IcmpErrIsHard;
>    BOOLEAN     IcmpErrNotify;
> 
> +  IcmpErrIsHard = FALSE;
> +  IcmpErrNotify = FALSE;
> +
>    if (Nbuf->TotalSize < sizeof (TCP_HEAD)) {
>      goto CLEAN_EXIT;
>    }
> --
> 2.28.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96600): https://edk2.groups.io/g/devel/message/96600
Mute This Topic: https://groups.io/mt/94918098/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-