Re: [edk2-devel] [edk2-wiki][PATCH v1 1/1] Adds EDK II Code Scanning page

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Michael,

Thank you for putting this summary together on CodeQL.  Looks like a great start to this content.

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Wednesday, November 16, 2022 7:57 AM
> To: devel@edk2.groups.io
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki <mikuback@linux.microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [edk2-wiki][PATCH v1 1/1] Adds EDK II Code Scanning page
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> This page includes:
> 
> 1. An explanation of how Code Scanning works in edk2
> 2. An overview of CodeQL
> 3. Links to key CodeQL resources
> 4. Links to key CodeQL files in the edk2 repo
> 5. A CodeQL query target list for edk2 with completion status
> 6. An explanation on how query filtering is used in edk2
> 7. The process for proposing new CodeQL queries for edk2
> 8. Basic CodeQL CLI usage for the edk2 build process
> 
> This is meant to be a starting point to document CodeQL information
> for edk2. It will be updated over time.
> 
> 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>
> ---
> 
> Notes:
>     A rendered version of this markdown document is
>     available on my fork:
> 
>     https://github.com/makubacki/tianocore.github.io/blob/add_edk2_code_scanning_page/EDK-II-Code-Scanning.md
> 
>  EDK-II-Code-Scanning.md | 232 ++++++++++++++++++++
>  1 file changed, 232 insertions(+)
> 
> diff --git a/EDK-II-Code-Scanning.md b/EDK-II-Code-Scanning.md
> new file mode 100644
> index 000000000000..c54c7bef4214
> --- /dev/null
> +++ b/EDK-II-Code-Scanning.md
> @@ -0,0 +1,232 @@
> +# EDK II Code Scanning
> +
> +CodeQL is a code analysis engine developed by Github to automate security checks.
> +
> +It is used for Code Scanning in the TianoCore edk2 repository.
> +
> +## Table of Contents
> +
> +1. [Overview](#overview)
> +2. [CodeQL Usage in edk2](#codeql-usage-in-edk2)
> +   - [Query Target List](#query-target-list)
> +   - [Query Filtering in edk2](#query-filtering-in-edk2)
> +   - [Process for Suggesting New Queries for edk2](#process-for-suggesting-new-queries-for-edk2)
> +   - [Query Enabling Process](#query-enabling-process)
> +   - [CodeQL in Pull Requests](#codeql-in-pull-requests)
> +   - [Dismissing CodeQL Alerts](#dismissing-codeql-alerts)
> +3. [CodeQL CLI Local Commands](#codeql-cli-local-commands)
> +4. [The CodeQL Project](#the-codeql-project)
> +
> +## Overview
> +
> +CodeQL is open source and free for open source projects. It is maintained by GitHub and naturally has excellent
> +integration with GitHub projects. CodeQL uses a semantic code analysis engine to discover vulnerabilities in a
> +number of programming languages (both compiled and interpreted).
> +
> +[General CodeQL Information](https://codeql.github.com/)
> +
> +TianoCore uses CodeQL C/C++ queries to find common programming errors and security vulnerabilities in firmware code.
> +Many open-source queries are officially supported and comprise the vulnerability analysis performed against the
> +database.
> +
> +[CodeQL Query Repository](https://github.com/github/codeql)
> +
> +In addition, anyone can leverage the code analysis engine by writing a custom query. Information around writing a
> +custom query is available in the official documentation.
> +
> +[CodeQL Query Documentation](https://codeql.github.com/docs/writing-codeql-queries/codeql-queries/#codeql-queries).
> +
> +The [edk2](https://github.com/tianocore/edk2) repository uses GitHub's Code Scanning feature (free for public
> +repositories on GitHub.com) to show alerts directly in the repository and run CodeQL on pull requests and pushes
> +to the repository.
> +
> +[About GitHub Code Scanning](https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-
> vulnerabilities-and-errors/about-code-scanning)
> +
> +Current CodeQL scanning results in the edk2 project are available in the "Actions" page of the GitHub repository.
> +
> +[edk2 CodeQL Workflow](https://github.com/tianocore/edk2/actions)
> +
> +A CodeQL command-line interface (CLI) is also available which can be run locally. A CodeQL CLI reference and manual
> +are available in the documentation to learn how to use the CLI.
> +
> +[CodeQL CLI Documenation](https://codeql.github.com/docs/codeql-cli/)
> +
> +At a high-level, there's two main phases of CodeQL execution to be aware of.
> +
> +1. CodeQL database generation
> +   - [CodeQL Database Documentation](https://codeql.github.com/docs/codeql-cli/creating-codeql-databases/)
> +2. CodeQL database analysis
> +   - [CodeQL Analysis Documentation](https://codeql.github.com/docs/codeql-cli/analyzing-databases-with-the-codeql-cli/)
> +
> +The CodeQL CLI hooks into the normal firmware build process to generate a CodeQL database. Once the database is
> +generated, any number of CodeQL queries can be run against the database for analysis.
> +
> +CodeQL analysis results can be stored in the
> +[SARIF](https://sarifweb.azurewebsites.net/) (Static Analysis Results Interchange Format) file format.
> +
> +[CodeQL SARIF documentation](https://codeql.github.com/docs/codeql-cli/sarif-output/)
> +
> +SARIF files are JSON following the SARIF specification/schema. The files can be opened with SARIF viewers to more
> +conveniently view the results in the file.
> +
> +For example, the [SARIF Viewer extension for VS Code](https://marketplace.visualstudio.com/items?itemName=MS-SarifVSCode.sarif-
> viewer)
> +can open a .sarif file generated by the CodeQL CLI and allow you to click links directly to the problematic line in
> +source files.
> +
> +In summary, the edk2 repository runs CodeQL on pull requests and CI builds. Any alerts will be flagged in the pull
> +request status checks area. The queries used by the edk2 repository are stored in the edk2 CodeQL query set file.
> +
> +[edk2 CodeQL Query Set](https://github.com/tianocore/edk2/blob/master/.github/codeql/edk2.qls)
> +
> +## CodeQL Usage in edk2
> +
> +CodeQL provides the capability to debug the actual queries and for our (TianoCore) community to write our own queries
> +and even contribute back to the upstream repo when appropriate. In other cases, we might choose to keep our own
> +queries in a separate TianoCore repo or within a directory in the edk2 code tree.
> +
> +This is all part of CodeQL Scanning. Information on the particular topic of running additional custom queries in
> +Code Scanning is documented [here](https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-
> vulnerabilities-and-errors/configuring-code-scanning#running-additional-queries)
> +in that page.
> +
> +In addition, CodeQL offers the flexibility to:
> +
> +- Build databases locally
> +- Retrieve databases from server builds
> +- Relatively quickly test queries locally against a database for a fast feedback loop
> +- Suppress false positives
> +- Customize the files and queries used in the edk2 project and quickly keep this list in sync between the server and local
> execution
> +
> +### Query Target List
> +
> +While CodeQL can scan various languages including Python and C/C++, the TianoCore project is only focused on C/C++
> +checks at this time. TianoCore has an initial set of queries to evaluate shown below (checked boxes are done).
> +
> +- [x] [cpp/conditionally-uninitialized-variable](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-
> 457/ConditionallyUninitializedVariable.ql)
> +- [x] [cpp/infinite-loop-with-unsatisfiable-exit-
> condition](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-
> 835/InfiniteLoopWithUnsatisfiableExitCondition.ql)
> +- [x] [cpp/overflow-buffer](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-119/OverflowBuffer.ql)
> +- [x] [cpp/pointer-overflow-
> check](https://github.com/github/codeql/blob/main/cpp/ql/src/Likely%20Bugs/Memory%20Management/PointerOverflow.ql)
> +- [x] [cpp/potential-buffer-
> overflow](https://github.com/github/codeql/blob/main/cpp/ql/src/Likely%20Bugs/Memory%20Management/PotentialBufferOverflow.ql)
> +- [ ] [cpp/toctou-race-condition](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-
> 367/TOCTOUFilesystemRace.ql)
> +- [ ] [cpp/unclear-array-index-validation](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-
> 129/ImproperArrayIndexValidation.ql)
> +- [ ] [cpp/unsafe-
> strncat](https://github.com/github/codeql/blob/main/cpp/ql/src/Likely%20Bugs/Memory%20Management/SuspiciousCallToStrncat.ql)
> +- [ ] [cpp/use-after-free](https://github.com/github/codeql/blob/main/cpp/ql/src/Critical/UseAfterFree.ql)
> +- [ ] [cpp/user-controlled-null-termination-tainted](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-
> 170/ImproperNullTerminationTainted.ql)
> +- [ ] [cpp/wrong-number-format-
> arguments](https://github.com/github/codeql/blob/main/cpp/ql/src/Likely%20Bugs/Format/WrongNumberOfFormatArguments.ql)
> +- [ ] [cpp/wrong-type-format-
> argument](https://github.com/github/codeql/blob/main/cpp/ql/src/Likely%20Bugs/Format/WrongTypeFormatArguments.ql)
> +
> +Additional queries completed:
> +
> +- [x] [cpp/overrunning-write](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-120/OverrunWrite.ql)
> +- [x] [cpp/overrunning-write-with-float](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-
> 120/OverrunWriteFloat.ql)
> +- [x] [cpp/very-likely-overrunning-write](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-
> 120/VeryLikelyOverrunWrite.ql)
> +
> +### Query Filtering in edk2
> +
> +CodeQL query files (`.ql` files) contain metadata about the query. For example,
> +[cpp/conditionally-uninitialized-variable](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-
> 457/ConditionallyUninitializedVariable.ql)
> +states the following about the query:
> +
> +```plaintext
> +/**
> + * @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
> + */
> +```
> +
> +edk2 automatically include queries against certain criteria using "query filters". For example, this could include any
> +`problem` query above a certain `security-severity` level. Or all queries with `security` in `tags`.
> +
> +Because edk2 favors consistency in CI results, the project maintains a relatively fixed query set that is updated with
> +individual queries over time.
> +
> +- [edk2 Active Queries](https://github.com/tianocore/edk2/blob/master/.github/codeql/edk2.qls)
> +- [edk2 Active  CodeQL Configuration](https://github.com/tianocore/edk2/blob/master/.github/codeql/codeql-config.yml)
> +
> +> _Note:_ Additional queries can be found here as well - https://lgtm.com/search?q=cpp&t=rules
> +
> +### Process for Suggesting New Queries for edk2
> +
> +New query adoption in edk2 can be proposed by sending an RFC to the TianoCore development mailing list
> +(devel@edk2.groups.io) with the query link and justification for adopting the query in edk2.
> +
> +Everyone is welcome to suggest new queries.
> +
> +### Query Enabling Process
> +
> +Enabling a new query may trigger zero to thousands of alerts. Therefore, two paths are used to enable a new query in
> +the project.
> +
> +1. A single patch series - The first set of patches fixes the issues needed for the query to pass. The later set of
> +   patches enables the query.
> +2. A query enabling branch - A branch is created where multiple contributors can work together on fixing issues related
> +   to enabling a new query. Once the branch is ready, the history is cleaned up into a patch series that is submitted
> +   to the edk2 project.
> +
> +(1) is recommended if the query is relatively simple to enable and one or two people are doing the work. (2) is
> +recommended if a lot of effort is needed to fix issues for the query especially issues spanning across packages.
> +
> +If a query is deemed fruitless during enabling testing, it can simply be rejected. The goal for CodeQL in edk2 is to
> +enable an effective set of queries that improve the codebase. As the list of enabled queries grows, total CodeQL
> +coverage will increase against active pull requests. We want to have relevant and effective coverage.
> +
> +### CodeQL in Pull Requests
> +
> +TianoCore is enabling CodeQL in a step-by-step fashion. The goal with this approach is to make steady progress
> +enabling CodeQL to become more comprehensive and useful while not impacting day-to-day code contributions.
> +
> +Throughout the process described in this section, CodeQL Code Scanning is be a mandatory status check for edk2
> +pull requests.
> +
> +### Dismissing CodeQL Alerts
> +
> +The following documentation describes how to dismiss alerts:
> +[Dismissing Alerts](https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-
> vulnerabilities-and-errors/managing-code-scanning-alerts-for-your-repository#dismissing--alerts)
> +
> +> _Note:_ If query has a false positive a GitHub Issue can be submitted in the
> +> [CodeQL repo issues page](https://github.com/github/codeql/issues) with the `false-positive` tag to help improve
> +> the query.
> +
> +## CodeQL CLI Local Commands
> +
> +The [CodeQL CLI](https://codeql.github.com/docs/codeql-cli/) can be used as follows to wrap around the edk2 build
> +process (`MdeModulePkg` in this case) to generate a database in the directory `cpp-database`. The example shown uses
> +[stuart](https://github.com/tianocore/edk2-pytool-extensions) build commands.
> +
> +```cmd
> +codeql database create cpp-database --language=cpp --command="stuart_ci_build -c .pytool/CISettings.py -p MdeModulePkg
> +-a IA32,X64 TOOL_CHAIN_TAG=VS2019 Target=DEBUG --clean" --overwrite
> +```
> +
> +The following command can be used to generate a [SARIF file](https://codeql.github.com/docs/codeql-cli/sarif-output/)
> +(called `query-results.sarif`) from that database with the results of the
> +[cpp/conditionally-uninitialized-variable](https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-
> 457/ConditionallyUninitializedVariable.ql) query:
> +
> +```cmd
> +codeql database analyze cpp-database codeql\cpp\ql\src\Security\CWE\CWE-457\ConditionallyUninitializedVariable.ql --
> format=sarifv2.1.0 --output=query-results.sarif
> +```
> +
> +SARIF logs can be read by log viewers such as the [Sarif Viewer](https://marketplace.visualstudio.com/items?itemName=MS-
> SarifVSCode.sarif-viewer) extension for [VS Code](https://code.visualstudio.com/).
> +
> +## The CodeQL Project
> +
> +CodeQL is an actively maintained project. Here is a comparison of edk2 commit activity versus CodeQL for reference:
> +
> +- [CodeQL Commit Activity](https://github.com/github/codeql/graphs/commit-activity)
> +- [edk2 Commit Activity](https://github.com/github/codeql/graphs/commit-activity)
> +
> +Because CodeQL does maintain a strong open-source presence, the TianoCore community should be able to file
> +[issues](https://github.com/github/codeql/issues) and [pull requests](https://github.com/github/codeql/pulls)
> +into the project.
> +
> +---
> +
> +> The original RFC for adoption of CodeQL in edk2 is available here for reference:
> +> [Adoption of CodeQL in edk2](https://github.com/tianocore/edk2/discussions/3258#discussioncomment-3682099)
> --
> 2.28.0.windows.1
> 
> 
> 
> 
>