[PATCH v5 00/12] Enable New CodeQL Queries

[PATCH v5 00/12] Enable New CodeQL Queries

[Michael Kubacki]

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

Adds queries for the following:

1. cpp/conditionallyuninitializedvariable
2. cpp/pointer-overflow-check
3. cpp/overrunning-write
4. cpp/overrunning-write-with-float
5. cpp/very-likely-overrunning-write

These check for vulnerabilities with the following CWEs:

  - https://cwe.mitre.org/data/definitions/120.html
  - https://cwe.mitre.org/data/definitions/457.html
  - https://cwe.mitre.org/data/definitions/676.html
  - https://cwe.mitre.org/data/definitions/758.html
  - https://cwe.mitre.org/data/definitions/787.html
  - https://cwe.mitre.org/data/definitions/805.html

The first part of this patch series contains fixes for CodeQL alerts
across various packages that are produced by the new queries being
enabled.

The second part updates the CodeQL queries.

Note: The changes are currently in the following pull request
https://github.com/tianocore/edk2/pull/4133

The UefiCpuPkg patch still needs a R-b.

v5 series changes:

1. Changed "1u" to "1" in UefiCpuPkg/CpuMpPei/CpuBist.c
2. Added Rb tags from v4 series

v4 series changes:

1. Simplify conditional logic in Patch 1 per Michael Brown's
   suggestion.

v3 series changes:

1. Rebased series onto 93a21b4 (current edk2/master)

2. Added v2 Rb tags

V2 series changes:

1. MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
   - Applied SafeUintnAdd() to both variables in the comparison
     in ParseAndAddExistingSmbiosTable()

    Addresses feedback from: Mike Kinney

2. CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
   - Changes:

     if (!(Inf & 0x80) && (Asn1Tag != V_ASN1_SEQUENCE)) {

     To:

     if (((Inf & 0x80) == 0x00) && (Asn1Tag != V_ASN1_SEQUENCE)) {

    Addresses feedback from: Mike Kinney

3. MdePkg/Library/BaseLib/String.c
   - Removes: #include <Uefi/UefiBaseType.h>
   - Changes conditional style in changes to if statement from
     ternary for changes made throughout the file
   - Updates commit message to describe change in return value

   Addresses feedback from: Mike Kinney

4. NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c
   - Changes:

     if (!EFI_ERROR (Status) && (Data > HTTP_URI_PORT_MAX_NUM)) {
       Status = EFI_INVALID_PARAMETER;
       goto ON_EXIT;
     }

     To:

     if (EFI_ERROR (Status) || (Data > HTTP_URI_PORT_MAX_NUM)) {
       Status = EFI_INVALID_PARAMETER;
       goto ON_EXIT;
     }

   Addresses feedback from: Mike Kinney

5. ShellPkg/Application/Shell/Shell.c
   - Initializes CalleeStatus to EFI_SUCCESS in DoStartupScript()
   - Restores original if statement logic in DoStartupScript()

   Addresses feedback from: Zhichao Gao

6. ShellPkg/Application/Shell/ShellProtocol.c
   - Adds additional check for return value from
     PARSE_HANDLE_DATABASE_UEFI_DRIVERS() in EfiShellGetDeviceName()

   Addresses feedback from: Zhichao Gao

7. Includes up-to-date R-b tags

---

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Michael Brown <mcb30@ipxe.org>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Erich McMillan (1):
  MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

Michael Kubacki (11):
  BaseTools/PatchCheck.py: Add PCCTS to tab exemption list
  BaseTools/VfrCompile: Fix potential buffer overwrites
  CryptoPkg: Fix conditionally uninitialized variable
  MdeModulePkg: Fix conditionally uninitialized variables
  MdePkg: Fix conditionally uninitialized variables
  NetworkPkg: Fix conditionally uninitialized variables
  PcAtChipsetPkg: Fix conditionally uninitialized variables
  ShellPkg: Fix conditionally uninitialized variables
  UefiCpuPkg: Fix conditionally uninitialized variables
  .github/codeql/edk2.qls: Enable CWE 457, 676, and 758 queries
  .github/codeql/edk2.qls: Enable CWE 120, 787, and 805 queries

 BaseTools/Source/C/VfrCompile/Pccts/antlr/gen.c               | 10 ++--
 BaseTools/Source/C/VfrCompile/Pccts/antlr/main.c              |  4 +-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c                 | 21 ++++---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c                        |  5 +-
 MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c                           | 24 +++++---
 MdeModulePkg/Core/Dxe/Mem/Page.c                              | 17 +++---
 MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c | 25 ++++----
 MdeModulePkg/Library/FileExplorerLib/FileExplorer.c           |  5 +-
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c                      | 33 ++++++-----
 MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c      | 11 ++--
 MdeModulePkg/Universal/HiiDatabaseDxe/Font.c                  | 14 +++--
 MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c                  |  8 +--
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c         |  2 +-
 MdePkg/Library/BaseLib/String.c                               | 40 ++++++++++---
 NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c                    |  2 +-
 NetworkPkg/TcpDxe/TcpInput.c                                  |  3 +
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c            |  9 ++-
 ShellPkg/Application/Shell/Shell.c                            |  1 +
 ShellPkg/Application/Shell/ShellProtocol.c                    | 60 ++++++++++----------
 ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c    | 56 +++++++++---------
 ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c            | 18 +++---
 ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c   |  9 ++-
 ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c        | 14 +++--
 ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c     | 17 ++++--
 ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c        | 21 +++----
 UefiCpuPkg/CpuMpPei/CpuBist.c                                 |  8 ++-
 UefiCpuPkg/CpuMpPei/CpuMpPei.c                                |  8 ++-
 UefiCpuPkg/CpuMpPei/CpuPaging.c                               |  9 ++-
 .github/codeql/edk2.qls                                       | 10 ++++
 BaseTools/Scripts/PatchCheck.py                               |  4 +-
 30 files changed, 285 insertions(+), 183 deletions(-)

-- 
2.40.0.windows.1

[PATCH v5 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

[Michael Kubacki]

From: Erich McMillan <emcmillan@microsoft.com>

Details for these CodeQL alerts can be found here:

- Pointer overflow check (cpp/pointer-overflow-check):
  - https://cwe.mitre.org/data/definitions/758.html

- Potential buffer overflow check (cpp/potential-buffer-overflow):
  - https://cwe.mitre.org/data/definitions/676.html

CodeQL alert:

  - Line 1612 in MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
    - Type: Pointer overflow check
    - Severity: Low
    - Problem: Range check relying on pointer overflow

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Erich McMillan <emcmillan@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
---
 MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
index 1d43adc7662c..dd077bb0cf19 100644
--- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
+++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
@@ -1608,9 +1608,7 @@ ParseAndAddExistingSmbiosTable (
     //
     // Make sure not to access memory beyond SmbiosEnd
     //
-    if ((Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw) ||
-        (Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw))
-    {
+    if ((UINTN)(SmbiosEnd.Raw - Smbios.Raw) < sizeof (SMBIOS_STRUCTURE)) {
       return EFI_INVALID_PARAMETER;
     }
 
@@ -1625,9 +1623,7 @@ ParseAndAddExistingSmbiosTable (
     // Make sure not to access memory beyond SmbiosEnd
     // Each structure shall be terminated by a double-null (0000h).
     //
-    if ((Smbios.Raw + Smbios.Hdr->Length + 2 * sizeof (UINT8) > SmbiosEnd.Raw) ||
-        (Smbios.Raw + Smbios.Hdr->Length + 2 * sizeof (UINT8) < Smbios.Raw))
-    {
+    if ((UINTN)(SmbiosEnd.Raw - Smbios.Raw) < (Smbios.Hdr->Length + 2U)) {
       return EFI_INVALID_PARAMETER;
     }
 
-- 
2.40.0.windows.1

[PATCH v5 02/12] BaseTools/PatchCheck.py: Add PCCTS to tab exemption list

[Michael Kubacki]

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

Purdue Compiler Construction Tool Set (PCCTS) source code was copied/
pasted into BaseTools/Source/C/VfrCompile/Pccts/.

The code contains tab characters instead of spaces.

PatchCheck.py gives an error on modifications to files that
contain tabs.

This change adds that directory to the pre-existing list of
directories in which tab checks are ignored in PatchCheck.py.

The goal of my upcoming change there is not to mix tabs and spaces
but to fix a bug while preserving its current formatting characters.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 BaseTools/Scripts/PatchCheck.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index fcdabfc8acea..84d56b8c8b70 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -383,7 +383,9 @@ class GitDiffCheck:
                     self.force_crlf = False
                     self.force_notabs = False
                 if os.path.basename(self.filename) == 'GNUmakefile' or \
-                   os.path.basename(self.filename) == 'Makefile':
+                   os.path.basename(self.filename) == 'Makefile' or \
+                   self.filename.startswith(
+                        'BaseTools/Source/C/VfrCompile/Pccts/'):
                     self.force_notabs = False
             elif len(line.rstrip()) != 0:
                 self.format_error("didn't find diff command")
-- 
2.40.0.windows.1

[PATCH v5 03/12] BaseTools/VfrCompile: Fix potential buffer overwrites

[Michael Kubacki]

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

While more portable methods exist to handle these cases, this change
does not attempt to do more than fix the immediate problem and
follow the conventions already established in this code.

`snprintf()` is introduced as the minimum improvement apart from
making the buffers larger.

Fixes the following CodeQL alerts:

1. Failure on line 2339 in
   BaseTools/Source/C/VfrCompile/Pccts/antlr/gen.c

   - Type: Potentially overrunning write
   - Severity: Critical
   - Problem: This 'call to sprintf' operation requires 17 bytes but
     the destination is only 16 bytes.

2. Failure on line 2341 in
   BaseTools/Source/C/VfrCompile/Pccts/antlr/gen.c

   - Type: Potentially overrunning write
   - Severity: Critical
   - Problem: This 'call to sprintf' operation requires 17 bytes but
     the destination is only 16 bytes.

3. Failure on line 1309 in
   BaseTools/Source/C/VfrCompile/Pccts/antlr/main.c

   - Type: Potentially overrunning write
   - Severity: Critical
   - Problem: This 'call to sprintf' operation requires 25 bytes but
     the destination is only 20 bytes.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 BaseTools/Source/C/VfrCompile/Pccts/antlr/gen.c  | 10 +++++-----
 BaseTools/Source/C/VfrCompile/Pccts/antlr/main.c |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/Pccts/antlr/gen.c b/BaseTools/Source/C/VfrCompile/Pccts/antlr/gen.c
index 8e41239f4751..33d9cac4c7de 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/antlr/gen.c
+++ b/BaseTools/Source/C/VfrCompile/Pccts/antlr/gen.c
@@ -2331,14 +2331,14 @@ TokNode *p;
 			set_nameErrSet = bufErrSet;								/* MR23 */
 		}
 		else {					/* wild card */
-			static char buf[sizeof("zzerr")+10];
-			static char bufErrSet[sizeof("zzerr")+10];
+			static char buf[sizeof("zzerr")+11];
+			static char bufErrSet[sizeof("zzerr")+11];
 			int n = DefErrSet( &b, 0, NULL );
 			int nErrSet = DefErrSetWithSuffix(0, &bErrSet, 1, NULL, "_set");
-			if ( GenCC ) sprintf(buf, "err%d", n);
-			else sprintf(buf, "zzerr%d", n);
+			if ( GenCC ) snprintf(buf, 11, "err%d", n);
+			else snprintf(buf, 11, "zzerr%d", n);
 			if ( GenCC ) sprintf(bufErrSet, "err%d", nErrSet);
-			else sprintf(bufErrSet, "zzerr%d", nErrSet);
+			else snprintf(bufErrSet, 11, "zzerr%d", nErrSet);
 			set_name = buf;
 			set_nameErrSet = bufErrSet;
 		}
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/antlr/main.c b/BaseTools/Source/C/VfrCompile/Pccts/antlr/main.c
index 051ee4ec5d28..488b4b90461c 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/antlr/main.c
+++ b/BaseTools/Source/C/VfrCompile/Pccts/antlr/main.c
@@ -1295,7 +1295,7 @@ int token;
 #endif
 {
 	int     j;
-        static    char    imag_name[20];
+        static    char    imag_name[25];
 
 	/* look in all lexclasses for the token */
 	if ( TokenString(token) != NULL ) return TokenString(token);
@@ -1306,7 +1306,7 @@ int token;
 	}
 
     if (1) {
-      sprintf(imag_name,"UnknownToken#%d",token);           /* MR13 */
+      snprintf(imag_name, 25, "UnknownToken#%d", token);    /* MR13 */
       return imag_name;                                     /* MR13 */
     }
 
-- 
2.40.0.windows.1

[PATCH v5 04/12] CryptoPkg: Fix conditionally uninitialized variable

[Michael Kubacki]

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

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

Checks the return value from `ASN1_get_object()` to verify values
set by the function are valid.

Note that the function returns literal `0x80`:
    `return (0x80);`

That is used to check the return value is as the case in other areas
of the code.

Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
---
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 21 +++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 2333157e0d17..1182323b63ee 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -807,6 +807,7 @@ X509GetTBSCert (
   UINT32       Asn1Tag;
   UINT32       ObjClass;
   UINTN        Length;
+  UINTN        Inf;
 
   //
   // Check input parameters.
@@ -836,9 +837,9 @@ X509GetTBSCert (
   //
   Temp   = Cert;
   Length = 0;
-  ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int *)&ObjClass, (long)CertSize);
+  Inf    = ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int *)&ObjClass, (long)CertSize);
 
-  if (Asn1Tag != V_ASN1_SEQUENCE) {
+  if (((Inf & 0x80) == 0x00) && (Asn1Tag != V_ASN1_SEQUENCE)) {
     return FALSE;
   }
 
@@ -848,7 +849,7 @@ X509GetTBSCert (
   //
   // Verify the parsed TBSCertificate is one correct SEQUENCE data.
   //
-  if (Asn1Tag != V_ASN1_SEQUENCE) {
+  if (((Inf & 0x80) == 0x00) && (Asn1Tag != V_ASN1_SEQUENCE)) {
     return FALSE;
   }
 
@@ -1888,18 +1889,20 @@ Asn1GetTag (
   IN     UINT32   Tag
   )
 {
-  UINT8  *PtrOld;
-  INT32  ObjTag;
-  INT32  ObjCls;
-  long   ObjLength;
+  UINT8   *PtrOld;
+  INT32   ObjTag;
+  INT32   ObjCls;
+  long    ObjLength;
+  UINT32  Inf;
 
   //
   // Save Ptr position
   //
   PtrOld = *Ptr;
 
-  ASN1_get_object ((CONST UINT8 **)Ptr, &ObjLength, &ObjTag, &ObjCls, (INT32)(End - (*Ptr)));
-  if ((ObjTag == (INT32)(Tag & CRYPTO_ASN1_TAG_VALUE_MASK)) &&
+  Inf = ASN1_get_object ((CONST UINT8 **)Ptr, &ObjLength, &ObjTag, &ObjCls, (INT32)(End - (*Ptr)));
+  if (((Inf & 0x80) == 0x00) &&
+      (ObjTag == (INT32)(Tag & CRYPTO_ASN1_TAG_VALUE_MASK)) &&
       (ObjCls == (INT32)(Tag & CRYPTO_ASN1_TAG_CLASS_MASK)))
   {
     *Length = (UINTN)ObjLength;
-- 
2.40.0.windows.1

[PATCH v5 05/12] MdeModulePkg: Fix conditionally uninitialized variables

[Michael Kubacki]

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

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

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c                        |  5 +--
 MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c                           | 24 ++++++++------
 MdeModulePkg/Core/Dxe/Mem/Page.c                              | 17 +++++-----
 MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c | 25 +++++++++------
 MdeModulePkg/Library/FileExplorerLib/FileExplorer.c           |  5 ++-
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c                      | 33 +++++++++++---------
 MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c      | 11 ++++---
 MdeModulePkg/Universal/HiiDatabaseDxe/Font.c                  | 14 ++++++---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c         |  2 +-
 9 files changed, 80 insertions(+), 56 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index 843815d0cb18..14bed5472958 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1407,6 +1407,7 @@ SupportPaletteSnoopAttributes (
   IN EFI_PCI_IO_PROTOCOL_ATTRIBUTE_OPERATION  Operation
   )
 {
+  EFI_STATUS     Status;
   PCI_IO_DEVICE  *Temp;
   UINT16         VGACommand;
 
@@ -1444,13 +1445,13 @@ SupportPaletteSnoopAttributes (
   // Check if they are on the same bus
   //
   if (Temp->Parent == PciIoDevice->Parent) {
-    PCI_READ_COMMAND_REGISTER (Temp, &VGACommand);
+    Status = PCI_READ_COMMAND_REGISTER (Temp, &VGACommand);
 
     //
     // If they are on the same bus, either one can
     // be set to snoop, the other set to decode
     //
-    if ((VGACommand & EFI_PCI_COMMAND_VGA_PALETTE_SNOOP) != 0) {
+    if (!EFI_ERROR (Status) && ((VGACommand & EFI_PCI_COMMAND_VGA_PALETTE_SNOOP) != 0)) {
       //
       // VGA has set to snoop, so GFX can be only set to disable snoop
       //
diff --git a/MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c b/MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c
index 48741085e507..496ffbd5c4cc 100644
--- a/MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c
+++ b/MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c
@@ -730,10 +730,12 @@ Uhci2ControlTransfer (
 
   Uhc->PciIo->Flush (Uhc->PciIo);
 
-  *TransferResult = QhResult.Result;
+  if (!EFI_ERROR (Status)) {
+    *TransferResult = QhResult.Result;
 
-  if (DataLength != NULL) {
-    *DataLength = QhResult.Complete;
+    if (DataLength != NULL) {
+      *DataLength = QhResult.Complete;
+    }
   }
 
   UhciDestoryTds (Uhc, TDs);
@@ -884,9 +886,11 @@ Uhci2BulkTransfer (
 
   Uhc->PciIo->Flush (Uhc->PciIo);
 
-  *TransferResult = QhResult.Result;
-  *DataToggle     = QhResult.NextToggle;
-  *DataLength     = QhResult.Complete;
+  if (!EFI_ERROR (Status)) {
+    *TransferResult = QhResult.Result;
+    *DataToggle     = QhResult.NextToggle;
+    *DataLength     = QhResult.Complete;
+  }
 
   UhciDestoryTds (Uhc, TDs);
   Uhc->PciIo->Unmap (Uhc->PciIo, DataMap);
@@ -1210,9 +1214,11 @@ Uhci2SyncInterruptTransfer (
   UhciUnlinkTdFromQh (Uhc->SyncIntQh, TDs);
   Uhc->PciIo->Flush (Uhc->PciIo);
 
-  *TransferResult = QhResult.Result;
-  *DataToggle     = QhResult.NextToggle;
-  *DataLength     = QhResult.Complete;
+  if (!EFI_ERROR (Status)) {
+    *TransferResult = QhResult.Result;
+    *DataToggle     = QhResult.NextToggle;
+    *DataLength     = QhResult.Complete;
+  }
 
   UhciDestoryTds (Uhc, TDs);
   Uhc->PciIo->Unmap (Uhc->PciIo, DataMap);
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 5903ce7ab525..41af50b3d5ab 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -449,14 +449,15 @@ PromoteMemoryResource (
     //
     Promoted = PromoteGuardedFreePages (&StartAddress, &EndAddress);
     if (Promoted) {
-      CoreGetMemorySpaceDescriptor (StartAddress, &Descriptor);
-      CoreAddRange (
-        EfiConventionalMemory,
-        StartAddress,
-        EndAddress,
-        Descriptor.Capabilities & ~(EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED |
-                                    EFI_MEMORY_TESTED | EFI_MEMORY_RUNTIME)
-        );
+      if (!EFI_ERROR (CoreGetMemorySpaceDescriptor (StartAddress, &Descriptor))) {
+        CoreAddRange (
+          EfiConventionalMemory,
+          StartAddress,
+          EndAddress,
+          Descriptor.Capabilities & ~(EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED |
+                                      EFI_MEMORY_TESTED | EFI_MEMORY_RUNTIME)
+          );
+      }
     }
   }
 
diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c
index cdaa2db15365..e22aaf3039f1 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c
@@ -909,23 +909,28 @@ BootFromFile (
   IN EFI_DEVICE_PATH_PROTOCOL  *FilePath
   )
 {
+  EFI_STATUS                    Status;
   EFI_BOOT_MANAGER_LOAD_OPTION  BootOption;
   CHAR16                        *FileName;
 
+  Status   = EFI_NOT_STARTED;
   FileName = NULL;
 
   FileName = ExtractFileNameFromDevicePath (FilePath);
   if (FileName != NULL) {
-    EfiBootManagerInitializeLoadOption (
-      &BootOption,
-      0,
-      LoadOptionTypeBoot,
-      LOAD_OPTION_ACTIVE,
-      FileName,
-      FilePath,
-      NULL,
-      0
-      );
+    Status = EfiBootManagerInitializeLoadOption (
+               &BootOption,
+               0,
+               LoadOptionTypeBoot,
+               LOAD_OPTION_ACTIVE,
+               FileName,
+               FilePath,
+               NULL,
+               0
+               );
+  }
+
+  if (!EFI_ERROR (Status)) {
     //
     // Since current no boot from removable media directly is allowed */
     //
diff --git a/MdeModulePkg/Library/FileExplorerLib/FileExplorer.c b/MdeModulePkg/Library/FileExplorerLib/FileExplorer.c
index ef949267fcc2..804a03d868f2 100644
--- a/MdeModulePkg/Library/FileExplorerLib/FileExplorer.c
+++ b/MdeModulePkg/Library/FileExplorerLib/FileExplorer.c
@@ -1075,7 +1075,10 @@ LibCreateNewFile (
   NewHandle    = NULL;
   FullFileName = NULL;
 
-  LibGetFileHandleFromDevicePath (gFileExplorerPrivate.RetDevicePath, &FileHandle, &ParentName, &DeviceHandle);
+  if (EFI_ERROR (LibGetFileHandleFromDevicePath (gFileExplorerPrivate.RetDevicePath, &FileHandle, &ParentName, &DeviceHandle))) {
+    return EFI_DEVICE_ERROR;
+  }
+
   FullFileName = LibAppendFileName (ParentName, FileName);
   if (FullFileName == NULL) {
     return EFI_OUT_OF_RESOURCES;
diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 766dde3aaeeb..72de8d3211b7 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -691,6 +691,7 @@ BdsEntry (
   EFI_DEVICE_PATH_PROTOCOL        *FilePath;
   EFI_STATUS                      BootManagerMenuStatus;
   EFI_BOOT_MANAGER_LOAD_OPTION    PlatformDefaultBootOption;
+  BOOLEAN                         PlatformDefaultBootOptionValid;
 
   HotkeyTriggered = NULL;
   Status          = EFI_SUCCESS;
@@ -809,24 +810,24 @@ BdsEntry (
     CpuDeadLoop ();
   }
 
-  Status = EfiBootManagerInitializeLoadOption (
-             &PlatformDefaultBootOption,
-             LoadOptionNumberUnassigned,
-             LoadOptionTypePlatformRecovery,
-             LOAD_OPTION_ACTIVE,
-             L"Default PlatformRecovery",
-             FilePath,
-             NULL,
-             0
-             );
-  ASSERT_EFI_ERROR (Status);
+  PlatformDefaultBootOptionValid = EfiBootManagerInitializeLoadOption (
+                                     &PlatformDefaultBootOption,
+                                     LoadOptionNumberUnassigned,
+                                     LoadOptionTypePlatformRecovery,
+                                     LOAD_OPTION_ACTIVE,
+                                     L"Default PlatformRecovery",
+                                     FilePath,
+                                     NULL,
+                                     0
+                                     ) == EFI_SUCCESS;
+  ASSERT (PlatformDefaultBootOptionValid == TRUE);
 
   //
   // System firmware must include a PlatformRecovery#### variable specifying
   // a short-form File Path Media Device Path containing the platform default
   // file path for removable media if the platform supports Platform Recovery.
   //
-  if (PcdGetBool (PcdPlatformRecoverySupport)) {
+  if (PlatformDefaultBootOptionValid && PcdGetBool (PcdPlatformRecoverySupport)) {
     LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypePlatformRecovery);
     if (EfiBootManagerFindLoadOption (&PlatformDefaultBootOption, LoadOptions, LoadOptionCount) == -1) {
       for (Index = 0; Index < LoadOptionCount; Index++) {
@@ -1104,15 +1105,17 @@ BdsEntry (
       LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypePlatformRecovery);
       ProcessLoadOptions (LoadOptions, LoadOptionCount);
       EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
-    } else {
+    } else if (PlatformDefaultBootOptionValid) {
       //
       // When platform recovery is not enabled, still boot to platform default file path.
       //
-      EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption);
+      PlatformDefaultBootOptionValid = EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption) == EFI_SUCCESS;
     }
   }
 
-  EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption);
+  if (PlatformDefaultBootOptionValid) {
+    EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption);
+  }
 
   DEBUG ((DEBUG_ERROR, "[Bds] Unable to boot!\n"));
   PlatformBootManagerUnableToBoot ();
diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
index dca3c1df07ba..0d4cfa4cf06f 100644
--- a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
+++ b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
@@ -944,13 +944,14 @@ PrintMismatchMenuInfo (
   UINTN                          FormsetBufferSize;
 
   Question = MenuOption->ThisTag;
-  HiiGetFormSetFromHiiHandle (gFormData->HiiHandle, &FormsetBuffer, &FormsetBufferSize);
 
-  FormSetTitleStr = GetToken (FormsetBuffer->FormSetTitle, gFormData->HiiHandle);
-  FormTitleStr    = GetToken (gFormData->FormTitle, gFormData->HiiHandle);
+  if (!EFI_ERROR (HiiGetFormSetFromHiiHandle (gFormData->HiiHandle, &FormsetBuffer, &FormsetBufferSize))) {
+    FormSetTitleStr = GetToken (FormsetBuffer->FormSetTitle, gFormData->HiiHandle);
+    FormTitleStr    = GetToken (gFormData->FormTitle, gFormData->HiiHandle);
 
-  DEBUG ((DEBUG_ERROR, "\n[%a]: Mismatch Formset    : Formset Guid = %g,  FormSet title = %s\n", gEfiCallerBaseName, &gFormData->FormSetGuid, FormSetTitleStr));
-  DEBUG ((DEBUG_ERROR, "[%a]: Mismatch Form       : FormId = %d,  Form title = %s.\n", gEfiCallerBaseName, gFormData->FormId, FormTitleStr));
+    DEBUG ((DEBUG_ERROR, "\n[%a]: Mismatch Formset    : Formset Guid = %g,  FormSet title = %s\n", gEfiCallerBaseName, &gFormData->FormSetGuid, FormSetTitleStr));
+    DEBUG ((DEBUG_ERROR, "[%a]: Mismatch Form       : FormId = %d,  Form title = %s.\n", gEfiCallerBaseName, gFormData->FormId, FormTitleStr));
+  }
 
   if (Question->OpCode->OpCode == EFI_IFR_ORDERED_LIST_OP) {
     QuestionName = GetToken (((EFI_IFR_ORDERED_LIST *)MenuOption->ThisTag->OpCode)->Question.Header.Prompt, gFormData->HiiHandle);
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c b/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c
index 399f90feb783..8a0b12f72fbe 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c
@@ -1745,6 +1745,7 @@ HiiStringToImage (
   Attributes = (UINT8 *)AllocateZeroPool (StrLength * sizeof (UINT8));
   ASSERT (Attributes != NULL);
 
+  FontInfo      = NULL;
   RowInfo       = NULL;
   Status        = EFI_SUCCESS;
   StringIn2     = NULL;
@@ -1787,11 +1788,14 @@ HiiStringToImage (
       Background  = ((EFI_FONT_DISPLAY_INFO *)StringInfo)->BackgroundColor;
     } else if (Status == EFI_SUCCESS) {
       FontInfo = &StringInfoOut->FontInfo;
-      IsFontInfoExisted (Private, FontInfo, NULL, NULL, &GlobalFont);
-      Height     = GlobalFont->FontPackage->Height;
-      BaseLine   = GlobalFont->FontPackage->BaseLine;
-      Foreground = StringInfoOut->ForegroundColor;
-      Background = StringInfoOut->BackgroundColor;
+      if (IsFontInfoExisted (Private, FontInfo, NULL, NULL, &GlobalFont)) {
+        Height     = GlobalFont->FontPackage->Height;
+        BaseLine   = GlobalFont->FontPackage->BaseLine;
+        Foreground = StringInfoOut->ForegroundColor;
+        Background = StringInfoOut->BackgroundColor;
+      } else {
+        goto Exit;
+      }
     } else {
       goto Exit;
     }
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 14c176887a55..3eb7d935b4d2 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2453,7 +2453,7 @@ VariableServiceGetVariable (
   AcquireLockOnlyAtBootTime (&mVariableModuleGlobal->VariableGlobal.VariableServicesLock);
 
   Status = FindVariable (VariableName, VendorGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);
-  if ((Variable.CurrPtr == NULL) || EFI_ERROR (Status)) {
+  if (EFI_ERROR (Status) || (Variable.CurrPtr == NULL)) {
     goto Done;
   }
 
-- 
2.40.0.windows.1

[PATCH v5 06/12] MdePkg: Fix conditionally uninitialized variables

[Michael Kubacki]

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

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

Note that this change affects the actual return value from the
following functions. The functions documented that if an integer
overflow occurred, MAX_UINTN would be returned. They were
implemented to actually return an undefined value from the stack.

This change makes the function follow its description. However, this
is technically different than what callers may have previously
expected.

MdePkg/Library/BaseLib/String.c:
  - StrDecimalToUintn()
  - StrDecimalToUint64()
  - StrHexToUintn()
  - StrHexToUint64()
  - AsciiStrDecimalToUintn()
  - AsciiStrDecimalToUint64()
  - AsciiStrHexToUintn()
  - AsciiStrHexToUint64()

Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
---
 MdePkg/Library/BaseLib/String.c | 40 ++++++++++++++++----
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/MdePkg/Library/BaseLib/String.c b/MdePkg/Library/BaseLib/String.c
index 98e6d31463e0..637c96e7b31b 100644
--- a/MdePkg/Library/BaseLib/String.c
+++ b/MdePkg/Library/BaseLib/String.c
@@ -408,7 +408,10 @@ StrDecimalToUintn (
 {
   UINTN  Result;
 
-  StrDecimalToUintnS (String, (CHAR16 **)NULL, &Result);
+  if (RETURN_ERROR (StrDecimalToUintnS (String, (CHAR16 **)NULL, &Result))) {
+    return MAX_UINTN;
+  }
+
   return Result;
 }
 
@@ -454,7 +457,10 @@ StrDecimalToUint64 (
 {
   UINT64  Result;
 
-  StrDecimalToUint64S (String, (CHAR16 **)NULL, &Result);
+  if (RETURN_ERROR (StrDecimalToUint64S (String, (CHAR16 **)NULL, &Result))) {
+    return MAX_UINT64;
+  }
+
   return Result;
 }
 
@@ -501,7 +507,10 @@ StrHexToUintn (
 {
   UINTN  Result;
 
-  StrHexToUintnS (String, (CHAR16 **)NULL, &Result);
+  if (RETURN_ERROR (StrHexToUintnS (String, (CHAR16 **)NULL, &Result))) {
+    return MAX_UINTN;
+  }
+
   return Result;
 }
 
@@ -548,7 +557,10 @@ StrHexToUint64 (
 {
   UINT64  Result;
 
-  StrHexToUint64S (String, (CHAR16 **)NULL, &Result);
+  if (RETURN_ERROR (StrHexToUint64S (String, (CHAR16 **)NULL, &Result))) {
+    return MAX_UINT64;
+  }
+
   return Result;
 }
 
@@ -989,7 +1001,10 @@ AsciiStrDecimalToUintn (
 {
   UINTN  Result;
 
-  AsciiStrDecimalToUintnS (String, (CHAR8 **)NULL, &Result);
+  if (RETURN_ERROR (AsciiStrDecimalToUintnS (String, (CHAR8 **)NULL, &Result))) {
+    return MAX_UINTN;
+  }
+
   return Result;
 }
 
@@ -1031,7 +1046,10 @@ AsciiStrDecimalToUint64 (
 {
   UINT64  Result;
 
-  AsciiStrDecimalToUint64S (String, (CHAR8 **)NULL, &Result);
+  if (RETURN_ERROR (AsciiStrDecimalToUint64S (String, (CHAR8 **)NULL, &Result))) {
+    return MAX_UINT64;
+  }
+
   return Result;
 }
 
@@ -1077,7 +1095,10 @@ AsciiStrHexToUintn (
 {
   UINTN  Result;
 
-  AsciiStrHexToUintnS (String, (CHAR8 **)NULL, &Result);
+  if (RETURN_ERROR (AsciiStrHexToUintnS (String, (CHAR8 **)NULL, &Result))) {
+    return MAX_UINTN;
+  }
+
   return Result;
 }
 
@@ -1123,7 +1144,10 @@ AsciiStrHexToUint64 (
 {
   UINT64  Result;
 
-  AsciiStrHexToUint64S (String, (CHAR8 **)NULL, &Result);
+  if (RETURN_ERROR (AsciiStrHexToUint64S (String, (CHAR8 **)NULL, &Result))) {
+    return MAX_UINT64;
+  }
+
   return Result;
 }
 
-- 
2.40.0.windows.1

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

[Michael Kubacki]

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>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.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..21813463aa4f 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.40.0.windows.1

[PATCH v5 08/12] PcAtChipsetPkg: Fix conditionally uninitialized variables

[Michael Kubacki]

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: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Ray Ni <ray.ni@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
---
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
index 9242a2e82600..57ea3153aa6b 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
@@ -344,7 +344,7 @@ PcRtcInit (
   // so we can use them to get and set wakeup time.
   //
   Status = PcRtcGetWakeupTime (&Enabled, &Pending, &Time, Global);
-  if ((Enabled) || (!EFI_ERROR (Status))) {
+  if ((!EFI_ERROR (Status)) || (Enabled)) {
     return EFI_SUCCESS;
   }
 
@@ -836,8 +836,11 @@ PcRtcSetWakeupTime (
     //
     // Just support set alarm time within 24 hours
     //
-    PcRtcGetTime (&RtcTime, &Capabilities, Global);
-    Status = RtcTimeFieldsValid (&RtcTime);
+    Status = PcRtcGetTime (&RtcTime, &Capabilities, Global);
+    if (!EFI_ERROR (Status)) {
+      Status = RtcTimeFieldsValid (&RtcTime);
+    }
+
     if (EFI_ERROR (Status)) {
       return EFI_DEVICE_ERROR;
     }
-- 
2.40.0.windows.1

[PATCH v5 09/12] ShellPkg: Fix conditionally uninitialized variables

[Michael Kubacki]

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: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
---
 ShellPkg/Application/Shell/Shell.c                          |  1 +
 ShellPkg/Application/Shell/ShellProtocol.c                  | 60 ++++++++++----------
 ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c  | 56 +++++++++---------
 ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c          | 18 +++---
 ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c |  9 ++-
 ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c      | 14 +++--
 ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c   | 17 ++++--
 ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c      | 21 +++----
 8 files changed, 107 insertions(+), 89 deletions(-)

diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Shell/Shell.c
index 0ae6e14a34bf..f95c799bb2a4 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -1300,6 +1300,7 @@ DoStartupScript (
   CHAR16         *FullFileStringPath;
   UINTN          NewSize;
 
+  CalleeStatus    = EFI_SUCCESS;
   Key.UnicodeChar = CHAR_NULL;
   Key.ScanCode    = 0;
 
diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Application/Shell/ShellProtocol.c
index e6d20ab16479..da8c31cb038a 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.c
+++ b/ShellPkg/Application/Shell/ShellProtocol.c
@@ -735,50 +735,52 @@ EfiShellGetDeviceName (
     //
     // Now check the parent controller using this as the child.
     //
-    if (DeviceNameToReturn == NULL) {
-      PARSE_HANDLE_DATABASE_PARENTS (DeviceHandle, &ParentControllerCount, &ParentControllerBuffer);
+    Status = PARSE_HANDLE_DATABASE_PARENTS (DeviceHandle, &ParentControllerCount, &ParentControllerBuffer);
+    if ((DeviceNameToReturn == NULL) && !EFI_ERROR (Status)) {
       for (LoopVar = 0; LoopVar < ParentControllerCount; LoopVar++) {
-        PARSE_HANDLE_DATABASE_UEFI_DRIVERS (ParentControllerBuffer[LoopVar], &ParentDriverCount, &ParentDriverBuffer);
-        for (HandleCount = 0; HandleCount < ParentDriverCount; HandleCount++) {
-          //
-          // try using that driver's component name with controller and our driver as the child.
-          //
-          Status = gBS->OpenProtocol (
-                          ParentDriverBuffer[HandleCount],
-                          &gEfiComponentName2ProtocolGuid,
-                          (VOID **)&CompName2,
-                          gImageHandle,
-                          NULL,
-                          EFI_OPEN_PROTOCOL_GET_PROTOCOL
-                          );
-          if (EFI_ERROR (Status)) {
+        Status = PARSE_HANDLE_DATABASE_UEFI_DRIVERS (ParentControllerBuffer[LoopVar], &ParentDriverCount, &ParentDriverBuffer);
+        if (!EFI_ERROR (Status)) {
+          for (HandleCount = 0; HandleCount < ParentDriverCount; HandleCount++) {
+            //
+            // try using that driver's component name with controller and our driver as the child.
+            //
             Status = gBS->OpenProtocol (
                             ParentDriverBuffer[HandleCount],
-                            &gEfiComponentNameProtocolGuid,
+                            &gEfiComponentName2ProtocolGuid,
                             (VOID **)&CompName2,
                             gImageHandle,
                             NULL,
                             EFI_OPEN_PROTOCOL_GET_PROTOCOL
                             );
-          }
+            if (EFI_ERROR (Status)) {
+              Status = gBS->OpenProtocol (
+                              ParentDriverBuffer[HandleCount],
+                              &gEfiComponentNameProtocolGuid,
+                              (VOID **)&CompName2,
+                              gImageHandle,
+                              NULL,
+                              EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                              );
+            }
+
+            if (EFI_ERROR (Status)) {
+              continue;
+            }
 
-          if (EFI_ERROR (Status)) {
-            continue;
+            Lang   = GetBestLanguageForDriver (CompName2->SupportedLanguages, Language, FALSE);
+            Status = CompName2->GetControllerName (CompName2, ParentControllerBuffer[LoopVar], DeviceHandle, Lang, &DeviceNameToReturn);
+            FreePool (Lang);
+            Lang = NULL;
+            if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) {
+              break;
+            }
           }
 
-          Lang   = GetBestLanguageForDriver (CompName2->SupportedLanguages, Language, FALSE);
-          Status = CompName2->GetControllerName (CompName2, ParentControllerBuffer[LoopVar], DeviceHandle, Lang, &DeviceNameToReturn);
-          FreePool (Lang);
-          Lang = NULL;
+          SHELL_FREE_NON_NULL (ParentDriverBuffer);
           if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) {
             break;
           }
         }
-
-        SHELL_FREE_NON_NULL (ParentDriverBuffer);
-        if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) {
-          break;
-        }
       }
 
       SHELL_FREE_NON_NULL (ParentControllerBuffer);
diff --git a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
index 36cf46fb2c38..4549cbde9b9a 100644
--- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
+++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
@@ -1399,10 +1399,11 @@ ShellCommandCreateInitialMappingsAndPaths (
   CHAR16                    *MapName;
   SHELL_MAP_LIST            *MapListItem;
 
-  SplitCurDir = NULL;
-  MapName     = NULL;
-  MapListItem = NULL;
-  HandleList  = NULL;
+  ConsistMappingTable = NULL;
+  SplitCurDir         = NULL;
+  MapName             = NULL;
+  MapListItem         = NULL;
+  HandleList          = NULL;
 
   //
   // Reset the static members back to zero
@@ -1458,32 +1459,35 @@ ShellCommandCreateInitialMappingsAndPaths (
     //
     PerformQuickSort (DevicePathList, Count, sizeof (EFI_DEVICE_PATH_PROTOCOL *), DevicePathCompare);
 
-    ShellCommandConsistMappingInitialize (&ConsistMappingTable);
-    //
-    // Assign new Mappings to all...
-    //
-    for (Count = 0; HandleList[Count] != NULL; Count++) {
+    if (!EFI_ERROR (ShellCommandConsistMappingInitialize (&ConsistMappingTable))) {
       //
-      // Get default name first
+      // Assign new Mappings to all...
       //
-      NewDefaultName = ShellCommandCreateNewMappingName (MappingTypeFileSystem);
-      ASSERT (NewDefaultName != NULL);
-      Status = ShellCommandAddMapItemAndUpdatePath (NewDefaultName, DevicePathList[Count], 0, TRUE);
-      ASSERT_EFI_ERROR (Status);
-      FreePool (NewDefaultName);
-
-      //
-      // Now do consistent name
-      //
-      NewConsistName = ShellCommandConsistMappingGenMappingName (DevicePathList[Count], ConsistMappingTable);
-      if (NewConsistName != NULL) {
-        Status = ShellCommandAddMapItemAndUpdatePath (NewConsistName, DevicePathList[Count], 0, FALSE);
+      for (Count = 0; HandleList[Count] != NULL; Count++) {
+        //
+        // Get default name first
+        //
+        NewDefaultName = ShellCommandCreateNewMappingName (MappingTypeFileSystem);
+        ASSERT (NewDefaultName != NULL);
+        Status = ShellCommandAddMapItemAndUpdatePath (NewDefaultName, DevicePathList[Count], 0, TRUE);
         ASSERT_EFI_ERROR (Status);
-        FreePool (NewConsistName);
+        FreePool (NewDefaultName);
+
+        //
+        // Now do consistent name
+        //
+        NewConsistName = ShellCommandConsistMappingGenMappingName (DevicePathList[Count], ConsistMappingTable);
+        if (NewConsistName != NULL) {
+          Status = ShellCommandAddMapItemAndUpdatePath (NewConsistName, DevicePathList[Count], 0, FALSE);
+          ASSERT_EFI_ERROR (Status);
+          FreePool (NewConsistName);
+        }
       }
     }
 
-    ShellCommandConsistMappingUnInitialize (ConsistMappingTable);
+    if (ConsistMappingTable != NULL) {
+      ShellCommandConsistMappingUnInitialize (ConsistMappingTable);
+    }
 
     SHELL_FREE_NON_NULL (HandleList);
     SHELL_FREE_NON_NULL (DevicePathList);
@@ -1626,12 +1630,12 @@ ShellCommandUpdateMapping (
     //
     PerformQuickSort (DevicePathList, Count, sizeof (EFI_DEVICE_PATH_PROTOCOL *), DevicePathCompare);
 
-    ShellCommandConsistMappingInitialize (&ConsistMappingTable);
+    Status = ShellCommandConsistMappingInitialize (&ConsistMappingTable);
 
     //
     // Assign new Mappings to remainders
     //
-    for (Count = 0; !EFI_ERROR (Status) && HandleList[Count] != NULL && !EFI_ERROR (Status); Count++) {
+    for (Count = 0; !EFI_ERROR (Status) && HandleList[Count] != NULL; Count++) {
       //
       // Skip ones that already have
       //
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
index 97a4b57a932f..5329b559ba46 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
@@ -158,7 +158,10 @@ ShellCommandRunDblk (
           ShellStatus = SHELL_INVALID_PARAMETER;
         }
 
-        ShellConvertStringToUint64 (LbaString, &Lba, TRUE, FALSE);
+        if (EFI_ERROR (ShellConvertStringToUint64 (LbaString, &Lba, TRUE, FALSE))) {
+          ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"dblk", LbaString);
+          ShellStatus = SHELL_INVALID_PARAMETER;
+        }
       }
 
       if (BlockCountString == NULL) {
@@ -169,12 +172,13 @@ ShellCommandRunDblk (
           ShellStatus = SHELL_INVALID_PARAMETER;
         }
 
-        ShellConvertStringToUint64 (BlockCountString, &BlockCount, TRUE, FALSE);
-        if (BlockCount > 0x10) {
-          BlockCount = 0x10;
-        } else if (BlockCount == 0) {
-          ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"dblk", BlockCountString);
-          ShellStatus = SHELL_INVALID_PARAMETER;
+        if (!EFI_ERROR (ShellConvertStringToUint64 (BlockCountString, &BlockCount, TRUE, FALSE))) {
+          if (BlockCount > 0x10) {
+            BlockCount = 0x10;
+          } else if (BlockCount == 0) {
+            ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"dblk", BlockCountString);
+            ShellStatus = SHELL_INVALID_PARAMETER;
+          }
         }
       }
 
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c
index 8bf23a2076a1..72f8c087cb69 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c
@@ -112,10 +112,13 @@ ShellCommandRunEfiDecompress (
 
         if (ShellStatus == SHELL_SUCCESS) {
           Status = FileHandleGetSize (InFileHandle, &Temp64Bit);
-          ASSERT (Temp64Bit <= (UINT32)(-1));
-          InSize = (UINTN)Temp64Bit;
           ASSERT_EFI_ERROR (Status);
-          InBuffer = AllocateZeroPool (InSize);
+          if (!EFI_ERROR (Status)) {
+            ASSERT (Temp64Bit <= (UINT32)(-1));
+            InSize   = (UINTN)Temp64Bit;
+            InBuffer = AllocateZeroPool (InSize);
+          }
+
           if (InBuffer == NULL) {
             Status = EFI_OUT_OF_RESOURCES;
           } else {
diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
index d7a133c0c5b4..870c5b0d1da7 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
@@ -508,9 +508,10 @@ ShellCommandRunConnect (
       Count  = ShellCommandLineGetCount (Package);
 
       if (Param1 != NULL) {
-        Status  = ShellConvertStringToUint64 (Param1, &Intermediate, TRUE, FALSE);
-        Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
-        if (EFI_ERROR (Status)) {
+        Status = ShellConvertStringToUint64 (Param1, &Intermediate, TRUE, FALSE);
+        if (!EFI_ERROR (Status)) {
+          Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+        } else {
           ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDLE), gShellDriver1HiiHandle, L"connect", Param1);
           ShellStatus = SHELL_INVALID_PARAMETER;
         }
@@ -519,9 +520,10 @@ ShellCommandRunConnect (
       }
 
       if (Param2 != NULL) {
-        Status  = ShellConvertStringToUint64 (Param2, &Intermediate, TRUE, FALSE);
-        Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
-        if (EFI_ERROR (Status)) {
+        Status = ShellConvertStringToUint64 (Param2, &Intermediate, TRUE, FALSE);
+        if (!EFI_ERROR (Status)) {
+          Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+        } else {
           ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDLE), gShellDriver1HiiHandle, L"connect", Param2);
           ShellStatus = SHELL_INVALID_PARAMETER;
         }
diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
index 009ae5282b27..fd49d1f7ceb4 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
@@ -160,12 +160,17 @@ ShellCommandRunDisconnect (
         Param1 = ShellCommandLineGetRawValue (Package, 1);
         Param2 = ShellCommandLineGetRawValue (Package, 2);
         Param3 = ShellCommandLineGetRawValue (Package, 3);
-        ShellConvertStringToUint64 (Param1, &Intermediate1, TRUE, FALSE);
-        Handle1 = Param1 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate1) : NULL;
-        ShellConvertStringToUint64 (Param2, &Intermediate2, TRUE, FALSE);
-        Handle2 = Param2 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate2) : NULL;
-        ShellConvertStringToUint64 (Param3, &Intermediate3, TRUE, FALSE);
-        Handle3 = Param3 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate3) : NULL;
+        if (!EFI_ERROR (ShellConvertStringToUint64 (Param1, &Intermediate1, TRUE, FALSE))) {
+          Handle1 = Param1 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate1) : NULL;
+        }
+
+        if (!EFI_ERROR (ShellConvertStringToUint64 (Param2, &Intermediate2, TRUE, FALSE))) {
+          Handle2 = Param2 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate2) : NULL;
+        }
+
+        if (!EFI_ERROR (ShellConvertStringToUint64 (Param3, &Intermediate3, TRUE, FALSE))) {
+          Handle3 = Param3 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate3) : NULL;
+        }
 
         if ((Param1 != NULL) && (Handle1 == NULL)) {
           ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDLE), gShellDriver1HiiHandle, L"disconnect", Param1);
diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c
index c645c9fd6882..8f70d6b6af39 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c
@@ -438,25 +438,22 @@ ShellCommandRunDrvDiag (
     ControllerHandleStr = ShellCommandLineGetRawValue (Package, 2);
     ChildHandleStr      = ShellCommandLineGetRawValue (Package, 3);
 
-    if (DriverHandleStr == NULL) {
-      Handle1 = NULL;
-    } else {
-      ShellConvertStringToUint64 (DriverHandleStr, &Intermediate, TRUE, FALSE);
+    if ((DriverHandleStr != NULL) && ShellConvertStringToUint64 (DriverHandleStr, &Intermediate, TRUE, FALSE)) {
       Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+    } else {
+      Handle1 = NULL;
     }
 
-    if (ControllerHandleStr == NULL) {
-      Handle2 = NULL;
-    } else {
-      ShellConvertStringToUint64 (ControllerHandleStr, &Intermediate, TRUE, FALSE);
+    if ((ControllerHandleStr != NULL) && ShellConvertStringToUint64 (ControllerHandleStr, &Intermediate, TRUE, FALSE)) {
       Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+    } else {
+      Handle2 = NULL;
     }
 
-    if (ChildHandleStr == NULL) {
-      Handle3 = NULL;
-    } else {
-      ShellConvertStringToUint64 (ChildHandleStr, &Intermediate, TRUE, FALSE);
+    if ((ChildHandleStr != NULL) && ShellConvertStringToUint64 (ChildHandleStr, &Intermediate, TRUE, FALSE)) {
       Handle3 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+    } else {
+      Handle3 = NULL;
     }
 
     Status = DoDiagnostics (
-- 
2.40.0.windows.1

[PATCH v5 10/12] UefiCpuPkg: Fix conditionally uninitialized variables

[Michael Kubacki]

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

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

Cc: Eric Dong <eric.dong@intel.com>
Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 UefiCpuPkg/CpuMpPei/CpuBist.c   | 8 +++++++-
 UefiCpuPkg/CpuMpPei/CpuMpPei.c  | 8 +++++++-
 UefiCpuPkg/CpuMpPei/CpuPaging.c | 9 ++++++++-
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuBist.c b/UefiCpuPkg/CpuMpPei/CpuBist.c
index 7dc93cd784d4..78e008703993 100644
--- a/UefiCpuPkg/CpuMpPei/CpuBist.c
+++ b/UefiCpuPkg/CpuMpPei/CpuBist.c
@@ -175,7 +175,13 @@ CollectBistDataFromPpi (
   EFI_SEC_PLATFORM_INFORMATION_RECORD2  *PlatformInformationRecord2;
   EFI_SEC_PLATFORM_INFORMATION_CPU      *CpuInstanceInHob;
 
-  MpInitLibGetNumberOfProcessors (&NumberOfProcessors, &NumberOfEnabledProcessors);
+  Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, &NumberOfEnabledProcessors);
+  ASSERT_EFI_ERROR (Status);
+
+  if (EFI_ERROR (Status)) {
+    NumberOfProcessors        = 1;
+    NumberOfEnabledProcessors = 1;
+  }
 
   BistInformationSize = sizeof (EFI_SEC_PLATFORM_INFORMATION_RECORD2) +
                         sizeof (EFI_SEC_PLATFORM_INFORMATION_CPU) * NumberOfProcessors;
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
index e7f1fe9f426c..a84304273168 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
@@ -473,7 +473,13 @@ InitializeMpExceptionStackSwitchHandlers (
     return;
   }
 
-  MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
+  Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
+  ASSERT_EFI_ERROR (Status);
+
+  if (EFI_ERROR (Status)) {
+    NumberOfProcessors = 1u;
+  }
+
   SwitchStackData = AllocatePages (EFI_SIZE_TO_PAGES (NumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT)));
   ASSERT (SwitchStackData != NULL);
   ZeroMem (SwitchStackData, NumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT));
diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index 135422225340..1322fcb77f28 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -538,6 +538,7 @@ SetupStackGuardPage (
   UINTN                 NumberOfProcessors;
   UINTN                 Bsp;
   UINTN                 Index;
+  EFI_STATUS            Status;
 
   //
   // One extra page at the bottom of the stack is needed for Guard page.
@@ -547,7 +548,13 @@ SetupStackGuardPage (
     ASSERT (FALSE);
   }
 
-  MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
+  Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
+  ASSERT_EFI_ERROR (Status);
+
+  if (EFI_ERROR (Status)) {
+    NumberOfProcessors = 1u;
+  }
+
   MpInitLibWhoAmI (&Bsp);
   for (Index = 0; Index < NumberOfProcessors; ++Index) {
     StackBase = 0;
-- 
2.40.0.windows.1

[PATCH v5 11/12] .github/codeql/edk2.qls: Enable CWE 457, 676, and 758 queries

[Michael Kubacki]

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

The previous commits fixed issues with these queries across various
packages. Now that those are resolved, enable the queries in the
edk2 query set so regressions can be found in the future.

Enables:

1. cpp/conditionallyuninitializedvariable
   - CWE: https://cwe.mitre.org/data/definitions/457.html
   - @name Conditionally uninitialized variable
   - @description An initialization function is used to initialize a
                  local variable, but the returned status code is
                  not checked. The variable may be left in an
                  uninitialized state, and reading the variable may
                  result in undefined behavior.
   - @kind problem
   - @problem.severity warning
   - @security-severity 7.8
   - @id cpp/conditionally-uninitialized-variable
   - @tags security
     - external/cwe/cwe-457
2. cpp/pointer-overflow-check
   - CWE: https://cwe.mitre.org/data/definitions/758.html
   - @name Pointer overflow check
   - @description Adding a value to a pointer to check if it
                  overflows relies on undefined behavior and
                  may lead to memory corruption.
   - @kind problem
   - @problem.severity error
   - @security-severity 2.1
   - @precision high
   - @id cpp/pointer-overflow-check
   - @tags reliability
     - security
     - external/cwe/cwe-758
3. cpp/potential-buffer-overflow
   - CWE: https://cwe.mitre.org/data/definitions/676.html
   - @name Potential buffer overflow
   - @description Using a library function that does not check
                  buffer bounds requires the surrounding program
                  to be very carefully written to avoid buffer
                  overflows.
   - @kind problem
   - @id cpp/potential-buffer-overflow
   - @problem.severity warning
   - @security-severity 10.0
   - @tags reliability
     - security
     - external/cwe/cwe-676
   - @deprecated This query is deprecated, use
                 Potentially overrunning write
                 (`cpp/overrunning-write`) and
                 Potentially overrunning write with float to string
                 conversion
                 (`cpp/overrunning-write-with-float`) instead.

Note that cpp/potential-buffer-overflow is deprecated. This query
will be updated to the succeeding queries in the next commit. The
query is used in this commit to show that we considered and tested
the query in history.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 .github/codeql/edk2.qls | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/.github/codeql/edk2.qls b/.github/codeql/edk2.qls
index ef9aae790f5f..dc2d87764e93 100644
--- a/.github/codeql/edk2.qls
+++ b/.github/codeql/edk2.qls
@@ -8,7 +8,14 @@
 
 # Enable individual queries below.
 
+- include:
+    id: cpp/conditionallyuninitializedvariable
 - include:
     id: cpp/infinite-loop-with-unsatisfiable-exit-condition
 - include:
     id: cpp/overflow-buffer
+- include:
+    id: cpp/pointer-overflow-check
+- include:
+    id: cpp/potential-buffer-overflow
+
-- 
2.40.0.windows.1

[PATCH v5 12/12] .github/codeql/edk2.qls: Enable CWE 120, 787, and 805 queries

[Michael Kubacki]

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

As recommended by CodeQL this change replaces
cpp/potential-buffer-overflow with cpp/overrunning-write-with-float
and cpp/overrunning-write.

Enables:

1. cpp/overrunning-write
   - @name Likely overrunning write
   - @description Buffer write operations that do not control the length
                  data written may overflow
   - @kind problem
   - @problem.severity error
   - @security-severity 9.3
   - @precision high
   - @id cpp/very-likely-overrunning-write
   - @tags reliability
     - security
     - external/cwe/cwe-120
     - external/cwe/cwe-787
     - external/cwe/cwe-805
2. cpp/overrunning-write-with-float
   - @name Potentially overrunning write with float to string conversion
   - @description Buffer write operations that do not control the length
                  of data written may overflow when floating point inputs
                  take extreme values.
   - @kind problem
   - @problem.severity error
   - @security-severity 9.3
   - @precision medium
   - @id cpp/overrunning-write-with-float
   - @tags reliability
     - security
     - external/cwe/cwe-120
     - external/cwe/cwe-787
     - external/cwe/cwe-805
3. cpp/very-likely-overrunning-write
   - @name Likely overrunning write
   - @description Buffer write operations that do not control the length
                  of data written may overflow
   - @kind problem
   - @problem.severity error
   - @security-severity 9.3
   - @precision high
   - @id cpp/very-likely-overrunning-write
   - @tags reliability
     - security
     - external/cwe/cwe-120
     - external/cwe/cwe-787
     - external/cwe/cwe-805

- CWEs:
  - https://cwe.mitre.org/data/definitions/120.html
  - https://cwe.mitre.org/data/definitions/787.html
  - https://cwe.mitre.org/data/definitions/805.html

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 .github/codeql/edk2.qls | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/.github/codeql/edk2.qls b/.github/codeql/edk2.qls
index dc2d87764e93..9bea9ba01f24 100644
--- a/.github/codeql/edk2.qls
+++ b/.github/codeql/edk2.qls
@@ -14,8 +14,11 @@
     id: cpp/infinite-loop-with-unsatisfiable-exit-condition
 - include:
     id: cpp/overflow-buffer
+- include:
+    id: cpp/overrunning-write
+- include:
+    id: cpp/overrunning-write-with-float
 - include:
     id: cpp/pointer-overflow-check
 - include:
-    id: cpp/potential-buffer-overflow
-
+    id: cpp/very-likely-overrunning-write
-- 
2.40.0.windows.1