1. Edk2 | Devel
  2. Topics

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

Michael Kubacki
 

From: Michael Kubacki <michael.kubacki@...>

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

Cc: Erich McMillan <emcmillan@...>
Cc: Jiaxin Wu <jiaxin.wu@...>
Cc: Maciej Rabeda <maciej.rabeda@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Michael Kubacki <mikuback@...>
Cc: Siyuan Fu <siyuan.fu@...>
Co-authored-by: Erich McMillan <emcmillan@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
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/Libr=
ary/DxeHttpLib/DxeHttpLib.c
index 6a5d78629bb3..71c98abc820e 100644
--- a/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -753,7 +753,7 @@ HttpUrlGetPort (
=20
Status =3D AsciiStrDecimalToUintnS (Url + Parser->FieldData[HTTP_URI_=
FIELD_PORT].Offset, (CHAR8 **)NULL, &Data);
=20
- if (Data > HTTP_URI_PORT_MAX_NUM) {
+ if (!EFI_ERROR (Status) && (Data > HTTP_URI_PORT_MAX_NUM)) {
Status =3D 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;
=20
+ IcmpErrIsHard =3D FALSE;
+ IcmpErrNotify =3D FALSE;
+
if (Nbuf->TotalSize < sizeof (TCP_HEAD)) {
goto CLEAN_EXIT;
}
--=20
2.28.0.windows.1
Michael D Kinney
 

Hi Michael,

Comment below.

Mike

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Wednesday, November 9, 2022 9:33 AM
To: devel@edk2.groups.io
Cc: Erich McMillan <emcmillan@...>; Wu, Jiaxin <jiaxin.wu@...>; Maciej Rabeda
<maciej.rabeda@...>; Kinney, Michael D <michael.d.kinney@...>; Michael Kubacki
<mikuback@...>; Siyuan Fu <siyuan.fu@...>
Subject: [PATCH v1 07/12] NetworkPkg: Fix conditionally uninitialized variables

From: Michael Kubacki <michael.kubacki@...>

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

Cc: Erich McMillan <emcmillan@...>
Cc: Jiaxin Wu <jiaxin.wu@...>
Cc: Maciej Rabeda <maciej.rabeda@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Michael Kubacki <mikuback@...>
Cc: Siyuan Fu <siyuan.fu@...>
Co-authored-by: Erich McMillan <emcmillan@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
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

1 - 2 of 2