* [edk2-devel] [PATCH] NetworkPkg:HttpDxe:CoverityIssues
@ 2024-05-10 11:06 Santhosh Kumar V via groups.io
0 siblings, 0 replies; 2+ messages in thread
From: Santhosh Kumar V via groups.io @ 2024-05-10 11:06 UTC (permalink / raw)
To: devel@edk2.groups.io, Santhosh Kumar V
Cc: Sivaraman Nainar, Raj V Akilan, Saloni Kasbekar,
Zachary Clark-williams
Resolved Coverity Issues in Http Dxe
1.HttpResponseWorker(DEADCODE)
The result of pointer arithmetic "HttpHeaders + AsciiStrLen("HTTP/1.1") + 1" is never null.
2.HttpDns4 (DEAD LOOP)
Coverity reports dead loop error since IsDone is always false ,In Some scenario it might not update the to true
3.HttpsSupport.c (NULL_RETURNS)
NetbufAlloc ,NetbufAllocSpace might return null pointer ,so Assigning: "NULL" to "PacketOut" and "DataOut" pointer.
Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
Cc: Zachary Clark-williams <zachary.clark-williams@intel.com>
Signed-off-by: SanthoshKumarV <santhoshkumarv@ami.com>
---
NetworkPkg/HttpDxe/HttpDns.c | 2 +-
NetworkPkg/HttpDxe/HttpImpl.c | 5 +----
NetworkPkg/HttpDxe/HttpsSupport.c | 21 ++++++++++++++++++++-
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/NetworkPkg/HttpDxe/HttpDns.c b/NetworkPkg/HttpDxe/HttpDns.c
index 13cbde0f34..b8ac6fba4b 100644
--- a/NetworkPkg/HttpDxe/HttpDns.c
+++ b/NetworkPkg/HttpDxe/HttpDns.c
@@ -150,7 +150,7 @@ HttpDns4 (
goto Exit;
}
- while (!IsDone) {
+ while (!IsDone && (Dns4->Poll != NULL)) {
Dns4->Poll (Dns4);
}
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 6606c29342..6d05c203b0 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1104,10 +1104,7 @@ HttpResponseWorker (
// Search for Status Code.
//
StatusCodeStr = HttpHeaders + AsciiStrLen (HTTP_VERSION_STR) + 1;
- if (StatusCodeStr == NULL) {
- Status = EFI_NOT_READY;
- goto Error;
- }
+
StatusCode = AsciiStrDecimalToUintn (StatusCodeStr);
diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
index 8d7bffe1e9..e40386a99c 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -732,7 +732,6 @@ TlsConfigureSession (
// the caller. The failure is pushed back to TLS DXE driver if the
// HTTP communication actually requires certificate.
//
- Status = EFI_SUCCESS;
} else {
DEBUG ((DEBUG_ERROR, "TLS Certificate Config Error!\n"));
return Status;
@@ -1250,6 +1249,10 @@ TlsConnectSession (
// Transmit ClientHello
//
PacketOut = NetbufAlloc ((UINT32)BufferOutSize);
+ if (PacketOut == NULL) {
+ FreePool (BufferOut);
+ return EFI_OUT_OF_RESOURCES;
+ }
DataOut = NetbufAllocSpace (PacketOut, (UINT32)BufferOutSize, NET_BUF_TAIL);
if (DataOut == NULL) {
FreePool (BufferOut);
@@ -1336,6 +1339,10 @@ TlsConnectSession (
// Transmit the response packet.
//
PacketOut = NetbufAlloc ((UINT32)BufferOutSize);
+ if (PacketOut == NULL) {
+ FreePool (BufferOut);
+ return EFI_OUT_OF_RESOURCES;
+ }
DataOut = NetbufAllocSpace (PacketOut, (UINT32)BufferOutSize, NET_BUF_TAIL);
if (DataOut == NULL) {
FreePool (BufferOut);
@@ -1493,6 +1500,10 @@ TlsCloseSession (
}
PacketOut = NetbufAlloc ((UINT32)BufferOutSize);
+ if (PacketOut == NULL) {
+ FreePool (BufferOut);
+ return EFI_OUT_OF_RESOURCES;
+ }
DataOut = NetbufAllocSpace (PacketOut, (UINT32)BufferOutSize, NET_BUF_TAIL);
if (DataOut == NULL) {
FreePool (BufferOut);
@@ -1781,6 +1792,10 @@ HttpsReceive (
if (BufferOutSize != 0) {
PacketOut = NetbufAlloc ((UINT32)BufferOutSize);
+ if (PacketOut == NULL) {
+ FreePool (BufferOut);
+ return EFI_OUT_OF_RESOURCES;
+ }
DataOut = NetbufAllocSpace (PacketOut, (UINT32)BufferOutSize, NET_BUF_TAIL);
if (DataOut == NULL) {
FreePool (BufferOut);
@@ -1873,6 +1888,10 @@ HttpsReceive (
if (BufferOutSize != 0) {
PacketOut = NetbufAlloc ((UINT32)BufferOutSize);
+ if (PacketOut == NULL) {
+ FreePool (BufferOut);
+ return EFI_OUT_OF_RESOURCES;
+ }
DataOut = NetbufAllocSpace (PacketOut, (UINT32)BufferOutSize, NET_BUF_TAIL);
if (DataOut == NULL) {
FreePool (BufferOut);
--
2.42.0.windows.2
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118813): https://edk2.groups.io/g/devel/message/118813
Mute This Topic: https://groups.io/mt/106018538/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 2+ messages in thread* [edk2-devel] [PATCH] NetworkPkg:HttpDxe:CoverityIssues
@ 2024-05-10 11:04 Santhosh Kumar V via groups.io
0 siblings, 0 replies; 2+ messages in thread
From: Santhosh Kumar V via groups.io @ 2024-05-10 11:04 UTC (permalink / raw)
To: devel@edk2.groups.io, Santhosh Kumar V; +Cc: Sivaraman Nainar, Raj V Akilan
Resolved Coverity Issues in Http Dxe
1.HttpResponseWorker(DEADCODE)
The result of pointer arithmetic "HttpHeaders + AsciiStrLen("HTTP/1.1") + 1" is never null.
2.HttpDns4 (DEAD LOOP)
Coverity reports dead loop error since IsDone is always false ,In Some scenario it might not update the to true
3.HttpsSupport.c (NULL_RETURNS)
NetbufAlloc ,NetbufAllocSpace might return null pointer ,so Assigning: "NULL" to "PacketOut" and "DataOut" pointer.
---
NetworkPkg/HttpDxe/HttpDns.c | 2 +-
NetworkPkg/HttpDxe/HttpImpl.c | 5 +----
NetworkPkg/HttpDxe/HttpsSupport.c | 21 ++++++++++++++++++++-
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/NetworkPkg/HttpDxe/HttpDns.c b/NetworkPkg/HttpDxe/HttpDns.c
index 13cbde0f34..b8ac6fba4b 100644
--- a/NetworkPkg/HttpDxe/HttpDns.c
+++ b/NetworkPkg/HttpDxe/HttpDns.c
@@ -150,7 +150,7 @@ HttpDns4 (
goto Exit;
}
- while (!IsDone) {
+ while (!IsDone && (Dns4->Poll != NULL)) {
Dns4->Poll (Dns4);
}
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 6606c29342..6d05c203b0 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1104,10 +1104,7 @@ HttpResponseWorker (
// Search for Status Code.
//
StatusCodeStr = HttpHeaders + AsciiStrLen (HTTP_VERSION_STR) + 1;
- if (StatusCodeStr == NULL) {
- Status = EFI_NOT_READY;
- goto Error;
- }
+
StatusCode = AsciiStrDecimalToUintn (StatusCodeStr);
diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
index 8d7bffe1e9..e40386a99c 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -732,7 +732,6 @@ TlsConfigureSession (
// the caller. The failure is pushed back to TLS DXE driver if the
// HTTP communication actually requires certificate.
//
- Status = EFI_SUCCESS;
} else {
DEBUG ((DEBUG_ERROR, "TLS Certificate Config Error!\n"));
return Status;
@@ -1250,6 +1249,10 @@ TlsConnectSession (
// Transmit ClientHello
//
PacketOut = NetbufAlloc ((UINT32)BufferOutSize);
+ if (PacketOut == NULL) {
+ FreePool (BufferOut);
+ return EFI_OUT_OF_RESOURCES;
+ }
DataOut = NetbufAllocSpace (PacketOut, (UINT32)BufferOutSize, NET_BUF_TAIL);
if (DataOut == NULL) {
FreePool (BufferOut);
@@ -1336,6 +1339,10 @@ TlsConnectSession (
// Transmit the response packet.
//
PacketOut = NetbufAlloc ((UINT32)BufferOutSize);
+ if (PacketOut == NULL) {
+ FreePool (BufferOut);
+ return EFI_OUT_OF_RESOURCES;
+ }
DataOut = NetbufAllocSpace (PacketOut, (UINT32)BufferOutSize, NET_BUF_TAIL);
if (DataOut == NULL) {
FreePool (BufferOut);
@@ -1493,6 +1500,10 @@ TlsCloseSession (
}
PacketOut = NetbufAlloc ((UINT32)BufferOutSize);
+ if (PacketOut == NULL) {
+ FreePool (BufferOut);
+ return EFI_OUT_OF_RESOURCES;
+ }
DataOut = NetbufAllocSpace (PacketOut, (UINT32)BufferOutSize, NET_BUF_TAIL);
if (DataOut == NULL) {
FreePool (BufferOut);
@@ -1781,6 +1792,10 @@ HttpsReceive (
if (BufferOutSize != 0) {
PacketOut = NetbufAlloc ((UINT32)BufferOutSize);
+ if (PacketOut == NULL) {
+ FreePool (BufferOut);
+ return EFI_OUT_OF_RESOURCES;
+ }
DataOut = NetbufAllocSpace (PacketOut, (UINT32)BufferOutSize, NET_BUF_TAIL);
if (DataOut == NULL) {
FreePool (BufferOut);
@@ -1873,6 +1888,10 @@ HttpsReceive (
if (BufferOutSize != 0) {
PacketOut = NetbufAlloc ((UINT32)BufferOutSize);
+ if (PacketOut == NULL) {
+ FreePool (BufferOut);
+ return EFI_OUT_OF_RESOURCES;
+ }
DataOut = NetbufAllocSpace (PacketOut, (UINT32)BufferOutSize, NET_BUF_TAIL);
if (DataOut == NULL) {
FreePool (BufferOut);
--
2.42.0.windows.2
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118812): https://edk2.groups.io/g/devel/message/118812
Mute This Topic: https://groups.io/mt/106018538/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-05-10 11:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-10 11:06 [edk2-devel] [PATCH] NetworkPkg:HttpDxe:CoverityIssues Santhosh Kumar V via groups.io
-- strict thread matches above, loose matches on Subject: below --
2024-05-10 11:04 Santhosh Kumar V via groups.io
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox