[edk2-devel] [PATCH v7 00/12] Enable New CodeQL Queries

Michael Kubacki posted 12 patches 1 year ago
Failed in applying to current master (apply log)
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                               |  5 +-
30 files changed, 286 insertions(+), 183 deletions(-)
[edk2-devel] [PATCH v7 00/12] Enable New CodeQL Queries
Posted by Michael Kubacki 1 year ago
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

v7 series changes:

1. Added R-b tag to UefiCpuPkg patch
2. Merged Rebecca's patch https://edk2.groups.io/g/devel/message/101819
   into [PATCH v7 02/12]

v6 series changes:

1. Also change "1u" to "1" in:
   - UefiCpuPkg/CpuMpPei/CpuPaging.c
   - UefiCpuPkg/CpuMpPei/CpuMpPei.c

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                               |  5 +-
 30 files changed, 286 insertions(+), 183 deletions(-)

-- 
2.40.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101844): https://edk2.groups.io/g/devel/message/101844
Mute This Topic: https://groups.io/mt/97834563/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 00/12] Enable New CodeQL Queries
Posted by Oliver Smith-Denny 1 year ago
With comments and for the patchset:

Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>

Thanks!

On 3/24/2023 3:30 PM, Michael Kubacki wrote:
> 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
> 
> v7 series changes:
> 
> 1. Added R-b tag to UefiCpuPkg patch
> 2. Merged Rebecca's patch https://edk2.groups.io/g/devel/message/101819
>     into [PATCH v7 02/12]
> 
> v6 series changes:
> 
> 1. Also change "1u" to "1" in:
>     - UefiCpuPkg/CpuMpPei/CpuPaging.c
>     - UefiCpuPkg/CpuMpPei/CpuMpPei.c
> 
> 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                               |  5 +-
>   30 files changed, 286 insertions(+), 183 deletions(-)
> 


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