From 74694ad780f56518591e12de067c3533cfae84d6 Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Wed, 4 Oct 2023 18:22:59 -0700 Subject: [PATCH 01/11] Update outdated test result for KeWaitLocal. --- src/drivers/test/diff/KeWaitLocal.sarif | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/drivers/test/diff/KeWaitLocal.sarif b/src/drivers/test/diff/KeWaitLocal.sarif index dea8275e..4ac852d9 100644 --- a/src/drivers/test/diff/KeWaitLocal.sarif +++ b/src/drivers/test/diff/KeWaitLocal.sarif @@ -1,7 +1,7 @@ { "all": { - "+": 0, - "-": 0 + "+": 1, + "-": 1 }, "error": { "+": 0, @@ -9,9 +9,20 @@ "codes": [] }, "warning": { - "+": 0, - "-": 0, - "codes": [] + "+": 1, + "-": 1, + "codes": [ + [ + "cpp/drivers/kewaitlocal-requires-kernel-mode [good_use](1): KeWaitForSingleObject should have a KernelMode AccessMode when the [first argument](2) is local", + 0, + 1 + ], + [ + "cpp/drivers/kewaitlocal-requires-kernel-mode KeWaitForSingleObject should have a KernelMode AccessMode when the first argument is local", + 1, + 0 + ] + ] }, "note": { "+": 0, From b1cc5852f28969ea1575a31941f2bb5d5174b44f Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Tue, 10 Oct 2023 17:06:57 -0700 Subject: [PATCH 02/11] Update to CodeQL version 2.14.4 (#82) * Update to CodeQL 2.14.4 Update cpp-all to 0.9.2, cpp-queries to 0.7.4 * Update README.md * Update build-codeql.yaml Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> --------- Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> --- .github/workflows/build-codeql.yaml | 2 +- README.md | 10 +++++----- src/codeql-pack.lock.yml | 16 +++++++++++----- src/qlpack.lock.yml | 10 ---------- src/qlpack.yml | 6 +++--- 5 files changed, 20 insertions(+), 24 deletions(-) delete mode 100644 src/qlpack.lock.yml diff --git a/.github/workflows/build-codeql.yaml b/.github/workflows/build-codeql.yaml index 62562631..3c358b1d 100644 --- a/.github/workflows/build-codeql.yaml +++ b/.github/workflows/build-codeql.yaml @@ -32,7 +32,7 @@ jobs: with: owner: "github" repo: "codeql-cli-binaries" - tag: "v2.11.5" + tag: "v2.14.4" file: "codeql-win64.zip" - name: Unzip CodeQL CLI diff --git a/README.md b/README.md index 81b6d62b..891f27de 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ This repository contains open-source components for supplemental use in developi | Branch to use | CodeQL CLI version | |--------------------------|--------------------| -| main | 2.11.5 | +| main | 2.14.4 | ### For Windows Hardware Compatibility Program Use @@ -17,7 +17,7 @@ This repository contains open-source components for supplemental use in developi | Windows 11 | WHCP_21H2 | 2.4.6 | | Windows 11, version 22H2 | WHCP_22H2 | 2.6.3 | -For general use, use the `main` branch along with [version 2.11.5 of the CodeQL CLI](https://github.com/github/codeql-cli-binaries/releases/tag/v2.11.5). +For general use, use the `main` branch along with [version 2.14.4 of the CodeQL CLI](https://github.com/github/codeql-cli-binaries/releases/tag/v2.14.4). ## Quickstart @@ -30,7 +30,7 @@ For general use, use the `main` branch along with [version 2.11.5 of the CodeQL For the WHCP Program, use the CodeQL CLI version in accordance with the table above and Windows release you are certifying for: [version 2.4.6](https://github.com/github/codeql-cli-binaries/releases/tag/v2.4.6) or [version 2.6.3](https://github.com/github/codeql-cli-binaries/releases/tag/v2.6.3). - For general use with the `main` branch, use [CodeQL CLI version 2.11.5](https://github.com/github/codeql-cli-binaries/releases/tag/v2.11.5). + For general use with the `main` branch, use [CodeQL CLI version 2.14.4](https://github.com/github/codeql-cli-binaries/releases/tag/v2.14.4). 1. Clone and install the Windows Driver Developer Supplemental Tools repository which contains the CodeQL queries specific for drivers: @@ -56,8 +56,8 @@ For general use, use the `main` branch along with [version 2.11.5 of the CodeQL 1. Verify CodeQL is installed correctly by checking the version: ``` D:\codeql-home\codeql>codeql --version - CodeQL command-line toolchain release 2.11.5. - Copyright (C) 2019-2022 GitHub, Inc. + CodeQL command-line toolchain release 2.14.4. + Copyright (C) 2019-2023 GitHub, Inc. Unpacked in: D:\codeql-home\codeql Analysis results depend critically on separately distributed query and extractor modules. To list modules that are visible to the toolchain, diff --git a/src/codeql-pack.lock.yml b/src/codeql-pack.lock.yml index 4fc89962..2c012295 100644 --- a/src/codeql-pack.lock.yml +++ b/src/codeql-pack.lock.yml @@ -2,11 +2,17 @@ lockVersion: 1.0.0 dependencies: codeql/cpp-all: - version: 0.4.6 - codeql/ssa: - version: 0.0.7 + version: 0.9.2 codeql/cpp-queries: - version: 0.4.6 + version: 0.7.4 + codeql/dataflow: + version: 0.0.3 + codeql/ssa: + version: 0.1.4 codeql/suite-helpers: - version: 0.3.6 + version: 0.6.4 + codeql/tutorial: + version: 0.1.4 + codeql/util: + version: 0.1.4 compiled: false diff --git a/src/qlpack.lock.yml b/src/qlpack.lock.yml deleted file mode 100644 index 5efc9529..00000000 --- a/src/qlpack.lock.yml +++ /dev/null @@ -1,10 +0,0 @@ ---- -dependencies: - codeql/cpp-queries: - version: 0.2.0 - codeql/suite-helpers: - version: 0.1.0 - codeql/cpp-all: - version: 0.3.5 -compiled: false -lockVersion: 1.0.0 diff --git a/src/qlpack.yml b/src/qlpack.yml index 15fcb2f0..1d1a053c 100644 --- a/src/qlpack.yml +++ b/src/qlpack.yml @@ -2,10 +2,10 @@ # Licensed under the MIT license. name: microsoft/windows-drivers -version: 0.1.0 +version: 0.2.0 dependencies: - codeql/cpp-queries: ^0.4.5 - codeql/cpp-all: ^0.4.5 + codeql/cpp-queries: 0.7.4 + codeql/cpp-all: 0.9.2 suites: suites extractor: cpp licenses: MIT From 1093495fbfcaa7144f272648ad5194f758bc613b Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Fri, 20 Oct 2023 13:58:06 -0700 Subject: [PATCH 03/11] Overhaul IRQL library and IRQLTooHigh/IRQLTooLowChecks. (#83) * Initial work at IRQL-checking * Significant extra IRQL work. * In-progress work * More puttering around with IRQL * Update to CodeQL 2.14.4 Update cpp-all to 0.9.2, cpp-queries to 0.7.4 * Commit more IRQL code. Needs cleanup. * Some cleanup and minor fixes to entry IRQL evaluation. * Replace old Irql high/low checks with new version and update library. Still needs cleanup. * Irql.qll cleanup * Get rid of old prototype version of IrqlTooLow * Update README.md * Clean up file names * Clean up queries. * Update test script for IRQL queries. * Update build-codeql.yaml Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Update ported_driver_ca_checks.qls * Test script fixes for IrqlTooHigh/IrqlTooLow --------- Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> --- .../IrqlTooHigh/IrqlTooHigh.qhelp} | 2 +- .../experimental/IrqlTooHigh/IrqlTooHigh.ql | 40 ++ .../IrqlTooHigh/IrqlTooHigh.sarif | 361 +++++++++++ .../IrqlTooHigh}/driver_snippet.c | 0 .../experimental/IrqlTooLow/IrqlTooLow.qhelp} | 2 +- .../experimental/IrqlTooLow/IrqlTooLow.ql | 39 ++ .../experimental/IrqlTooLow/IrqlTooLow.sarif | 299 +++++++++ .../experimental/IrqlTooLow}/driver_snippet.c | 0 src/drivers/libraries/Irql.qll | 603 ++++++++++++++++-- src/drivers/libraries/SAL.qll | 5 + .../test/build_create_analyze_test.cmd | 4 +- src/drivers/test/diff/IrqlTooHigh.sarif | 21 + src/drivers/test/diff/IrqlTooLow.sarif | 21 + src/drivers/wdm/libraries/WdmDrivers.qll | 7 + .../experimental/IrqTooHigh/IrqTooHigh.ql | 61 -- .../experimental/IrqTooHigh/IrqTooHigh.sarif | 169 ----- .../experimental/IrqTooLow/IrqTooLow.ql | 26 - .../experimental/IrqTooLow/IrqTooLow.sarif | 107 ---- src/suites/ported_driver_ca_checks.qls | 4 +- 19 files changed, 1344 insertions(+), 427 deletions(-) rename src/drivers/{wdm/queries/experimental/IrqTooHigh/IrqTooHigh.qhelp => general/queries/experimental/IrqlTooHigh/IrqlTooHigh.qhelp} (86%) create mode 100644 src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.ql create mode 100644 src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.sarif rename src/drivers/{wdm/queries/experimental/IrqTooHigh => general/queries/experimental/IrqlTooHigh}/driver_snippet.c (100%) rename src/drivers/{wdm/queries/experimental/IrqTooLow/IrqTooLow.qhelp => general/queries/experimental/IrqlTooLow/IrqlTooLow.qhelp} (87%) create mode 100644 src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.ql create mode 100644 src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.sarif rename src/drivers/{wdm/queries/experimental/IrqTooLow => general/queries/experimental/IrqlTooLow}/driver_snippet.c (100%) create mode 100644 src/drivers/test/diff/IrqlTooHigh.sarif create mode 100644 src/drivers/test/diff/IrqlTooLow.sarif delete mode 100644 src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.ql delete mode 100644 src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.sarif delete mode 100644 src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.ql delete mode 100644 src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.sarif diff --git a/src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.qhelp b/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.qhelp similarity index 86% rename from src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.qhelp rename to src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.qhelp index fda952f8..83ee1a7c 100644 --- a/src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.qhelp +++ b/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.qhelp @@ -7,7 +7,7 @@

- The driver is executing at an IRQL that is too high for the function that it is calling. Consult the WDK documentation for the function and verify the IRQL at which the function can be called. + The driver is executing at an IRQL that is too high for the function that it is calling. Consult the WDK documentation for the function and verify the IRQL at which the function can be called. If you have applied custom IRQL annotations to your own functions, confirm that they are accurate.

diff --git a/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.ql b/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.ql new file mode 100644 index 00000000..afd1a25a --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.ql @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +/** + * @id cpp/drivers/irql-too-high + * @name IRQL too high (C28120) + * @description A function annotated with IRQL requirements was called at an IRQL too high for the requirements. + * @platform Desktop + * @security.severity Low + * @feature.area Multiple + * @impact Exploitable Design + * @repro.text The following function call is taking place at an IRQL too high for what the call target is annotated as. + * @owner.email sdat@microsoft.com + * @opaqueid CQLD-C28120 + * @kind problem + * @problem.severity warning + * @precision medium + * @tags correctness + * wddst + * @scope domainspecific + * @query-version v2 + */ + +import cpp +import drivers.libraries.Irql + +from FunctionCall call, IrqlRestrictsFunction irqlFunc, ControlFlowNode prior, int irqlRequirement +where + call.getTarget() = irqlFunc and + prior = call.getAPredecessor() and + ( + irqlFunc.(IrqlMaxAnnotatedFunction).getIrqlLevel() = irqlRequirement + or + irqlFunc.(IrqlRequiresAnnotatedFunction).getIrqlLevel() = irqlRequirement + ) and + irqlRequirement < min(getPotentialExitIrqlAtCfn(prior)) +select call, + "$@: IRQL potentially too high at call to $@. Maximum IRQL for this call: " + irqlRequirement + + ", IRQL at preceding node: " + min(getPotentialExitIrqlAtCfn(prior)), + call.getControlFlowScope(), call.getControlFlowScope().getQualifiedName(), call, + call.getTarget().toString() diff --git a/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.sarif b/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.sarif new file mode 100644 index 00000000..52a2f7a3 --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.sarif @@ -0,0 +1,361 @@ +{ + "$schema" : "https://json.schemastore.org/sarif-2.1.0.json", + "version" : "2.1.0", + "runs" : [ { + "tool" : { + "driver" : { + "name" : "CodeQL", + "organization" : "GitHub", + "semanticVersion" : "2.14.4", + "notifications" : [ { + "id" : "cpp/baseline/expected-extracted-files", + "name" : "cpp/baseline/expected-extracted-files", + "shortDescription" : { + "text" : "Expected extracted files" + }, + "fullDescription" : { + "text" : "Files appearing in the source archive that are expected to be extracted." + }, + "defaultConfiguration" : { + "enabled" : true + }, + "properties" : { + "tags" : [ "expected-extracted-files", "telemetry" ] + } + } ], + "rules" : [ { + "id" : "cpp/drivers/irql-too-high", + "name" : "cpp/drivers/irql-too-high", + "shortDescription" : { + "text" : "IRQL too high (C28120)" + }, + "fullDescription" : { + "text" : "A function annotated with IRQL requirements was called at an IRQL too high for the requirements." + }, + "defaultConfiguration" : { + "enabled" : true, + "level" : "warning" + }, + "properties" : { + "tags" : [ "correctness", "wddst" ], + "description" : "A function annotated with IRQL requirements was called at an IRQL too high for the requirements.", + "feature.area" : "Multiple", + "id" : "cpp/drivers/irql-too-high", + "impact" : "Exploitable Design", + "kind" : "problem", + "name" : "IRQL too high (C28120)", + "opaqueid" : "CQLD-C28120", + "owner.email" : "sdat@microsoft.com", + "platform" : "Desktop", + "precision" : "medium", + "problem.severity" : "warning", + "query-version" : "v2", + "repro.text" : "The following function call is taking place at an IRQL too high for what the call target is annotated as.", + "scope" : "domainspecific", + "security.severity" : "Low" + } + } ] + }, + "extensions" : [ { + "name" : "microsoft/windows-drivers", + "semanticVersion" : "0.2.0+4842fd4116871d3b47eede85c2c4497b43c34d57", + "locations" : [ { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", + "description" : { + "text" : "The QL pack root directory." + } + }, { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/qlpack.yml", + "description" : { + "text" : "The QL pack definition file." + } + } ] + } ] + }, + "invocations" : [ { + "toolExecutionNotifications" : [ { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + } ], + "executionSuccessful" : true + } ], + "artifacts" : [ { + "location" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } ], + "results" : [ { + "ruleId" : "cpp/drivers/irql-too-high", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-too-high", + "index" : 0 + }, + "message" : { + "text" : "[TestInner1](1): IRQL potentially too high at call to [TestInner2](2). Maximum IRQL for this call: 1, IRQL at preceding node: 2" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 42, + "startColumn" : 12, + "endColumn" : 22 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "1defbc9e59f0310b:1", + "primaryLocationStartColumnFingerprint" : "7" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 41, + "startColumn" : 10, + "endColumn" : 20 + } + }, + "message" : { + "text" : "TestInner1" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 42, + "startColumn" : 12, + "endColumn" : 22 + } + }, + "message" : { + "text" : "TestInner2" + } + } ] + }, { + "ruleId" : "cpp/drivers/irql-too-high", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-too-high", + "index" : 0 + }, + "message" : { + "text" : "[TestInner2](1): IRQL potentially too high at call to [TestInner4](2). Maximum IRQL for this call: 0, IRQL at preceding node: 2" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 36, + "startColumn" : 14, + "endColumn" : 24 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "7ae2af586e0dd70a:1", + "primaryLocationStartColumnFingerprint" : "9" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 26, + "startColumn" : 10, + "endColumn" : 20 + } + }, + "message" : { + "text" : "TestInner2" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 36, + "startColumn" : 14, + "endColumn" : 24 + } + }, + "message" : { + "text" : "TestInner4" + } + } ] + }, { + "ruleId" : "cpp/drivers/irql-too-high", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-too-high", + "index" : 0 + }, + "message" : { + "text" : "[DpcForIsrRoutine](1): IRQL potentially too high at call to [IoGetInitialStack](2). Maximum IRQL for this call: 1, IRQL at preceding node: 2" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + }, + "region" : { + "startLine" : 366, + "startColumn" : 5, + "endColumn" : 22 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "48e9dbeaff18e9e7:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + }, + "region" : { + "startLine" : 347, + "endColumn" : 17 + } + }, + "message" : { + "text" : "DpcForIsrRoutine" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + }, + "region" : { + "startLine" : 366, + "startColumn" : 5, + "endColumn" : 22 + } + }, + "message" : { + "text" : "IoGetInitialStack" + } + } ] + } ], + "columnKind" : "utf16CodeUnits", + "properties" : { + "semmle.formatSpecifier" : "sarifv2.1.0" + } + } ] +} \ No newline at end of file diff --git a/src/drivers/wdm/queries/experimental/IrqTooHigh/driver_snippet.c b/src/drivers/general/queries/experimental/IrqlTooHigh/driver_snippet.c similarity index 100% rename from src/drivers/wdm/queries/experimental/IrqTooHigh/driver_snippet.c rename to src/drivers/general/queries/experimental/IrqlTooHigh/driver_snippet.c diff --git a/src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.qhelp b/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.qhelp similarity index 87% rename from src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.qhelp rename to src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.qhelp index cda052cd..7582b8fb 100644 --- a/src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.qhelp +++ b/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.qhelp @@ -7,7 +7,7 @@

- The driver is executing at an IRQL that is too low for the function that it is calling. Consult the WDK documentation for the function and verify the IRQL at which the function can be called. + The driver is executing at an IRQL that is too low for the function that it is calling. Consult the WDK documentation for the function and verify the IRQL at which the function can be called. If you have applied custom IRQL annotations to your own functions, confirm that they are accurate.

diff --git a/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.ql b/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.ql new file mode 100644 index 00000000..9b42ad7a --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.ql @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +/** + * @id cpp/drivers/irql-too-low + * @name IRQL too low (C28121) + * @description A function annotated with IRQL requirements was called at an IRQL too low for the requirements. + * @platform Desktop + * @security.severity Low + * @feature.area Multiple + * @impact Exploitable Design + * @repro.text The following function call is taking place at an IRQL too low for what the call target is annotated as. + * @owner.email sdat@microsoft.com + * @opaqueid CQLD-C28121 + * @kind problem + * @problem.severity warning + * @precision medium + * @tags correctness + * wddst + * @scope domainspecific + * @query-version v2 + */ + +import cpp +import drivers.libraries.Irql + +from FunctionCall call, IrqlRestrictsFunction irqlFunc, ControlFlowNode prior, int irqlRequirement +where + call.getTarget() = irqlFunc and + prior = call.getAPredecessor() and + ( + irqlFunc.(IrqlMinAnnotatedFunction).getIrqlLevel() = irqlRequirement + or + irqlFunc.(IrqlRequiresAnnotatedFunction).getIrqlLevel() = irqlRequirement + ) and + irqlRequirement > max(getPotentialExitIrqlAtCfn(prior)) +select call, + "$@: IRQL potentially too low at call to $@. Minimum IRQL for this call: " + irqlRequirement + + ", IRQL at preceding node: " + max(getPotentialExitIrqlAtCfn(prior)), call.getControlFlowScope(), + call.getControlFlowScope().getQualifiedName(), call, call.getTarget().toString() diff --git a/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.sarif b/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.sarif new file mode 100644 index 00000000..3c5d989e --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.sarif @@ -0,0 +1,299 @@ +{ + "$schema" : "https://json.schemastore.org/sarif-2.1.0.json", + "version" : "2.1.0", + "runs" : [ { + "tool" : { + "driver" : { + "name" : "CodeQL", + "organization" : "GitHub", + "semanticVersion" : "2.14.4", + "notifications" : [ { + "id" : "cpp/baseline/expected-extracted-files", + "name" : "cpp/baseline/expected-extracted-files", + "shortDescription" : { + "text" : "Expected extracted files" + }, + "fullDescription" : { + "text" : "Files appearing in the source archive that are expected to be extracted." + }, + "defaultConfiguration" : { + "enabled" : true + }, + "properties" : { + "tags" : [ "expected-extracted-files", "telemetry" ] + } + } ], + "rules" : [ { + "id" : "cpp/drivers/irql-too-low", + "name" : "cpp/drivers/irql-too-low", + "shortDescription" : { + "text" : "IRQL too low (C28121)" + }, + "fullDescription" : { + "text" : "A function annotated with IRQL requirements was called at an IRQL too low for the requirements." + }, + "defaultConfiguration" : { + "enabled" : true, + "level" : "warning" + }, + "properties" : { + "tags" : [ "correctness", "wddst" ], + "description" : "A function annotated with IRQL requirements was called at an IRQL too low for the requirements.", + "feature.area" : "Multiple", + "id" : "cpp/drivers/irql-too-low", + "impact" : "Exploitable Design", + "kind" : "problem", + "name" : "IRQL too low (C28121)", + "opaqueid" : "CQLD-C28121", + "owner.email" : "sdat@microsoft.com", + "platform" : "Desktop", + "precision" : "medium", + "problem.severity" : "warning", + "query-version" : "v2", + "repro.text" : "The following function call is taking place at an IRQL too low for what the call target is annotated as.", + "scope" : "domainspecific", + "security.severity" : "Low" + } + } ] + }, + "extensions" : [ { + "name" : "microsoft/windows-drivers", + "semanticVersion" : "0.2.0+4842fd4116871d3b47eede85c2c4497b43c34d57", + "locations" : [ { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", + "description" : { + "text" : "The QL pack root directory." + } + }, { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/qlpack.yml", + "description" : { + "text" : "The QL pack definition file." + } + } ] + } ] + }, + "invocations" : [ { + "toolExecutionNotifications" : [ { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + } ], + "executionSuccessful" : true + } ], + "artifacts" : [ { + "location" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } ], + "results" : [ { + "ruleId" : "cpp/drivers/irql-too-low", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-too-low", + "index" : 0 + }, + "message" : { + "text" : "[TestInner1](1): IRQL potentially too low at call to [TestInner2](2). Minimum IRQL for this call: 1, IRQL at preceding node: 0" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 41, + "startColumn" : 12, + "endColumn" : 22 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "1defbc9e59f0310b:1", + "primaryLocationStartColumnFingerprint" : "7" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 40, + "startColumn" : 10, + "endColumn" : 20 + } + }, + "message" : { + "text" : "TestInner1" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 41, + "startColumn" : 12, + "endColumn" : 22 + } + }, + "message" : { + "text" : "TestInner2" + } + } ] + }, { + "ruleId" : "cpp/drivers/irql-too-low", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-too-low", + "index" : 0 + }, + "message" : { + "text" : "[someFunc](1): IRQL potentially too low at call to [TestInner3](2). Minimum IRQL for this call: 2, IRQL at preceding node: 0" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 21, + "startColumn" : 12, + "endColumn" : 22 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "bf32240018f4d9fb:1", + "primaryLocationStartColumnFingerprint" : "7" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 20, + "startColumn" : 10, + "endColumn" : 18 + } + }, + "message" : { + "text" : "someFunc" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 21, + "startColumn" : 12, + "endColumn" : 22 + } + }, + "message" : { + "text" : "TestInner3" + } + } ] + } ], + "columnKind" : "utf16CodeUnits", + "properties" : { + "semmle.formatSpecifier" : "sarifv2.1.0" + } + } ] +} \ No newline at end of file diff --git a/src/drivers/wdm/queries/experimental/IrqTooLow/driver_snippet.c b/src/drivers/general/queries/experimental/IrqlTooLow/driver_snippet.c similarity index 100% rename from src/drivers/wdm/queries/experimental/IrqTooLow/driver_snippet.c rename to src/drivers/general/queries/experimental/IrqlTooLow/driver_snippet.c diff --git a/src/drivers/libraries/Irql.qll b/src/drivers/libraries/Irql.qll index f809bad5..5d335316 100644 --- a/src/drivers/libraries/Irql.qll +++ b/src/drivers/libraries/Irql.qll @@ -2,30 +2,137 @@ // Licensed under the MIT license. import cpp import drivers.libraries.SAL +import drivers.wdm.libraries.WdmDrivers -class IrqlTypeDefinition extends SALAnnotation { +/** + * A macro in wdm.h that represents an IRQL level, + * such as PASSIVE_LEVEL, DISPATCH_LEVEL, etc. + */ +class IrqlMacro extends Macro { + int irqlLevelAsInt; + + IrqlMacro() { + this.getName().matches("%_LEVEL") and + this.getFile().getBaseName().matches("wdm.h") and + this.getBody().toInt() = irqlLevelAsInt and + irqlLevelAsInt >= 0 and + irqlLevelAsInt <= 31 + } + + /** Returns the integer value of this IRQL level. */ + int getIrqlLevel() { result = irqlLevelAsInt } + + /** + * Returns the highest IRQL in wdm.h across this database. + * May cause incorrect results if database contains both 32-bit + * and 64-bit builds. + */ + int getGlobalMaxIrqlLevel() { + result = + any(int i | + exists(IrqlMacro im | + i = im.getIrqlLevel() and + not exists(IrqlMacro im2 | im2 != im and im2.getIrqlLevel() > im.getIrqlLevel()) + ) + ) + } +} + +/** An \_IRQL\_saves\_global\_(parameter, kind) annotation. */ +class IrqlSavesGlobalAnnotation extends SALAnnotation { + MacroInvocation irqlMacroInvocation; + + IrqlSavesGlobalAnnotation() { + // Needs to include other function and parameter annotations too + this.getMacroName().matches(["_IRQL_saves_global_"]) and + irqlMacroInvocation.getParentInvocation() = this + } +} + +/** An \_IRQL\_restores\_global\_(parameter, kind) annotation. */ +class IrqlRestoresGlobalAnnotation extends SALAnnotation { + MacroInvocation irqlMacroInvocation; + + IrqlRestoresGlobalAnnotation() { + // Needs to include other function and parameter annotations too + this.getMacroName().matches(["_IRQL_restores_global_"]) and + irqlMacroInvocation.getParentInvocation() = this + } +} + +/** Standard IRQL annotations which apply to entire functions and manipulate or constrain the IRQL. */ +class IrqlFunctionAnnotation extends SALAnnotation { string irqlLevel; string irqlAnnotationName; + MacroInvocation irqlMacroInvocation; - //IRQL Annotation Types - IrqlTypeDefinition() { - //Needs to include other function and parameter annotations too - this.getMacroName().matches(["_IRQL_requires_"]) and + IrqlFunctionAnnotation() { + this.getMacroName() + .matches([ + "_IRQL_requires_", "_IRQL_requires_min_", "_IRQL_requires_max_", "_IRQL_raises_", + "_IRQL_saves_" + ]) and irqlAnnotationName = this.getMacroName() and - exists(MacroInvocation mi | - mi.getParentInvocation() = this and - irqlLevel = mi.getMacro().getHead() - ) + irqlMacroInvocation.getParentInvocation() = this and + irqlLevel = irqlMacroInvocation.getMacro().getHead() } - string getIrqlLevelFull() { result = irqlLevel } + /** Returns the raw text of the IRQL value used in this annotation. */ + string getIrqlLevelString() { result = irqlLevel } + + /** Returns the text of this annotation (i.e. \_IRQL\_requires\_, etc.) */ + string getIrqlMacroName() { result = irqlAnnotationName } + + /** Evaluate the IRQL specified in this annotation, if possible. */ + int getIrqlLevel() { + if exists(IrqlMacro im | im.getHead().matches(this.getIrqlLevelString())) + then + result = + any(int i | + exists(IrqlMacro im | + im.getIrqlLevel() = i and + im.getHead().matches(this.getIrqlLevelString()) + ) + ) + else result = -1 + } +} + +/** Represents an "\_IRQL\_requires\_same\_" annotation. */ +class IrqlSameAnnotation extends SALAnnotation { + string irqlAnnotationName; + + IrqlSameAnnotation() { + this.getMacroName().matches(["_IRQL_requires_same_"]) and + irqlAnnotationName = this.getMacroName() + } string getIrqlMacroName() { result = irqlAnnotationName } } +/** An "\_IRQL\_requires\_max\_" annotation. */ +class IrqlMaxAnnotation extends IrqlFunctionAnnotation { + IrqlMaxAnnotation() { this.getMacroName().matches("_IRQL_requires_max_") } +} + +/** An "\_IRQL\_raises\_" annotation. */ +class IrqlRaisesAnnotation extends IrqlFunctionAnnotation { + IrqlRaisesAnnotation() { this.getMacroName().matches("_IRQL_raises_") } +} + +/** An "\_IRQL\_requires\_min\_" annotation. */ +class IrqlMinAnnotation extends IrqlFunctionAnnotation { + IrqlMinAnnotation() { this.getMacroName().matches("_IRQL_requires_min_") } +} + +/** An "\_IRQL\_requires\_" annotation. */ +class IrqlRequiresAnnotation extends IrqlFunctionAnnotation { + IrqlRequiresAnnotation() { this.getMacroName().matches("_IRQL_requires_") } +} + /** - * Represents a SAL annotation indicating that the parameter in - * question is used as part of adjusting the IRQL. + * A SAL annotation indicating that the parameter in + * question is used to store or restore the IRQL. */ class IrqlParameterAnnotation extends SALAnnotation { string irqlAnnotationName; @@ -36,89 +143,469 @@ class IrqlParameterAnnotation extends SALAnnotation { exists(MacroInvocation mi | mi.getParentInvocation() = this) } + /** Get the text of the annotation. */ string getIrqlMacroName() { result = irqlAnnotationName } } /** - * Represents a SAL annotation indicating that the parameter in + * A SAL annotation indicating that the parameter in * question contains an IRQL value that the system will be set to. */ class IrqlRestoreAnnotation extends IrqlParameterAnnotation { - IrqlRestoreAnnotation() { - this.getMacroName().matches(["_IRQL_restores_"]) - } + IrqlRestoreAnnotation() { this.getMacroName().matches(["_IRQL_restores_"]) } } /** - * Represents a SAL annotation indicating that the parameter in - * question will have the current IRQL saved to it. + * A SAL annotation indicating that can be used in two ways: + * - If applied to a function, the function returns the previous IRQL or otherwise saves the IRQL. + * - If applied to a parameter, the function saves the IRQL to the parameter. */ -class IrqlSaveAnnotation extends IrqlParameterAnnotation { - IrqlSaveAnnotation() { - this.getMacroName().matches(["_IRQL_saves_"]) - } +class IrqlSaveAnnotation extends IrqlFunctionAnnotation { + IrqlSaveAnnotation() { this.getMacroName().matches(["_IRQL_saves_"]) } } -/** Represents a parameter that is annotated with "\_IRQL\_restores\_". */ +/** A parameter that is annotated with "\_IRQL\_restores\_". */ class IrqlRestoreParameter extends Parameter { IrqlRestoreParameter() { exists(IrqlRestoreAnnotation ira | ira.getDeclaration() = this) } } -/** Represents a parameter that is annotated with "\_IRQL\_saves\_". */ +/** A parameter that is annotated with "\_IRQL\_saves\_". */ class IrqlSaveParameter extends Parameter { IrqlSaveParameter() { exists(IrqlSaveAnnotation isa | isa.getDeclaration() = this) } } -//Represents Irql annotationed functions. -class IrqlAnnotatedFunction extends Function { - string funcIrqlLevel; - string funcIrqlAnnotationName; +/** A typedef that has IRQL annotations applied to it. */ +class IrqlAnnotatedTypedef extends TypedefType { + IrqlFunctionAnnotation irqlAnnotation; + + IrqlAnnotatedTypedef() { irqlAnnotation.getTypedefDeclarations() = this } + + IrqlFunctionAnnotation getIrqlAnnotation() { result = irqlAnnotation } +} + +/** + * A function that is annotated in such a way that + * either its entry or exit IRQL is restricted, either by having a min/max value, + * a required value, or by raising the IRQL to a known value. + */ +cached +class IrqlRestrictsFunction extends Function { + IrqlFunctionAnnotation irqlAnnotation; + + cached + IrqlRestrictsFunction() { + exists(FunctionDeclarationEntry fde | + fde = this.getADeclarationEntry() and + irqlAnnotation.getDeclarationEntry() = fde + ) + or + exists(FunctionDeclarationEntry fde | + fde.getFunction() = this and + fde.getTypedefType().(IrqlAnnotatedTypedef).getIrqlAnnotation() = irqlAnnotation + ) + } + + cached + string getFuncIrqlAnnotation() { result = irqlAnnotation.getIrqlMacroName() } +} + +/** A function that changes the IRQL. */ +abstract class IrqlChangesFunction extends Function { } - IrqlAnnotatedFunction() { - exists(IrqlTypeDefinition itd, FunctionDeclarationEntry fde | +/** A function that is explicitly annotated to enter and exit at the same IRQL. */ +class IrqlRequiresSameAnnotatedFunction extends Function { + IrqlSameAnnotation irqlAnnotation; + + IrqlRequiresSameAnnotatedFunction() { + exists(FunctionDeclarationEntry fde | fde = this.getADeclarationEntry() and - itd.getDeclarationEntry() = fde and - funcIrqlLevel = itd.getIrqlLevelFull() and - funcIrqlAnnotationName = itd.getIrqlMacroName() + irqlAnnotation.getDeclarationEntry() = fde ) } +} + +/** A function that is annotated to run at a specific IRQL. */ +class IrqlRequiresAnnotatedFunction extends IrqlRestrictsFunction { + IrqlRequiresAnnotatedFunction() { irqlAnnotation instanceof IrqlRequiresAnnotation } + + int getIrqlLevel() { result = irqlAnnotation.(IrqlRequiresAnnotation).getIrqlLevel() } +} - private string getLevel() { result = funcIrqlLevel } +/** A function that is annotated to enter at or below a given IRQL. */ +class IrqlMaxAnnotatedFunction extends IrqlRestrictsFunction { + IrqlMaxAnnotatedFunction() { irqlAnnotation instanceof IrqlMaxAnnotation } + + int getIrqlLevel() { result = irqlAnnotation.(IrqlMaxAnnotation).getIrqlLevel() } +} + +/** A function that is annotated to enter at or above a given IRQL. */ +class IrqlMinAnnotatedFunction extends IrqlRestrictsFunction { + IrqlMinAnnotatedFunction() { irqlAnnotation instanceof IrqlMinAnnotation } + + int getIrqlLevel() { result = irqlAnnotation.(IrqlMinAnnotation).getIrqlLevel() } +} + +/** A function that is annotated to raise the IRQL to a given value. */ +class IrqlRaisesAnnotatedFunction extends IrqlRestrictsFunction, IrqlChangesFunction { + IrqlRaisesAnnotatedFunction() { irqlAnnotation instanceof IrqlRaisesAnnotation } + + int getIrqlLevel() { result = irqlAnnotation.(IrqlRaisesAnnotation).getIrqlLevel() } +} + +/** A function annotated to save the IRQL at the specified location upon entry. */ +class IrqlSavesGlobalAnnotatedFunction extends IrqlChangesFunction { + IrqlSavesGlobalAnnotation irqlAnnotation; + string irqlKind; + int irqlParamIndex; + + IrqlSavesGlobalAnnotatedFunction() { + exists(FunctionDeclarationEntry fde | + fde = this.getADeclarationEntry() and + irqlAnnotation.getDeclarationEntry() = fde + ) and + irqlKind = irqlAnnotation.getExpandedArgument(0) and + this.getParameter(irqlParamIndex).getName().matches(irqlAnnotation.getExpandedArgument(1)) + } + + string getIrqlKind() { result = irqlKind } + + int getIrqlParameterSlot() { result = irqlParamIndex } +} + +/** A function annotated to restore the IRQL from the specified location upon exit. */ +class IrqlRestoresGlobalAnnotatedFunction extends IrqlChangesFunction { + IrqlRestoresGlobalAnnotation irqlAnnotation; + string irqlKind; + int irqlParamIndex; + + IrqlRestoresGlobalAnnotatedFunction() { + exists(FunctionDeclarationEntry fde | + fde = this.getADeclarationEntry() and + irqlAnnotation.getDeclarationEntry() = fde + ) and + irqlKind = irqlAnnotation.getExpandedArgument(0) and + this.getParameter(irqlParamIndex).getName().matches(irqlAnnotation.getExpandedArgument(1)) + } - string getFuncIrqlName() { result = funcIrqlAnnotationName } + string getIrqlKind() { result = irqlKind } + + int getIrqlParameterSlot() { result = irqlParamIndex } +} + +/** + * An abstract class for functions that use the \_IRQL\_saves\_ annotation, + * either on the function definition or on a specific parameter. + */ +abstract class IrqlSavesFunction extends Function { } + +/** A function that has a parameter annotated \_IRQL\_saves\_. */ +class IrqlSavesToParameterFunction extends IrqlSavesFunction { + IrqlSaveParameter saveParameter; + int irqlParamIndex; + + IrqlSavesToParameterFunction() { this.getParameter(irqlParamIndex) = saveParameter } + + int getIrqlParameterSlot() { result = irqlParamIndex } +} + +/** A function that saves the IRQL as a return value. */ +class IrqlSavesViaReturnFunction extends IrqlSavesFunction { + IrqlSaveAnnotation irqlAnnotation; + + IrqlSavesViaReturnFunction() { + exists(FunctionDeclarationEntry fde | + fde = this.getADeclarationEntry() and + irqlAnnotation.getDeclarationEntry() = fde + ) + } +} + +/** A function that has a parameter annotated \_IRQL\_restores\_. */ +class IrqlRestoreAnnotatedFunction extends Function { + IrqlRestoreParameter restoreParameter; + int irqlParamIndex; + + IrqlRestoreAnnotatedFunction() { this.getParameter(irqlParamIndex) = restoreParameter } + + int getIrqlParameterSlot() { result = irqlParamIndex } +} + +/** A call to a function that has a parameter annotated \_IRQL\_restores\_. */ +class IrqlRestoreCall extends FunctionCall { + IrqlRestoreCall() { this.getTarget() instanceof IrqlRestoreAnnotatedFunction } /** - * Needs to include other levels too. From MSDN doc: Very few functions have both an upper bound other than DISPATCH_LEVEL and a lower bound other than PASSIVE_LEVEL. - * - * The other IRQL levels are Processor-specific + * A heuristic evaluation of the IRQL that the system is changing to. This is defined as + * "the IRQL before the corresponding save global call." */ int getIrqlLevel() { - if getLevel().matches("%PASSIVE_LEVEL%") - then result = 0 - else - if getLevel().matches("%APC_LEVEL%") - then result = 1 - else result = 2 + result = any(getPotentialExitIrqlAtCfn(this.getMostRecentRaise().getAPredecessor())) + } + + /** Returns the matching call to a function that saved the IRQL. */ + IrqlSaveCall getMostRecentRaise() { + result = + any(IrqlSaveCall sgic | + this.getAPredecessor*() = sgic and + matchingSaveCall(sgic) and + not exists(SavesGlobalIrqlCall sgic2 | + sgic2 != sgic and sgic2.getAPredecessor*() = sgic and matchingSaveCall(sgic2) + ) + ) + } + + /** + * Holds if a given call to an \_IRQL\_saves\_global\_ annotated function is using the same IRQL location as this. + */ + private predicate matchingSaveCall(IrqlSaveCall sgic) { + // Attempting to match all expr children leads to an explosion in runtime, so for now just compare + // the expr itself and the first child of each argument. This covers the common &variable case. + exists(int i, int j | + i = this.getTarget().(IrqlRestoreAnnotatedFunction).getIrqlParameterSlot() and + j = sgic.getTarget().(IrqlSavesToParameterFunction).getIrqlParameterSlot() and + exprsMatchText(this.getArgument(i), sgic.getArgument(j)) + ) + or + exists(int i | + i = this.getTarget().(IrqlRestoreAnnotatedFunction).getIrqlParameterSlot() and + sgic.getTarget() instanceof IrqlSavesViaReturnFunction and + exprsMatchText(this.getArgument(i), sgic.getSavedValue()) + ) and + this.getControlFlowScope() = sgic.getControlFlowScope() } } -//Evaluates to true if a FunctionCall at some points calls Irql annotated Function in its call hierarchy -predicate containsIrqlCall(FunctionCall fc) { - exists(Function fc2 | - fc.getTarget().calls*(fc2) and - fc2 instanceof IrqlAnnotatedFunction - ) +/** A call to a function that has is annotated \_IRQL\_saves\_. */ +class IrqlSaveCall extends FunctionCall { + IrqlSaveCall() { this.getTarget() instanceof IrqlSavesFunction } + + Expr getSavedValue() { + result = + any(Expr e | + exists(AssignExpr ae | + ae.getLValue() = e and + ae.getRValue() = this and + this.getTarget() instanceof IrqlSavesViaReturnFunction + ) + ) + } } -//Returns functions in the ControlFlow path that are instance of IrqlAnnotatedFunction -IrqlAnnotatedFunction getActualIrqlFunc(FunctionCall fc) { - exists(Function fc2 | - fc.getTarget().calls*(fc2) and - fc2 instanceof IrqlAnnotatedFunction and - result = fc2 +/** A call to a KeRaiseIRQL API that directly raises the IRQL. */ +class KeRaiseIrqlCall extends FunctionCall { + KeRaiseIrqlCall() { + this.getTarget() + .getName() + .matches(any([ + "KeRaiseIrql", "KfRaiseIrql", "KeRaiseIrqlToDPCLevel", "KfRaiseIrqlToDPCLevel" + ] + )) + } + + int getIrqlLevel() { + if this.getTarget().getName().matches(any(["KeRaiseIrqlToDPCLevel", "KfRaiseIrqlToDPCLevel"])) + then result = 2 + else result = this.getArgument(0).(Literal).getValue().toInt() + } +} + +/** A direct call to a function that lowers the IRQL. */ +class KeLowerIrqlCall extends FunctionCall { + KeLowerIrqlCall() { this.getTarget().getName().matches(any(["KeLowerIrql", "KfLowerIrql"])) } + + /** + * A heuristic evaluation of the IRQL that the system is lowering to. This is defined as + * "the IRQL before the most recent KeRaiseIrql call". + */ + int getIrqlLevel() { + result = any(getPotentialExitIrqlAtCfn(this.getMostRecentRaise().getAPredecessor())) + } + + /** + * Get the most recent call before this call that explicitly raised the IRQL. + */ + KeRaiseIrqlCall getMostRecentRaise() { + result = + any(KeRaiseIrqlCall sgic | + this.getAPredecessor*() = sgic and + not exists(KeRaiseIrqlCall kric2 | kric2 != sgic and kric2.getAPredecessor*() = sgic) + ) + } +} + +/** A call to a function that restores the IRQL from a specified state. */ +class SavesGlobalIrqlCall extends FunctionCall { + SavesGlobalIrqlCall() { this.getTarget() instanceof IrqlSavesGlobalAnnotatedFunction } +} + +/** A call to a function that restores the IRQL from a specified state. */ +class RestoresGlobalIrqlCall extends FunctionCall { + RestoresGlobalIrqlCall() { this.getTarget() instanceof IrqlRestoresGlobalAnnotatedFunction } + + /** + * A heuristic evaluation of the IRQL that the system is changing to. This is defined as + * "the IRQL before the corresponding save global call." + */ + int getIrqlLevel() { + result = any(getPotentialExitIrqlAtCfn(this.getMostRecentRaise().getAPredecessor())) + } + + /** Returns the matching call to a function that saved the IRQL to a global state. */ + SavesGlobalIrqlCall getMostRecentRaise() { + result = + any(SavesGlobalIrqlCall sgic | + this.getAPredecessor*() = sgic and + matchingSaveCall(sgic) and + not exists(SavesGlobalIrqlCall sgic2 | + sgic2 != sgic and sgic2.getAPredecessor*() = sgic and matchingSaveCall(sgic2) + ) + ) + } + + /** + * Holds if a given call to an _IRQL_saves_global_ annotated function is using the same IRQL location as this. + */ + private predicate matchingSaveCall(SavesGlobalIrqlCall sgic) { + // Attempting to match all expr children leads to an explosion in runtime, so for now just compare + // the expr itself and the first child of each argument. This covers the common &variable case. + exists(int i, int j | + i = this.getTarget().(IrqlRestoresGlobalAnnotatedFunction).getIrqlParameterSlot() and + j = sgic.getTarget().(IrqlSavesGlobalAnnotatedFunction).getIrqlParameterSlot() and + exprsMatchText(this.getArgument(i), sgic.getArgument(j)) + ) and + this.getTarget().(IrqlRestoresGlobalAnnotatedFunction).getIrqlKind() = + sgic.getTarget().(IrqlSavesGlobalAnnotatedFunction).getIrqlKind() + } +} + +/** + * A utility function to determine if two exprs are (textually) the same. + * Because checking all children of the expression causes an explosion in evaluation time, we just + * check the first child. + * + * This function is obviously _not_ a guarantee that two expressions refer to the same thing. + * Use this locally and with caution. + */ +pragma[inline] +private predicate exprsMatchText(Expr e1, Expr e2) { + e1.toString().matches(e2.toString()) and + exists(Expr child | + child = e1.getAChild() and + e1.getChild(0).toString().matches(e2.getChild(0).toString()) ) + or + not exists(Expr child | child = e1.getAChild() or child = e2.getAChild()) } -class CallsToIrqlAnnotatedFunction extends FunctionCall { - CallsToIrqlAnnotatedFunction() { containsIrqlCall(this) } +/** + * Attempt to provide the IRQL **once this control flow node exits**, based on annotations and direct calls to raising/lowering functions. + * This predicate functions as follows: + * - If calling a "Raise IRQL" function, then it returns the value of the argument passed in (the target IRQL). + * - If calling a "Lower IRQL" function, then it returns the value of the argument passed in (the target IRQL). + * - If calling a function annotated to restore the IRQL from a previously saved spot, then the result is the IRQL before that save call. + * - If calling a function annotated to raise the IRQL, then it returns the annotated value (the target IRQL). + * - If calling a function annotated to maintain the same IRQL, then the result is the IRQL at the previous CFN. + * - If this node is calling a function with no annotations, the result is the IRQL that function exits at. + * - If there is a prior CFN in the CFG, the result is the result for that prior CFN. + * - If there is no prior CFN, then the result is whatever the IRQL was at a statement prior to a function call to this function. + * - If there are no prior CFNs and no calls to this function, then the IRQL is determined by annotations. + * - If there is nothing else, then IRQL is 0. + * + * Not implemented: _IRQL_limited_to_ + */ +cached +int getPotentialExitIrqlAtCfn(ControlFlowNode cfn) { + if cfn instanceof KeRaiseIrqlCall + then result = cfn.(KeRaiseIrqlCall).getIrqlLevel() + else + if cfn instanceof KeLowerIrqlCall + then result = cfn.(KeLowerIrqlCall).getIrqlLevel() + else + if cfn instanceof RestoresGlobalIrqlCall + then result = cfn.(RestoresGlobalIrqlCall).getIrqlLevel() + else + if cfn instanceof IrqlRestoreCall + then result = cfn.(IrqlRestoreCall).getIrqlLevel() + else + if + cfn instanceof FunctionCall and + cfn.(FunctionCall).getTarget() instanceof IrqlRaisesAnnotatedFunction + then result = cfn.(FunctionCall).getTarget().(IrqlRaisesAnnotatedFunction).getIrqlLevel() + else + if + cfn instanceof FunctionCall and + cfn.(FunctionCall).getTarget() instanceof IrqlRequiresSameAnnotatedFunction + then result = any(getPotentialExitIrqlAtCfn(cfn.getAPredecessor())) + else + if cfn instanceof FunctionCall + then + result = + any(getPotentialExitIrqlAtCfn(getExitPointsOfFunction(cfn.(FunctionCall) + .getTarget())) + ) + else + if exists(ControlFlowNode cfn2 | cfn2 = cfn.getAPredecessor()) + then result = any(getPotentialExitIrqlAtCfn(cfn.getAPredecessor())) + else + if + exists(FunctionCall fc, ControlFlowNode cfn2 | + fc.getTarget() = cfn.getControlFlowScope() and + cfn2.getASuccessor() = fc + ) + then + result = + any(getPotentialExitIrqlAtCfn(any(ControlFlowNode cfn2 | + cfn2.getASuccessor().(FunctionCall).getTarget() = + cfn.getControlFlowScope() + )) + ) + else + if + cfn.getControlFlowScope() instanceof IrqlRestrictsFunction and + getAllowableIrqlLevel(cfn.getControlFlowScope()) != -1 + then result = getAllowableIrqlLevel(cfn.getControlFlowScope()) + else result = 0 +} + +import semmle.code.cpp.controlflow.Dominance + +/** Utility function to get exit points of a function. */ +private ControlFlowNode getExitPointsOfFunction(Function f) { + result = + any(ControlFlowNode cfn | + cfn.getControlFlowScope() = f and + functionExit(cfn) + ) +} + +/** + * Attempt to find the range of valid IRQL values when **entering** a given IRQL-annotated function. + */ +cached +int getAllowableIrqlLevel(IrqlRestrictsFunction irqlFunc) { + if irqlFunc instanceof IrqlRequiresAnnotatedFunction + then result = irqlFunc.(IrqlRequiresAnnotatedFunction).getIrqlLevel() + else + if + irqlFunc instanceof IrqlMaxAnnotatedFunction and + irqlFunc instanceof IrqlMinAnnotatedFunction + then + result = + any([irqlFunc.(IrqlMinAnnotatedFunction).getIrqlLevel() .. irqlFunc + .(IrqlMaxAnnotatedFunction) + .getIrqlLevel()] + ) + else + if irqlFunc instanceof IrqlMaxAnnotatedFunction + then result = any([0 .. irqlFunc.(IrqlMaxAnnotatedFunction).getIrqlLevel()]) + else + if irqlFunc instanceof IrqlMinAnnotatedFunction + then + result = + any([irqlFunc.(IrqlMinAnnotatedFunction).getIrqlLevel() .. any(IrqlMacro im) + .getGlobalMaxIrqlLevel()] + ) + else + // No known restriction + result = any([0 .. any(IrqlMacro im).getGlobalMaxIrqlLevel()]) } diff --git a/src/drivers/libraries/SAL.qll b/src/drivers/libraries/SAL.qll index 64442c78..8c0d704c 100644 --- a/src/drivers/libraries/SAL.qll +++ b/src/drivers/libraries/SAL.qll @@ -49,6 +49,11 @@ class SALAnnotation extends MacroInvocation { not result instanceof Type // exclude typedefs } + Declaration getTypedefDeclarations() { + annotatesAt(this, result.getADeclarationEntry(), _, _) and + result instanceof Type // include + } + /** Gets the `DeclarationEntry` annotated by `this`. */ DeclarationEntry getDeclarationEntry() { annotatesAt(this, result, _, _) and diff --git a/src/drivers/test/build_create_analyze_test.cmd b/src/drivers/test/build_create_analyze_test.cmd index 9948923d..e471e9c7 100644 --- a/src/drivers/test/build_create_analyze_test.cmd +++ b/src/drivers/test/build_create_analyze_test.cmd @@ -7,8 +7,8 @@ call :test NoPagingSegment WDMTestTemplate wdm queries call :test OpaqueMdlUse WDMTestTemplate wdm queries call :test OpaqueMdlWrite WDMTestTemplate wdm queries call :test KeWaitLocal WDMTestTemplate wdm queries -call :test IrqTooHigh WDMTestTemplate wdm queries\experimental -call :test IrqTooLow WDMTestTemplate wdm queries\experimental +call :test IrqlTooHigh WDMTestTemplate general queries\experimental +call :test IrqlTooLow WDMTestTemplate general queries\experimental call :test WrongDispatchTableAssignment WDMTestTemplate wdm queries call :test ExtendedDeprecatedApis WDMTestTemplate general queries call :test WdkDeprecatedApis WDMTestTemplate general queries diff --git a/src/drivers/test/diff/IrqlTooHigh.sarif b/src/drivers/test/diff/IrqlTooHigh.sarif new file mode 100644 index 00000000..dea8275e --- /dev/null +++ b/src/drivers/test/diff/IrqlTooHigh.sarif @@ -0,0 +1,21 @@ +{ + "all": { + "+": 0, + "-": 0 + }, + "error": { + "+": 0, + "-": 0, + "codes": [] + }, + "warning": { + "+": 0, + "-": 0, + "codes": [] + }, + "note": { + "+": 0, + "-": 0, + "codes": [] + } +} \ No newline at end of file diff --git a/src/drivers/test/diff/IrqlTooLow.sarif b/src/drivers/test/diff/IrqlTooLow.sarif new file mode 100644 index 00000000..dea8275e --- /dev/null +++ b/src/drivers/test/diff/IrqlTooLow.sarif @@ -0,0 +1,21 @@ +{ + "all": { + "+": 0, + "-": 0 + }, + "error": { + "+": 0, + "-": 0, + "codes": [] + }, + "warning": { + "+": 0, + "-": 0, + "codes": [] + }, + "note": { + "+": 0, + "-": 0, + "codes": [] + } +} \ No newline at end of file diff --git a/src/drivers/wdm/libraries/WdmDrivers.qll b/src/drivers/wdm/libraries/WdmDrivers.qll index 173bb19c..3dbd57f6 100644 --- a/src/drivers/wdm/libraries/WdmDrivers.qll +++ b/src/drivers/wdm/libraries/WdmDrivers.qll @@ -48,6 +48,8 @@ class WdmCallbackRoutineTypedef extends TypedefType { or this.getName().matches("DRIVER_INITIALIZE") or + this.getName().matches("DRIVER_CANCEL") + or this.getName().matches("IO_COMPLETION_ROUTINE") or this.getName().matches("KSERVICE_ROUTINE") @@ -93,6 +95,11 @@ class WdmDriverEntry extends WdmCallbackRoutine { WdmDriverEntry() { callbackType.getName().matches("DRIVER_INITIALIZE") } } +/** A WDM DriverCancel callback routine. */ +class WdmDriverCancel extends WdmCallbackRoutine { + WdmDriverCancel() { callbackType.getName().matches("DRIVER_CANCEL")} +} + /** * WDM dispatch routine class. * We hold a routine to be a dispatch routine if there is diff --git a/src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.ql b/src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.ql deleted file mode 100644 index 0f57ca5c..00000000 --- a/src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.ql +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. -/** - * @name IrqTooHigh - * @kind problem - * @description The function is not permitted to be called at the current IRQ level. The current level is too high. - * @problem.severity error - * @id cpp/portedqueries/irq-too-high - * @platform Desktop - * @repro.text The following line(s) potentially contains a function call that is supposed to be called at a higher Irql level. - * @feature.area Multiple - * @version 1.0 - */ - -import cpp -import drivers.libraries.Irql - -//Evaluates to true if KeLowerIrql call is made before a call to IrqlAnnotatedFunction is made -predicate preceedingKeLowerIrqlCall(CallsToIrqlAnnotatedFunction iafc) { - exists(FunctionCall fc2, Function fc3 | - iafc.getAPredecessor*() = fc2 and - ( - fc2.getTarget().getName() = "KeLowerIrql" - or - fc2.getTarget().calls*(fc3) and fc3.getName() = "KeLowerIrql" - ) - ) -} - -// Evaluates to true for FunctionCalls inside IrqlAnnotatedFunction where the call requires a lower Irql level -predicate irqlAnnotationViolatingCall(CallsToIrqlAnnotatedFunction ciaf) { - exists(IrqlAnnotatedFunction iaf, int current, int called | - ciaf.getEnclosingFunction() = iaf and - iaf.getIrqlLevel() = current and - getActualIrqlFunc(ciaf).getIrqlLevel() = called and - current > called - ) -} - -//Evaluates to true if there is a call from KeRaiseIrql to a IrqlAnnotatedFunction before any KeLowerIrql call in between. -predicate irqlNotLoweredCall(CallsToIrqlAnnotatedFunction fc) { - exists(FunctionCall kr, int called, int current | - kr.getASuccessor*() = fc and - getActualIrqlFunc(fc).getIrqlLevel() = called and - ( - kr.getTarget().getName().matches("KfRaiseIrql") and - kr.getArgument(0).getValue().toInt() = current - or - kr.getTarget().getName().matches("KeRaiseIrqlToDpcLevel") and - 2 = current - ) and - called < current and - not preceedingKeLowerIrqlCall(fc) - ) -} - -from CallsToIrqlAnnotatedFunction ciaf -where irqlNotLoweredCall(ciaf) or irqlAnnotationViolatingCall(ciaf) -select ciaf, - ciaf.getTarget().getName() + - " can not be called at the current Irql level. The current level is too high" diff --git a/src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.sarif b/src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.sarif deleted file mode 100644 index 622dbeb8..00000000 --- a/src/drivers/wdm/queries/experimental/IrqTooHigh/IrqTooHigh.sarif +++ /dev/null @@ -1,169 +0,0 @@ -{ - "$schema" : "https://json.schemastore.org/sarif-2.1.0.json", - "version" : "2.1.0", - "runs" : [ { - "tool" : { - "driver" : { - "name" : "CodeQL", - "organization" : "GitHub", - "semanticVersion" : "2.11.5", - "rules" : [ { - "id" : "cpp/portedqueries/irq-too-high", - "name" : "cpp/portedqueries/irq-too-high", - "shortDescription" : { - "text" : "IrqTooHigh" - }, - "fullDescription" : { - "text" : "The function is not permitted to be called at the current IRQ level. The current level is too high." - }, - "defaultConfiguration" : { - "enabled" : true, - "level" : "error" - }, - "properties" : { - "description" : "The function is not permitted to be called at the current IRQ level. The current level is too high.", - "feature.area" : "Multiple", - "id" : "cpp/portedqueries/irq-too-high", - "kind" : "problem", - "name" : "IrqTooHigh", - "platform" : "Desktop", - "problem.severity" : "error", - "repro.text" : "The following line(s) potentially contains a function call that is supposed to be called at a higher Irql level.", - "version" : "1.0" - } - } ] - }, - "extensions" : [ { - "name" : "microsoft/windows-drivers", - "semanticVersion" : "0.1.0+933e876f096a70922173e4d5ad604d99d4481af4", - "locations" : [ { - "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", - "description" : { - "text" : "The QL pack root directory." - } - }, { - "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/qlpack.yml", - "description" : { - "text" : "The QL pack definition file." - } - } ] - }, { - "name" : "legacy-upgrades", - "semanticVersion" : "0.0.0", - "locations" : [ { - "uri" : "file:///C:/codeql-home/codeql/legacy-upgrades/", - "description" : { - "text" : "The QL pack root directory." - } - }, { - "uri" : "file:///C:/codeql-home/codeql/legacy-upgrades/qlpack.yml", - "description" : { - "text" : "The QL pack definition file." - } - } ] - } ] - }, - "artifacts" : [ { - "location" : { - "uri" : "driver/driver_snippet.c", - "uriBaseId" : "%SRCROOT%", - "index" : 0 - } - }, { - "location" : { - "uri" : "driver/fail_driver1.c", - "uriBaseId" : "%SRCROOT%", - "index" : 1 - } - } ], - "results" : [ { - "ruleId" : "cpp/portedqueries/irq-too-high", - "ruleIndex" : 0, - "rule" : { - "id" : "cpp/portedqueries/irq-too-high", - "index" : 0 - }, - "message" : { - "text" : "IrqlLowTestFunction can not be called at the current Irql level. The current level is too high" - }, - "locations" : [ { - "physicalLocation" : { - "artifactLocation" : { - "uri" : "driver/driver_snippet.c", - "uriBaseId" : "%SRCROOT%", - "index" : 0 - }, - "region" : { - "startLine" : 63, - "startColumn" : 5, - "endColumn" : 24 - } - } - } ], - "partialFingerprints" : { - "primaryLocationLineHash" : "1b74530608bc3ddf:1", - "primaryLocationStartColumnFingerprint" : "0" - } - }, { - "ruleId" : "cpp/portedqueries/irq-too-high", - "ruleIndex" : 0, - "rule" : { - "id" : "cpp/portedqueries/irq-too-high", - "index" : 0 - }, - "message" : { - "text" : "TestInner4 can not be called at the current Irql level. The current level is too high" - }, - "locations" : [ { - "physicalLocation" : { - "artifactLocation" : { - "uri" : "driver/driver_snippet.c", - "uriBaseId" : "%SRCROOT%", - "index" : 0 - }, - "region" : { - "startLine" : 36, - "startColumn" : 14, - "endColumn" : 24 - } - } - } ], - "partialFingerprints" : { - "primaryLocationLineHash" : "7ae2af586e0dd70a:1", - "primaryLocationStartColumnFingerprint" : "9" - } - }, { - "ruleId" : "cpp/portedqueries/irq-too-high", - "ruleIndex" : 0, - "rule" : { - "id" : "cpp/portedqueries/irq-too-high", - "index" : 0 - }, - "message" : { - "text" : "failForIrqlTooHigh can not be called at the current Irql level. The current level is too high" - }, - "locations" : [ { - "physicalLocation" : { - "artifactLocation" : { - "uri" : "driver/fail_driver1.c", - "uriBaseId" : "%SRCROOT%", - "index" : 1 - }, - "region" : { - "startLine" : 322, - "startColumn" : 5, - "endColumn" : 23 - } - } - } ], - "partialFingerprints" : { - "primaryLocationLineHash" : "251f68b02b7c2bea:1", - "primaryLocationStartColumnFingerprint" : "0" - } - } ], - "columnKind" : "utf16CodeUnits", - "properties" : { - "semmle.formatSpecifier" : "sarifv2.1.0" - } - } ] -} \ No newline at end of file diff --git a/src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.ql b/src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.ql deleted file mode 100644 index 5fda52f7..00000000 --- a/src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.ql +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. -/** - * @name IrqTooLow - * @kind problem - * @description The function is not permitted to be called at the current IRQ level. The current level is too low. - * @problem.severity error - * @id cpp/portedqueries/irq-too-low - * @platform Desktop - * @repro.text The following line(s) potentially contains a function call that is supposed to be called at a lower Irql level. - * @feature.area Multiple - * @version 1.0 - */ - -import cpp -import drivers.libraries.Irql - -from IrqlAnnotatedFunction iaf, CallsToIrqlAnnotatedFunction ciaf, int curr, int called -where - ciaf.getEnclosingFunction() = iaf and - iaf.getIrqlLevel() = curr and - getActualIrqlFunc(ciaf).getIrqlLevel() = called and - curr < called -select ciaf, - "Irql was set to " + curr + " in " + iaf.getName() + " but " + ciaf.getTarget().getName() + - "() in its call hierarchy requires " + called + " Irql level." diff --git a/src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.sarif b/src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.sarif deleted file mode 100644 index 0e3b8d32..00000000 --- a/src/drivers/wdm/queries/experimental/IrqTooLow/IrqTooLow.sarif +++ /dev/null @@ -1,107 +0,0 @@ -{ - "$schema" : "https://json.schemastore.org/sarif-2.1.0.json", - "version" : "2.1.0", - "runs" : [ { - "tool" : { - "driver" : { - "name" : "CodeQL", - "organization" : "GitHub", - "semanticVersion" : "2.11.5", - "rules" : [ { - "id" : "cpp/portedqueries/irq-too-low", - "name" : "cpp/portedqueries/irq-too-low", - "shortDescription" : { - "text" : "IrqTooLow" - }, - "fullDescription" : { - "text" : "The function is not permitted to be called at the current IRQ level. The current level is too low." - }, - "defaultConfiguration" : { - "enabled" : true, - "level" : "error" - }, - "properties" : { - "description" : "The function is not permitted to be called at the current IRQ level. The current level is too low.", - "feature.area" : "Multiple", - "id" : "cpp/portedqueries/irq-too-low", - "kind" : "problem", - "name" : "IrqTooLow", - "platform" : "Desktop", - "problem.severity" : "error", - "repro.text" : "The following line(s) potentially contains a function call that is supposed to be called at a lower Irql level.", - "version" : "1.0" - } - } ] - }, - "extensions" : [ { - "name" : "microsoft/windows-drivers", - "semanticVersion" : "0.1.0+933e876f096a70922173e4d5ad604d99d4481af4", - "locations" : [ { - "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", - "description" : { - "text" : "The QL pack root directory." - } - }, { - "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/qlpack.yml", - "description" : { - "text" : "The QL pack definition file." - } - } ] - }, { - "name" : "legacy-upgrades", - "semanticVersion" : "0.0.0", - "locations" : [ { - "uri" : "file:///C:/codeql-home/codeql/legacy-upgrades/", - "description" : { - "text" : "The QL pack root directory." - } - }, { - "uri" : "file:///C:/codeql-home/codeql/legacy-upgrades/qlpack.yml", - "description" : { - "text" : "The QL pack definition file." - } - } ] - } ] - }, - "artifacts" : [ { - "location" : { - "uri" : "driver/driver_snippet.c", - "uriBaseId" : "%SRCROOT%", - "index" : 0 - } - } ], - "results" : [ { - "ruleId" : "cpp/portedqueries/irq-too-low", - "ruleIndex" : 0, - "rule" : { - "id" : "cpp/portedqueries/irq-too-low", - "index" : 0 - }, - "message" : { - "text" : "Irql was set to 1 in TestInner2 but someFunc() in its call hierarchy requires 2 Irql level." - }, - "locations" : [ { - "physicalLocation" : { - "artifactLocation" : { - "uri" : "driver/driver_snippet.c", - "uriBaseId" : "%SRCROOT%", - "index" : 0 - }, - "region" : { - "startLine" : 31, - "startColumn" : 14, - "endColumn" : 22 - } - } - } ], - "partialFingerprints" : { - "primaryLocationLineHash" : "ad7afdefc432f9c8:1", - "primaryLocationStartColumnFingerprint" : "9" - } - } ], - "columnKind" : "utf16CodeUnits", - "properties" : { - "semmle.formatSpecifier" : "sarifv2.1.0" - } - } ] -} \ No newline at end of file diff --git a/src/suites/ported_driver_ca_checks.qls b/src/suites/ported_driver_ca_checks.qls index e417ffc2..c4a43c99 100644 --- a/src/suites/ported_driver_ca_checks.qls +++ b/src/suites/ported_driver_ca_checks.qls @@ -13,8 +13,8 @@ - drivers/general/queries/PoolTagIntegral/PoolTagIntegral.ql - drivers/general/queries/WdkDeprecatedApis/wdk-deprecated-api.ql - drivers/kmdf/queries/StrSafe/StrSafe.ql - - drivers/wdm/experimental/IrqTooHigh/IrqTooHigh.ql - - drivers/wdm/experimental/IrqTooLow/IrqTooLow.ql + - drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.ql + - drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.ql - drivers/wdm/queries/ExaminedValue/ExaminedValue.ql - drivers/wdm/queries/IllegalFieldAccess/IllegalFieldAccess.ql - drivers/wdm/queries/IllegalFieldAccess2/IllegalFieldAccess2.ql From 1afd4c012a3d64eecf03ff5038170a79f9a45d18 Mon Sep 17 00:00:00 2001 From: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> Date: Wed, 25 Oct 2023 13:39:42 -0700 Subject: [PATCH 04/11] Port C28127 to CodeQL query (#87) * codeql port of code analysis rule C28127 --------- Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> --- .../RoutineFunctionTypeNotExpected.ql | 38 +++ .../RoutineFunctionTypeNotExpected.qlhelp | 23 ++ .../RoutineFunctionTypeNotExpected.sarif | 313 ++++++++++++++++++ .../driver_snippet.c | 62 ++++ .../test/build_create_analyze_test.cmd | 1 + 5 files changed, 437 insertions(+) create mode 100644 src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.ql create mode 100644 src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.qlhelp create mode 100644 src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.sarif create mode 100644 src/drivers/general/queries/RoutineFunctionTypeNotExpected/driver_snippet.c diff --git a/src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.ql b/src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.ql new file mode 100644 index 00000000..84292a4a --- /dev/null +++ b/src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.ql @@ -0,0 +1,38 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +/** + * @id cpp/drivers/routine-function-type-not-expected + * @kind problem + * @name Unexpected function return type for routine (C28127) + * @description The function being used as a routine does not exactly match the type expected. + * @platform Desktop + * @security.severity Low + * @feature.area Multiple + * @impact Attack Surface Reduction + * @repro.text The following code locations use a function pointer with a return type that does not match the expected type + * @owner.email sdat@microsoft.com + * @opaqueid CQLD-C28127 + * @problem.severity warning + * @precision high + * @tags correctness + * wddst + * @scope domainspecific + * @query-version v1 + */ + +import cpp + +from FunctionCall fc, Parameter p, int n +where + p.getFunction() = fc.getTarget() and + p.getUnspecifiedType() instanceof FunctionPointerType and + p.getIndex() = n and + fc.getArgument(n).getUnspecifiedType() instanceof FunctionPointerType and + fc.getArgument(n).getUnspecifiedType().(FunctionPointerType).getReturnType().getUnspecifiedType() != + p.getUnspecifiedType().(FunctionPointerType).getReturnType().getUnspecifiedType() + +select fc, + "Function " + fc + " may use a function pointer (" + fc.getArgument(n) + + ") with an unexpected return type: " + + fc.getArgument(n).getUnspecifiedType().(FunctionPointerType).getReturnType() + " expected: " + + p.getUnspecifiedType().(FunctionPointerType).getReturnType() diff --git a/src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.qlhelp b/src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.qlhelp new file mode 100644 index 00000000..1bafd5bb --- /dev/null +++ b/src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.qlhelp @@ -0,0 +1,23 @@ + + + +

+ The return type of a function pointer used in a function call should match the declaration of the calling function +

+ + +

+ Verify function pointer is correct +

+ + + + + +
  • + + C28127 warning - Windows Drivers + +
    • +     +     diff --git a/src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.sarif b/src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.sarif new file mode 100644 index 00000000..08476746 --- /dev/null +++ b/src/drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.sarif @@ -0,0 +1,313 @@ +{ + "$schema" : "https://json.schemastore.org/sarif-2.1.0.json", + "version" : "2.1.0", + "runs" : [ { + "tool" : { + "driver" : { + "name" : "CodeQL", + "organization" : "GitHub", + "semanticVersion" : "2.14.6", + "notifications" : [ { + "id" : "cpp/baseline/expected-extracted-files", + "name" : "cpp/baseline/expected-extracted-files", + "shortDescription" : { + "text" : "Expected extracted files" + }, + "fullDescription" : { + "text" : "Files appearing in the source archive that are expected to be extracted." + }, + "defaultConfiguration" : { + "enabled" : true + }, + "properties" : { + "tags" : [ "expected-extracted-files", "telemetry" ] + } + } ], + "rules" : [ { + "id" : "cpp/drivers/routine-function-type-not-expected", + "name" : "cpp/drivers/routine-function-type-not-expected", + "shortDescription" : { + "text" : "Unexpected function return type for routine (C28127)" + }, + "fullDescription" : { + "text" : "The function being used as a routine does not exactly match the type expected." + }, + "defaultConfiguration" : { + "enabled" : true, + "level" : "warning" + }, + "properties" : { + "tags" : [ "correctness", "wddst" ], + "description" : "The function being used as a routine does not exactly match the type expected.", + "feature.area" : "Multiple", + "id" : "cpp/drivers/routine-function-type-not-expected", + "impact" : "Attack Surface Reduction", + "kind" : "problem", + "name" : "Unexpected function return type for routine (C28127)", + "opaqueid" : "CQLD-C28127", + "owner.email" : "sdat@microsoft.com", + "platform" : "Desktop", + "precision" : "high", + "problem.severity" : "warning", + "query-version" : "v1", + "repro.text" : "The following code locations use a function pointer with a return type that does not match the expected type", + "scope" : "domainspecific", + "security.severity" : "Low" + } + } ] + }, + "extensions" : [ { + "name" : "microsoft/windows-drivers", + "semanticVersion" : "0.1.0+626ab2156fae247d66b189fb2fa9a69c03082e3a", + "locations" : [ { + "uri" : "file:///c:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", + "description" : { + "text" : "The QL pack root directory." + } + }, { + "uri" : "file:///c:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/qlpack.yml", + "description" : { + "text" : "The QL pack definition file." + } + } ] + } ] + }, + "invocations" : [ { + "toolExecutionNotifications" : [ { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + } ], + "executionSuccessful" : true + } ], + "artifacts" : [ { + "location" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } ], + "results" : [ { + "ruleId" : "cpp/drivers/routine-function-type-not-expected", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/routine-function-type-not-expected", + "index" : 0 + }, + "message" : { + "text" : "Function call to functionCallThatUsesFunctionPointer may use a function pointer (fun_ptr1) with an unexpected return type: int expected: void" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 52, + "startColumn" : 5, + "endColumn" : 40 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "b6c3b797b0277bdd:1", + "primaryLocationStartColumnFingerprint" : "0" + } + }, { + "ruleId" : "cpp/drivers/routine-function-type-not-expected", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/routine-function-type-not-expected", + "index" : 0 + }, + "message" : { + "text" : "Function call to functionCallThatUsesFunctionPointer may use a function pointer (f3) with an unexpected return type: int expected: void" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 54, + "startColumn" : 5, + "endColumn" : 40 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "789a3ee1dd677a33:1", + "primaryLocationStartColumnFingerprint" : "0" + } + }, { + "ruleId" : "cpp/drivers/routine-function-type-not-expected", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/routine-function-type-not-expected", + "index" : 0 + }, + "message" : { + "text" : "Function call to functionCallThatUsesFunctionPointer may use a function pointer (& ...) with an unexpected return type: int expected: void" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 56, + "startColumn" : 5, + "endColumn" : 40 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "b16f3331cfb3f2dd:1", + "primaryLocationStartColumnFingerprint" : "0" + } + }, { + "ruleId" : "cpp/drivers/routine-function-type-not-expected", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/routine-function-type-not-expected", + "index" : 0 + }, + "message" : { + "text" : "Function call to functionCallThatUsesFunctionPointer may use a function pointer (intFunctionToCall) with an unexpected return type: int expected: void" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 59, + "startColumn" : 5, + "endColumn" : 40 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "8a5840d23f7ece01:1", + "primaryLocationStartColumnFingerprint" : "0" + } + }, { + "ruleId" : "cpp/drivers/routine-function-type-not-expected", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/routine-function-type-not-expected", + "index" : 0 + }, + "message" : { + "text" : "Function call to functionCallThatUsesFunctionPointer2 may use a function pointer (intFunctionToCall) with an unexpected return type: int expected: void" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 61, + "startColumn" : 5, + "endColumn" : 41 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "b2e192116459028c:1", + "primaryLocationStartColumnFingerprint" : "0" + } + } ], + "columnKind" : "utf16CodeUnits", + "properties" : { + "semmle.formatSpecifier" : "sarifv2.1.0" + } + } ] +} \ No newline at end of file diff --git a/src/drivers/general/queries/RoutineFunctionTypeNotExpected/driver_snippet.c b/src/drivers/general/queries/RoutineFunctionTypeNotExpected/driver_snippet.c new file mode 100644 index 00000000..be558b52 --- /dev/null +++ b/src/drivers/general/queries/RoutineFunctionTypeNotExpected/driver_snippet.c @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +// driver_snippet.c +// + +#define SET_DISPATCH 1 + +// Template. Not called in this test. +void top_level_call() {} + +typedef void +functionCall1(void); + +typedef functionCall1 *funcCall; + +typedef int +functionCallInt(void); + +typedef functionCallInt *funcCallInt; + +void voidFunctionToCall(void) +{ + return; +} +int intFunctionToCall(void) +{ + return 0; +} + +int (*fun_ptr1)(void) = intFunctionToCall; +void (*fun_ptr2)(void) = voidFunctionToCall; + +char functionCallThatUsesFunctionPointer(funcCall functionPointer) +{ + functionPointer(); + return 'a'; +} +char functionCallThatUsesFunctionPointer2(char a, funcCall functionPointer, char b) +{ + functionPointer(); + return a + b; +} +void callFunctionCallThatUsesFunctionPointer(void) +{ + funcCall f1 = &voidFunctionToCall; + funcCall f2 = &intFunctionToCall; // funcCall type specifies a void return type, but this is a function pointer to intFunctionToCall which returns an int + funcCallInt f3 = &intFunctionToCall; + functionCallThatUsesFunctionPointer(f1); // pass + functionCallThatUsesFunctionPointer(fun_ptr2); // pass + + functionCallThatUsesFunctionPointer(fun_ptr1); // fail because fun_ptr1 is a function pointer to intFunctionToCall which returns an int + functionCallThatUsesFunctionPointer(f2); // This SHOULD fail because f2 is a function pointer to intFunctionToCall which returns an int, but it is declared with a funcCall type which specifies a void return type so it passes + functionCallThatUsesFunctionPointer(f3); // fail because f3 is a function pointer to intFunctionToCall which returns an int + functionCallThatUsesFunctionPointer(&voidFunctionToCall); // pass + functionCallThatUsesFunctionPointer(&intFunctionToCall); // fail because this intFunctionToCall returns an int + + functionCallThatUsesFunctionPointer(voidFunctionToCall); // pass + functionCallThatUsesFunctionPointer(intFunctionToCall); // fail because this intFunctionToCall returns an int + + functionCallThatUsesFunctionPointer2('a', intFunctionToCall, 'b'); // fail because this intFunctionToCall returns an int +} diff --git a/src/drivers/test/build_create_analyze_test.cmd b/src/drivers/test/build_create_analyze_test.cmd index e471e9c7..9859d885 100644 --- a/src/drivers/test/build_create_analyze_test.cmd +++ b/src/drivers/test/build_create_analyze_test.cmd @@ -23,6 +23,7 @@ call :test IrqlNotUsed WDMTestTemplate general queries call :test IrqlNotSaved WDMTestTemplate general queries call :test IllegalFieldWrite WDMTestTemplate wdm queries call :test IllegalFieldAccess2 WDMTestTemplate wdm queries +call :test RoutineFunctionTypeNotExpected WDMTestTemplate general queries exit /b 0 From 45e474f024fec29dd7c2ea496cc6ce787e199fd3 Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Thu, 26 Oct 2023 17:49:58 -0700 Subject: [PATCH 05/11] Update to CodeQL version v2.15.1. (#89) Update to CodeQL v2.15.1. --- .github/workflows/build-codeql.yaml | 2 +- README.md | 8 ++++---- src/codeql-pack.lock.yml | 16 +++++++++------- src/qlpack.yml | 4 ++-- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/.github/workflows/build-codeql.yaml b/.github/workflows/build-codeql.yaml index 3c358b1d..68aa6724 100644 --- a/.github/workflows/build-codeql.yaml +++ b/.github/workflows/build-codeql.yaml @@ -32,7 +32,7 @@ jobs: with: owner: "github" repo: "codeql-cli-binaries" - tag: "v2.14.4" + tag: "v2.15.1" file: "codeql-win64.zip" - name: Unzip CodeQL CLI diff --git a/README.md b/README.md index 891f27de..bd70a2b8 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ This repository contains open-source components for supplemental use in developi | Branch to use | CodeQL CLI version | |--------------------------|--------------------| -| main | 2.14.4 | +| main | 2.15.1 | ### For Windows Hardware Compatibility Program Use @@ -17,7 +17,7 @@ This repository contains open-source components for supplemental use in developi | Windows 11 | WHCP_21H2 | 2.4.6 | | Windows 11, version 22H2 | WHCP_22H2 | 2.6.3 | -For general use, use the `main` branch along with [version 2.14.4 of the CodeQL CLI](https://github.com/github/codeql-cli-binaries/releases/tag/v2.14.4). +For general use, use the `main` branch along with [version 2.15.1 of the CodeQL CLI](https://github.com/github/codeql-cli-binaries/releases/tag/v2.15.1). ## Quickstart @@ -30,7 +30,7 @@ For general use, use the `main` branch along with [version 2.14.4 of the CodeQL For the WHCP Program, use the CodeQL CLI version in accordance with the table above and Windows release you are certifying for: [version 2.4.6](https://github.com/github/codeql-cli-binaries/releases/tag/v2.4.6) or [version 2.6.3](https://github.com/github/codeql-cli-binaries/releases/tag/v2.6.3). - For general use with the `main` branch, use [CodeQL CLI version 2.14.4](https://github.com/github/codeql-cli-binaries/releases/tag/v2.14.4). + For general use with the `main` branch, use [CodeQL CLI version 2.15.1](https://github.com/github/codeql-cli-binaries/releases/tag/v2.15.1). 1. Clone and install the Windows Driver Developer Supplemental Tools repository which contains the CodeQL queries specific for drivers: @@ -56,7 +56,7 @@ For general use, use the `main` branch along with [version 2.14.4 of the CodeQL 1. Verify CodeQL is installed correctly by checking the version: ``` D:\codeql-home\codeql>codeql --version - CodeQL command-line toolchain release 2.14.4. + CodeQL command-line toolchain release 2.15.1. Copyright (C) 2019-2023 GitHub, Inc. Unpacked in: D:\codeql-home\codeql Analysis results depend critically on separately distributed query and diff --git a/src/codeql-pack.lock.yml b/src/codeql-pack.lock.yml index 2c012295..ba37d46e 100644 --- a/src/codeql-pack.lock.yml +++ b/src/codeql-pack.lock.yml @@ -2,17 +2,19 @@ lockVersion: 1.0.0 dependencies: codeql/cpp-all: - version: 0.9.2 + version: 0.10.1 codeql/cpp-queries: - version: 0.7.4 + version: 0.8.1 codeql/dataflow: - version: 0.0.3 + version: 0.1.1 codeql/ssa: - version: 0.1.4 + version: 0.2.1 codeql/suite-helpers: - version: 0.6.4 + version: 0.7.1 codeql/tutorial: - version: 0.1.4 + version: 0.2.1 + codeql/typetracking: + version: 0.2.1 codeql/util: - version: 0.1.4 + version: 0.2.1 compiled: false diff --git a/src/qlpack.yml b/src/qlpack.yml index 1d1a053c..cbe7887f 100644 --- a/src/qlpack.yml +++ b/src/qlpack.yml @@ -4,8 +4,8 @@ name: microsoft/windows-drivers version: 0.2.0 dependencies: - codeql/cpp-queries: 0.7.4 - codeql/cpp-all: 0.9.2 + codeql/cpp-queries: 0.8.1 + codeql/cpp-all: 0.10.1 suites: suites extractor: cpp licenses: MIT From 19401e9f3eb20780266bfe4fc07175690fa63419 Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Thu, 26 Oct 2023 18:59:27 -0700 Subject: [PATCH 06/11] Fix CLI typo in README.md. Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index bd70a2b8..5b9251ee 100644 --- a/README.md +++ b/README.md @@ -77,11 +77,11 @@ For general use, use the `main` branch along with [version 2.15.1 of the CodeQL 1. Build your CodeQL database: ``` - D:\codeql-home\codeql>codeql database create --language=cpp --source= --command= + D:\codeql-home\codeql>codeql database create --language=cpp --source-root= --command= ``` - Single driver example: `codeql database create D:\DriverDatabase --language=cpp --source=D:\Drivers\SingleDriver --command="msbuild /t:rebuild D:\Drivers\SingleDriver\SingleDriver.sln"` + Single driver example: `codeql database create D:\DriverDatabase --language=cpp --source-root=D:\Drivers\SingleDriver --command="msbuild /t:rebuild D:\Drivers\SingleDriver\SingleDriver.sln"` - Multiple drivers example: `codeql database create D:\SampleDriversDatabase --language=cpp --source=D:\AllMyDrivers\SampleDrivers --command=D:\AllMyDrivers\SampleDrivers\BuildAllSampleDrivers.cmd` + Multiple drivers example: `codeql database create D:\SampleDriversDatabase --language=cpp --source-root=D:\AllMyDrivers\SampleDrivers --command=D:\AllMyDrivers\SampleDrivers\BuildAllSampleDrivers.cmd` _(Parameters: path for your new database, language, driver source directory, build command.)_ From 29689e2cc276fa16ba9ad83378b327144d745e86 Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Thu, 26 Oct 2023 18:59:58 -0700 Subject: [PATCH 07/11] Update README.md Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5b9251ee..00e5faf8 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ For general use, use the `main` branch along with [version 2.15.1 of the CodeQL For WHCP BRANCHES: Skip this step. - For MAIN BRANCH use: + For MAIN AND DEVELOPMENT BRANCHES use: ``` D:\codeql-home\codeql>codeql pack install D:\codeql-home\Windows-Driver-Developer-Supplemental-Tools\src From f0d768514a9c4e9cec378db17e702d0350b8039b Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Thu, 26 Oct 2023 23:50:48 -0700 Subject: [PATCH 08/11] Update README.md Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 00e5faf8..987520ac 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ For general use, use the `main` branch along with [version 2.15.1 of the CodeQL D:\codeql-home\>git clone https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools.git --recurse-submodules ``` - For MAIN BRANCH use: + For MAIN AND DEVELOPMENT BRANCHES use: ``` D:\codeql-home\>git clone https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools.git From 984e2ad29c4f65ac740fd4221a764bc0fbca891a Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:36:33 -0700 Subject: [PATCH 09/11] Add IrqlSetTooHigh, IrqlSetTooLow queries + update IRQL library (#88) * Initial work at IRQL-checking * Significant extra IRQL work. * In-progress work * More puttering around with IRQL * Update to CodeQL 2.14.4 Update cpp-all to 0.9.2, cpp-queries to 0.7.4 * Commit more IRQL code. Needs cleanup. * Some cleanup and minor fixes to entry IRQL evaluation. * Replace old Irql high/low checks with new version and update library. Still needs cleanup. * Irql.qll cleanup * Get rid of old prototype version of IrqlTooLow * Update README.md * Clean up file names * Clean up queries. * Update test script for IRQL queries. * Update build-codeql.yaml Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Update ported_driver_ca_checks.qls * Add IrqlSetTooHigh/IrqlSetTooLow queries. * Bugfix for IrqlTooHigh/IrqlTooLow The changes to Irql.qll needed for IrqlSetTooHigh, etc. means we are more likely to see IRQL evaluations that return -1. Update queries to exclude those numbers. * Fix test issues for several IRQL checks. * WIP unit tests for IrqlSetTooHigh and IrqlSetTooLow queries * WIP unit tests for IrqlSetTooHigh and IrqlSetTooLow queries * WIP more tests and comments * bug fixes * WIP updates to tests * WIP update tests * remove bad tests. Fix run script to run all tests again. run script now cleans first automatically. * update tests for IrqlSetTooHigh * WIP IrqlSetTooLow tests * Fix typo in Irql.qll * Fix typo in Irql.qll * irqlSetTooHigh tests remove calls to KeGetCurrentIRQL as they are not needed * update IrqlSetTooLow tests * update tests. line 90 should be a failling test but isnt * fix IrqlLowerWithFunctionCall1 to call IrqlMinDispatchLowerIrql_fail1 * Revert"fix IrqlLowerWithFunctionCall1 to call IrqlMinDispatchLowerIrql_fail1" This reverts commit fd9084b7f74133647044a813a79fe668c8ee8f6b. * fix IrqlLowerWithFunctionCall1 to call IrqlMinDispatchLowerIrql_fail1 * Add some interprocedural IRQL analysis + comments * Add some interprocedural IRQL analysis + comments * Fix typos * Restore non-IRQL test results * Fix bug in driver_snippet.c that stopped compilation. * Fix bug in IrqlSetTooHigh Also refactor IrqlSetTooLow to match * Fix up test results for IRQL queries * Fix typos * Fix bug in IrqlSetTooHigh Also refactor IrqlSetTooLow to match * Fix line endings in diffs * Regressions due to IRQL changes (+1 benign change) * Update src/drivers/general/queries/experimental/IrqlSetTooHigh/driver_snippet.c Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Signed-off-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> * Update src/drivers/general/queries/experimental/IrqlSetTooHigh/driver_snippet.c Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Signed-off-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> * Update src/drivers/general/queries/experimental/IrqlSetTooHigh/driver_snippet.c Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Signed-off-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> * Update src/drivers/general/queries/experimental/IrqlSetTooHigh/driver_snippet.c Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Signed-off-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> * Fix IrqlNotSaved with the new library. * Update InitNotUsed with new DataFlow and a fix. Removes a false positive in our unit tests. * Update ported_driver_ca_checks.qls --------- Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Signed-off-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> Co-authored-by: jacob-ronstadt Co-authored-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> --- .../queries/IrqlNotSaved/IrqlNotSaved.ql | 104 ++--- .../queries/IrqlNotSaved/IrqlNotSaved.sarif | 171 ++++--- .../queries/IrqlNotSaved/driver_snippet.c | 18 +- .../queries/IrqlNotUsed/IrqlNotUsed.ql | 9 +- .../queries/IrqlNotUsed/IrqlNotUsed.sarif | 161 ++++--- .../IrqlSetTooHigh/IrqlSetTooHigh.qhelp | 23 + .../IrqlSetTooHigh/IrqlSetTooHigh.ql | 55 +++ .../IrqlSetTooHigh/IrqlSetTooHigh.sarif | 425 ++++++++++++++++++ .../IrqlSetTooHigh/driver_snippet.c | 241 ++++++++++ .../IrqlSetTooLow/IrqlSetTooLow.qhelp | 23 + .../IrqlSetTooLow/IrqlSetTooLow.ql | 48 ++ .../IrqlSetTooLow/IrqlSetTooLow.sarif | 362 +++++++++++++++ .../IrqlSetTooLow/driver_snippet.c | 126 ++++++ .../experimental/IrqlTooHigh/IrqlTooHigh.ql | 1 + .../experimental/IrqlTooLow/IrqlTooLow.ql | 1 + src/drivers/libraries/Irql.qll | 140 ++++-- src/drivers/libraries/IrqlDataFlow.qll | 43 ++ .../test/build_create_analyze_test.cmd | 7 + src/drivers/test/diff/IrqlSetTooHigh.sarif | 21 + src/drivers/test/diff/IrqlSetTooLow.sarif | 21 + src/suites/ported_driver_ca_checks.qls | 2 + 21 files changed, 1771 insertions(+), 231 deletions(-) create mode 100644 src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.qhelp create mode 100644 src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.ql create mode 100644 src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.sarif create mode 100644 src/drivers/general/queries/experimental/IrqlSetTooHigh/driver_snippet.c create mode 100644 src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.qhelp create mode 100644 src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.ql create mode 100644 src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.sarif create mode 100644 src/drivers/general/queries/experimental/IrqlSetTooLow/driver_snippet.c create mode 100644 src/drivers/libraries/IrqlDataFlow.qll create mode 100644 src/drivers/test/diff/IrqlSetTooHigh.sarif create mode 100644 src/drivers/test/diff/IrqlSetTooLow.sarif diff --git a/src/drivers/general/queries/IrqlNotSaved/IrqlNotSaved.ql b/src/drivers/general/queries/IrqlNotSaved/IrqlNotSaved.ql index ecfcfdb2..709faec2 100644 --- a/src/drivers/general/queries/IrqlNotSaved/IrqlNotSaved.ql +++ b/src/drivers/general/queries/IrqlNotSaved/IrqlNotSaved.ql @@ -22,23 +22,8 @@ import cpp import drivers.libraries.Irql -import semmle.code.cpp.dataflow.DataFlow -import semmle.code.cpp.dataflow.DataFlow2 - -/** - * A function that has at least one parameter annotated with "\_IRQL\_save\_". - */ -class IrqlSaveFunction extends Function { - Parameter p; - int irqlIndex; - - IrqlSaveFunction() { - p = this.getParameter(irqlIndex) and - p instanceof IrqlSaveParameter - } - - int getIrqlIndex() { result = irqlIndex } -} +import semmle.code.cpp.dataflow.new.DataFlow +import semmle.code.cpp.dataflow.new.DataFlow2 /** * A data-flow configuration describing flow from an @@ -55,7 +40,12 @@ class IrqlFlowConfiguration extends DataFlow::Configuration { override predicate isSink(DataFlow::Node sink) { exists(FunctionCall fc, FundamentalIrqlSaveFunction fisf | fc.getTarget() = fisf and - sink.asExpr() = fc.getArgument(fisf.getIrqlIndex()) + ( + sink.asExpr() = + fc.getArgument(fisf.(IrqlSavesGlobalAnnotatedFunction).getIrqlParameterSlot()) + or + sink.asExpr() = fc.getArgument(fisf.(IrqlSavesToParameterFunction).getIrqlParameterSlot()) + ) ) } } @@ -65,17 +55,25 @@ class IrqlFlowConfiguration extends DataFlow::Configuration { * by the Windows OS itself. This is in general in a Windows Kits header. For * extra clarity and internal use, we also list the exact header files. */ -class FundamentalIrqlSaveFunction extends IrqlSaveFunction { +class FundamentalIrqlSaveFunction extends IrqlSavesFunction { FundamentalIrqlSaveFunction() { - this.getFile().getAbsolutePath().matches("%Windows Kits%.h") or - this.getFile() - .getBaseName() - .matches(["wdm.h", "wdfsync.h", "ntifs.h", "ndis.h", "video.h", "wdfinterrupt.h"]) + ( + this.getFile().getAbsolutePath().matches("%Windows Kits%.h") + or + this.getFile() + .getBaseName() + .matches(["wdm.h", "wdfsync.h", "ntifs.h", "ndis.h", "video.h", "wdfinterrupt.h"]) + ) and + ( + this instanceof IrqlSavesToParameterFunction or + this instanceof IrqlSavesViaReturnFunction or + this instanceof IrqlSavesGlobalAnnotatedFunction + ) } } /** - * A simple data flow from any IrqlSaveParameter to another variable. + * A simple data flow from any IrqlSaveParameter. */ class IrqlSaveParameterFlowConfiguration extends DataFlow2::Configuration { IrqlSaveParameterFlowConfiguration() { this = "IrqlSaveParameterFlowConfiguration" } @@ -84,7 +82,7 @@ class IrqlSaveParameterFlowConfiguration extends DataFlow2::Configuration { source.asParameter() instanceof IrqlSaveParameter } - override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof VariableAccess } + override predicate isSink(DataFlow::Node sink) { sink instanceof DataFlow::Node } } /** @@ -97,29 +95,15 @@ class IrqlAssignmentFlowConfiguration extends DataFlow::Configuration { override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof FunctionCall and - source - .asExpr() - .(FunctionCall) - .getTarget() - .getName() - .matches([ - "KeRaiseIrqlToDpcLevel", "KfRaiseIrql", "KfAcquireSpinLock", - "KeAcquireSpinLockAtDpcLevel", "KeAcquireSpinLock", "KeAcquireSpinLockRaiseToDpc" - ]) + source.asExpr().(FunctionCall).getTarget() instanceof FundamentalIrqlSaveFunction and + source.asExpr().(FunctionCall).getTarget() instanceof IrqlSavesViaReturnFunction } override predicate isSink(DataFlow::Node sink) { - // Either we're sinking to a direct reference of a parameter, or... - sink.asExpr().(VariableAccess).getTarget() instanceof IrqlSaveParameter - or - // We a dereferenced pointer to the variable. - sink.asPartialDefinition() - .(PointerDereferenceExpr) - .getOperand() - .(AddressOfExpr) - .getOperand() - .(VariableAccess) - .getTarget() instanceof IrqlSaveVariableFlowedTo + exists(Assignment a | + a.getLValue().getAChild*().(VariableAccess).getTarget() instanceof IrqlSaveVariableFlowedTo and + a.getRValue() = sink.asExpr() + ) } } @@ -132,11 +116,14 @@ class IrqlSaveVariableFlowedTo extends Variable { IrqlSaveVariableFlowedTo() { exists( - IrqlSaveParameterFlowConfiguration difca, DataFlow::Node parameter, DataFlow::Node access + IrqlSaveParameterFlowConfiguration ispfc, DataFlow::Node parameter, DataFlow::Node assignment | - access.asExpr().(VariableAccess).getTarget() = this and + ( + this.getAnAssignedValue() = assignment.asExpr() or + this = assignment.asParameter() + ) and parameter.asParameter() = isp and - difca.hasFlow(parameter, access) + ispfc.hasFlow(parameter, assignment) ) or this = isp @@ -150,26 +137,19 @@ where // Exclude OS functions not isp.getFunction() instanceof FundamentalIrqlSaveFunction and /* - * Case one: does the IrqlSaveParameter (or an alias of it) have the IRQL assigned to it - * directly by calling, for example, KeRaiseIrql? + * Case one: does the IrqlSaveParameter (or an alias of it) have the IRQL assigned to it + * directly by calling, for example, KeRaiseIrql? */ not exists( - DataFlow::Node node, IrqlSaveVariableFlowedTo isvft, IrqlAssignmentFlowConfiguration difc + DataFlow::Node node, IrqlSaveVariableFlowedTo isvft, IrqlAssignmentFlowConfiguration iafc | isvft.getSaveParameter() = isp and - ( - node.asExpr().(VariableAccess).getTarget() = isvft - or - node.asPartialDefinition() - .(PointerDereferenceExpr) - .getOperand() - .(AddressOfExpr) - .getOperand() - .(VariableAccess) - .getTarget() = isvft + exists(Assignment a | + a.getLValue().getAChild*().(VariableAccess).getTarget() = isvft and + a.getRValue() = node.asExpr() ) and - difc.hasFlow(_, node) + iafc.hasFlow(_, node) ) and // Case two: is the IrqlSaveParameter passed into an OS function that will save a value to it? not exists(DataFlow::Node node, IrqlFlowConfiguration ifc | diff --git a/src/drivers/general/queries/IrqlNotSaved/IrqlNotSaved.sarif b/src/drivers/general/queries/IrqlNotSaved/IrqlNotSaved.sarif index e945eee7..ebd33304 100644 --- a/src/drivers/general/queries/IrqlNotSaved/IrqlNotSaved.sarif +++ b/src/drivers/general/queries/IrqlNotSaved/IrqlNotSaved.sarif @@ -6,7 +6,23 @@ "driver" : { "name" : "CodeQL", "organization" : "GitHub", - "semanticVersion" : "2.11.5", + "semanticVersion" : "2.14.4", + "notifications" : [ { + "id" : "cpp/baseline/expected-extracted-files", + "name" : "cpp/baseline/expected-extracted-files", + "shortDescription" : { + "text" : "Expected extracted files" + }, + "fullDescription" : { + "text" : "Files appearing in the source archive that are expected to be extracted." + }, + "defaultConfiguration" : { + "enabled" : true + }, + "properties" : { + "tags" : [ "expected-extracted-files", "telemetry" ] + } + } ], "rules" : [ { "id" : "cpp/drivers/irql-not-saved", "name" : "cpp/drivers/irql-not-saved", @@ -42,7 +58,7 @@ }, "extensions" : [ { "name" : "microsoft/windows-drivers", - "semanticVersion" : "0.1.0+933e876f096a70922173e4d5ad604d99d4481af4", + "semanticVersion" : "0.2.0+be14ce4fcaa9006e7636d8605fc53ea7c422a561", "locations" : [ { "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", "description" : { @@ -54,28 +70,99 @@ "text" : "The QL pack definition file." } } ] + } ] + }, + "invocations" : [ { + "toolExecutionNotifications" : [ { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } }, { - "name" : "legacy-upgrades", - "semanticVersion" : "0.0.0", "locations" : [ { - "uri" : "file:///C:/codeql-home/codeql/legacy-upgrades/", - "description" : { - "text" : "The QL pack root directory." + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } } - }, { - "uri" : "file:///C:/codeql-home/codeql/legacy-upgrades/qlpack.yml", - "description" : { - "text" : "The QL pack definition file." + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" } - } ] - } ] - }, + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + } ], + "executionSuccessful" : true + } ], "artifacts" : [ { "location" : { "uri" : "driver/driver_snippet.c", "uriBaseId" : "%SRCROOT%", "index" : 0 } + }, { + "location" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } } ], "results" : [ { "ruleId" : "cpp/drivers/irql-not-saved", @@ -85,7 +172,7 @@ "index" : 0 }, "message" : { - "text" : "The parameter [outIrql](1) is annotated \"_IRQL_saves_\" but never has the IRQL saved to it." + "text" : "The parameter [outIrqlFail](1) is annotated \"_IRQL_saves_\" but never has the IRQL saved to it." }, "locations" : [ { "physicalLocation" : { @@ -97,12 +184,12 @@ "region" : { "startLine" : 51, "startColumn" : 44, - "endColumn" : 51 + "endColumn" : 55 } } } ], "partialFingerprints" : { - "primaryLocationLineHash" : "6276e1ac4864af21:1", + "primaryLocationLineHash" : "e054bbd9d7acc717:1", "primaryLocationStartColumnFingerprint" : "43" }, "relatedLocations" : [ { @@ -116,57 +203,11 @@ "region" : { "startLine" : 51, "startColumn" : 44, - "endColumn" : 51 - } - }, - "message" : { - "text" : "outIrql" - } - } ] - }, { - "ruleId" : "cpp/drivers/irql-not-saved", - "ruleIndex" : 0, - "rule" : { - "id" : "cpp/drivers/irql-not-saved", - "index" : 0 - }, - "message" : { - "text" : "The parameter [myLock](1) is annotated \"_IRQL_saves_\" but never has the IRQL saved to it." - }, - "locations" : [ { - "physicalLocation" : { - "artifactLocation" : { - "uri" : "driver/driver_snippet.c", - "uriBaseId" : "%SRCROOT%", - "index" : 0 - }, - "region" : { - "startLine" : 43, - "startColumn" : 48, - "endColumn" : 54 - } - } - } ], - "partialFingerprints" : { - "primaryLocationLineHash" : "342705500599b584:1", - "primaryLocationStartColumnFingerprint" : "47" - }, - "relatedLocations" : [ { - "id" : 1, - "physicalLocation" : { - "artifactLocation" : { - "uri" : "driver/driver_snippet.c", - "uriBaseId" : "%SRCROOT%", - "index" : 0 - }, - "region" : { - "startLine" : 43, - "startColumn" : 48, - "endColumn" : 54 + "endColumn" : 55 } }, "message" : { - "text" : "myLock" + "text" : "outIrqlFail" } } ] } ], diff --git a/src/drivers/general/queries/IrqlNotSaved/driver_snippet.c b/src/drivers/general/queries/IrqlNotSaved/driver_snippet.c index 95508dd0..449039b1 100644 --- a/src/drivers/general/queries/IrqlNotSaved/driver_snippet.c +++ b/src/drivers/general/queries/IrqlNotSaved/driver_snippet.c @@ -17,24 +17,24 @@ void top_level_call() {} /* Auxillary function used in passing. Note that even though this function doesn't annotate KIRQL, it restores the IRQL correctly. */ -VOID IrqlNotSaved_passAux(KIRQL outIrql) { +VOID IrqlNotSaved_passAux(KIRQL outIrqlPassAux) { - KeRaiseIrql(KeGetCurrentIrql(), &outIrql); + KeRaiseIrql(KeGetCurrentIrql(), &outIrqlPassAux); } /* Passing case one. Function directly calls a function that saves IRQL. */ -VOID IrqlNotSaved_pass(_IRQL_saves_ KIRQL outIrql, PKSPIN_LOCK myLock) { +VOID IrqlNotSaved_pass(_IRQL_saves_ KIRQL outIrqlPass, PKSPIN_LOCK myLock) { - KeAcquireSpinLock(myLock, &outIrql); + KeAcquireSpinLock(myLock, &outIrqlPass); } /* Passing case two. Function indirectly calls a function that saves IRQL. */ -VOID IrqlNotSaved_pass2(_IRQL_saves_ KIRQL outIrql, PKSPIN_LOCK myLock) { +VOID IrqlNotSaved_pass2(_IRQL_saves_ KIRQL outIrqlPass2, PKSPIN_LOCK myLock) { - IrqlNotSaved_passAux(outIrql); + IrqlNotSaved_passAux(outIrqlPass2); } @@ -48,16 +48,16 @@ VOID IrqlNotSaved_pass3(_IRQL_saves_ PTestLock myLock) { /* Failing case one. Function does nothing with IRQL. */ -VOID IrqlNotSaved_fail1(_IRQL_saves_ KIRQL outIrql, PKSPIN_LOCK myLock) { +VOID IrqlNotSaved_fail1(_IRQL_saves_ KIRQL outIrqlFail, PKSPIN_LOCK myLock) { } /* Failing case two. Function has a path where the IRQL is not saved. This requires must-flow analysis. */ -VOID IrqlNotSaved_fail2(_IRQL_saves_ KIRQL outIrql, PKSPIN_LOCK myLock, int testValue) { +VOID IrqlNotSaved_fail2(_IRQL_saves_ KIRQL outIrqlFail2, PKSPIN_LOCK myLock, int testValue) { - if (testValue > 15) {KeRaiseIrql(KeGetCurrentIrql(), &outIrql); } + if (testValue > 15) {KeRaiseIrql(KeGetCurrentIrql(), &outIrqlFail2); } else { } } \ No newline at end of file diff --git a/src/drivers/general/queries/IrqlNotUsed/IrqlNotUsed.ql b/src/drivers/general/queries/IrqlNotUsed/IrqlNotUsed.ql index cca628b5..da32c0bf 100644 --- a/src/drivers/general/queries/IrqlNotUsed/IrqlNotUsed.ql +++ b/src/drivers/general/queries/IrqlNotUsed/IrqlNotUsed.ql @@ -22,7 +22,8 @@ import cpp import drivers.libraries.Irql -import semmle.code.cpp.dataflow.DataFlow +import semmle.code.cpp.dataflow.new.DataFlow +import semmle.code.cpp.dataflow.new.DataFlow2 /** * A function that has at least one parameter annotated with "\_IRQL\_restores\_". @@ -70,7 +71,11 @@ class IrqlFlowConfiguration extends DataFlow::Configuration { override predicate isSink(DataFlow::Node sink) { exists(FunctionCall fc, FundamentalIrqlRestoreFunction firf | fc.getTarget() = firf and - sink.asExpr() = fc.getArgument(firf.getIrqlIndex()) + ( + sink.asExpr() = fc.getArgument(firf.getIrqlIndex()) + or + sink.asExpr() = fc.getArgument(firf.getIrqlIndex()).getAChild*() + ) ) } } diff --git a/src/drivers/general/queries/IrqlNotUsed/IrqlNotUsed.sarif b/src/drivers/general/queries/IrqlNotUsed/IrqlNotUsed.sarif index 2009fa39..782cdfe8 100644 --- a/src/drivers/general/queries/IrqlNotUsed/IrqlNotUsed.sarif +++ b/src/drivers/general/queries/IrqlNotUsed/IrqlNotUsed.sarif @@ -6,7 +6,23 @@ "driver" : { "name" : "CodeQL", "organization" : "GitHub", - "semanticVersion" : "2.11.5", + "semanticVersion" : "2.14.4", + "notifications" : [ { + "id" : "cpp/baseline/expected-extracted-files", + "name" : "cpp/baseline/expected-extracted-files", + "shortDescription" : { + "text" : "Expected extracted files" + }, + "fullDescription" : { + "text" : "Files appearing in the source archive that are expected to be extracted." + }, + "defaultConfiguration" : { + "enabled" : true + }, + "properties" : { + "tags" : [ "expected-extracted-files", "telemetry" ] + } + } ], "rules" : [ { "id" : "cpp/drivers/irql-not-used", "name" : "cpp/drivers/irql-not-used", @@ -42,7 +58,7 @@ }, "extensions" : [ { "name" : "microsoft/windows-drivers", - "semanticVersion" : "0.1.0+933e876f096a70922173e4d5ad604d99d4481af4", + "semanticVersion" : "0.2.0+079028b57055ccd2fea3bae5a9ecc283d9ee56bb", "locations" : [ { "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", "description" : { @@ -54,28 +70,99 @@ "text" : "The QL pack definition file." } } ] + } ] + }, + "invocations" : [ { + "toolExecutionNotifications" : [ { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } }, { - "name" : "legacy-upgrades", - "semanticVersion" : "0.0.0", "locations" : [ { - "uri" : "file:///C:/codeql-home/codeql/legacy-upgrades/", - "description" : { - "text" : "The QL pack root directory." + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } } - }, { - "uri" : "file:///C:/codeql-home/codeql/legacy-upgrades/qlpack.yml", - "description" : { - "text" : "The QL pack definition file." + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" } - } ] - } ] - }, + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + } ], + "executionSuccessful" : true + } ], "artifacts" : [ { "location" : { "uri" : "driver/driver_snippet.c", "uriBaseId" : "%SRCROOT%", "index" : 0 } + }, { + "location" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } } ], "results" : [ { "ruleId" : "cpp/drivers/irql-not-used", @@ -123,52 +210,6 @@ "text" : "inIrql" } } ] - }, { - "ruleId" : "cpp/drivers/irql-not-used", - "ruleIndex" : 0, - "rule" : { - "id" : "cpp/drivers/irql-not-used", - "index" : 0 - }, - "message" : { - "text" : "This function has annotated the parameter [myLock](1) with \"_IRQL_restores_\" but does not use it to restore the IRQL." - }, - "locations" : [ { - "physicalLocation" : { - "artifactLocation" : { - "uri" : "driver/driver_snippet.c", - "uriBaseId" : "%SRCROOT%", - "index" : 0 - }, - "region" : { - "startLine" : 43, - "startColumn" : 6, - "endColumn" : 23 - } - } - } ], - "partialFingerprints" : { - "primaryLocationLineHash" : "41c0a3dc1c9cab79:1", - "primaryLocationStartColumnFingerprint" : "5" - }, - "relatedLocations" : [ { - "id" : 1, - "physicalLocation" : { - "artifactLocation" : { - "uri" : "driver/driver_snippet.c", - "uriBaseId" : "%SRCROOT%", - "index" : 0 - }, - "region" : { - "startLine" : 43, - "startColumn" : 50, - "endColumn" : 56 - } - }, - "message" : { - "text" : "myLock" - } - } ] } ], "columnKind" : "utf16CodeUnits", "properties" : { diff --git a/src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.qhelp b/src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.qhelp new file mode 100644 index 00000000..19ac4ad8 --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.qhelp @@ -0,0 +1,23 @@ + + + +

    + The function has raised the IRQL to a level above what is allowed. +

    +     + +

    + A function has been annotated as having a max IRQL, but the execution of that function raises the IRQL above that maximum. If you have applied custom IRQL annotations to your own functions, confirm that they are accurate. +

    +     + + + + +
  • + + C28150 + +
    • +     +     diff --git a/src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.ql b/src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.ql new file mode 100644 index 00000000..11cbc3b0 --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.ql @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +/** + * @id cpp/drivers/irql-set-too-high + * @name IRQL set too high (C28150) + * @description A function annotated with a maximum IRQL for execution raises the IRQL above that amount. + * @platform Desktop + * @security.severity Low + * @feature.area Multiple + * @impact Exploitable Design + * @repro.text The following statement exits at an IRQL too high for the function it is contained in. + * @owner.email sdat@microsoft.com + * @opaqueid CQLD-C28150 + * @kind problem + * @problem.severity warning + * @precision medium + * @tags correctness + * wddst + * @scope domainspecific + * @query-version v1 + */ + +import cpp +import drivers.libraries.Irql + +bindingset[irqlRequirement] +predicate tooHighForFunc( + IrqlRestrictsFunction irqlFunc, ControlFlowNode statement, int irqlRequirement +) { + statement.getControlFlowScope() = irqlFunc and + irqlRequirement < min(getPotentialExitIrqlAtCfn(statement)) and + irqlRequirement != -1 +} + +from IrqlRestrictsFunction irqlFunc, ControlFlowNode statement, int irqlRequirement +where + ( + irqlFunc.(IrqlAlwaysMaxFunction).getIrqlLevel() = irqlRequirement + or + // If we don't have an explicit max annotation but do raise the IRQL, + // we treat the raised-to level as the implicit max. + not irqlFunc instanceof IrqlAlwaysMaxFunction and + irqlFunc.(IrqlRaisesAnnotatedFunction).getIrqlLevel() = irqlRequirement + ) and + tooHighForFunc(irqlFunc, statement, irqlRequirement) and + // Only get the first node which is set too low + not exists(ControlFlowNode otherNode | + otherNode.getControlFlowScope() = irqlFunc and + otherNode = statement.getAPredecessor() and + tooHighForFunc(irqlFunc, otherNode, irqlRequirement) + ) +select statement, + "$@: IRQL potentially set too high at $@. Maximum IRQL for this function: " + irqlRequirement + + ", IRQL at statement: " + min(getPotentialExitIrqlAtCfn(statement)), irqlFunc, + irqlFunc.getQualifiedName(), statement, statement.toString() diff --git a/src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.sarif b/src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.sarif new file mode 100644 index 00000000..35ad3d14 --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.sarif @@ -0,0 +1,425 @@ +{ + "$schema" : "https://json.schemastore.org/sarif-2.1.0.json", + "version" : "2.1.0", + "runs" : [ { + "tool" : { + "driver" : { + "name" : "CodeQL", + "organization" : "GitHub", + "semanticVersion" : "2.14.4", + "notifications" : [ { + "id" : "cpp/baseline/expected-extracted-files", + "name" : "cpp/baseline/expected-extracted-files", + "shortDescription" : { + "text" : "Expected extracted files" + }, + "fullDescription" : { + "text" : "Files appearing in the source archive that are expected to be extracted." + }, + "defaultConfiguration" : { + "enabled" : true + }, + "properties" : { + "tags" : [ "expected-extracted-files", "telemetry" ] + } + } ], + "rules" : [ { + "id" : "cpp/drivers/irql-set-too-high", + "name" : "cpp/drivers/irql-set-too-high", + "shortDescription" : { + "text" : "IRQL set too high (C28150)" + }, + "fullDescription" : { + "text" : "A function annotated with a maximum IRQL for execution raises the IRQL above that amount." + }, + "defaultConfiguration" : { + "enabled" : true, + "level" : "warning" + }, + "properties" : { + "tags" : [ "correctness", "wddst" ], + "description" : "A function annotated with a maximum IRQL for execution raises the IRQL above that amount.", + "feature.area" : "Multiple", + "id" : "cpp/drivers/irql-set-too-high", + "impact" : "Exploitable Design", + "kind" : "problem", + "name" : "IRQL set too high (C28150)", + "opaqueid" : "CQLD-C28150", + "owner.email" : "sdat@microsoft.com", + "platform" : "Desktop", + "precision" : "medium", + "problem.severity" : "warning", + "query-version" : "v1", + "repro.text" : "The following statement exits at an IRQL too high for the function it is contained in.", + "scope" : "domainspecific", + "security.severity" : "Low" + } + } ] + }, + "extensions" : [ { + "name" : "microsoft/windows-drivers", + "semanticVersion" : "0.2.0+9e5ae32394a3e411584e20e992a697add48b30b5", + "locations" : [ { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", + "description" : { + "text" : "The QL pack root directory." + } + }, { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/qlpack.yml", + "description" : { + "text" : "The QL pack definition file." + } + } ] + } ] + }, + "invocations" : [ { + "toolExecutionNotifications" : [ { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + } ], + "executionSuccessful" : true + } ], + "artifacts" : [ { + "location" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } ], + "results" : [ { + "ruleId" : "cpp/drivers/irql-set-too-high", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-set-too-high", + "index" : 0 + }, + "message" : { + "text" : "[CallFunctionThatRaisesIRQL_fail5](1): IRQL potentially set too high at [call to IrqlSetHigherFromPassive_pass0](2). Maximum IRQL for this function: 0, IRQL at statement: 2" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 131, + "startColumn" : 5, + "endColumn" : 35 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "50d7736bf7d9212d:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 129, + "startColumn" : 10, + "endColumn" : 42 + } + }, + "message" : { + "text" : "CallFunctionThatRaisesIRQL_fail5" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 131, + "startColumn" : 5, + "endColumn" : 35 + } + }, + "message" : { + "text" : "call to IrqlSetHigherFromPassive_pass0" + } + } ] + }, { + "ruleId" : "cpp/drivers/irql-set-too-high", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-set-too-high", + "index" : 0 + }, + "message" : { + "text" : "[IrqlRaiseLevelExplicit_fail4](1): IRQL potentially set too high at [call to KfRaiseIrql](2). Maximum IRQL for this function: 0, IRQL at statement: 2" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 121, + "startColumn" : 5, + "endColumn" : 42 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "b7bb153208f2004d:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 118, + "startColumn" : 10, + "endColumn" : 38 + } + }, + "message" : { + "text" : "IrqlRaiseLevelExplicit_fail4" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 121, + "startColumn" : 5, + "endColumn" : 42 + } + }, + "message" : { + "text" : "call to KfRaiseIrql" + } + } ] + }, { + "ruleId" : "cpp/drivers/irql-set-too-high", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-set-too-high", + "index" : 0 + }, + "message" : { + "text" : "[IrqlRaiseLevelExplicit_fail3](1): IRQL potentially set too high at [call to KfRaiseIrql](2). Maximum IRQL for this function: 0, IRQL at statement: 2" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 112, + "startColumn" : 5, + "endColumn" : 42 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "988957c55591351a:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 109, + "startColumn" : 10, + "endColumn" : 38 + } + }, + "message" : { + "text" : "IrqlRaiseLevelExplicit_fail3" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 112, + "startColumn" : 5, + "endColumn" : 42 + } + }, + "message" : { + "text" : "call to KfRaiseIrql" + } + } ] + }, { + "ruleId" : "cpp/drivers/irql-set-too-high", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-set-too-high", + "index" : 0 + }, + "message" : { + "text" : "[IrqlRaiseLevelExplicit_fail0](1): IRQL potentially set too high at [call to KfRaiseIrql](2). Maximum IRQL for this function: 1, IRQL at statement: 2" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 102, + "startColumn" : 5, + "endColumn" : 42 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "71b218a9127ea6cb:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 99, + "startColumn" : 10, + "endColumn" : 38 + } + }, + "message" : { + "text" : "IrqlRaiseLevelExplicit_fail0" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 102, + "startColumn" : 5, + "endColumn" : 42 + } + }, + "message" : { + "text" : "call to KfRaiseIrql" + } + } ] + } ], + "columnKind" : "utf16CodeUnits", + "properties" : { + "semmle.formatSpecifier" : "sarifv2.1.0" + } + } ] +} \ No newline at end of file diff --git a/src/drivers/general/queries/experimental/IrqlSetTooHigh/driver_snippet.c b/src/drivers/general/queries/experimental/IrqlSetTooHigh/driver_snippet.c new file mode 100644 index 00000000..2dc1afc3 --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlSetTooHigh/driver_snippet.c @@ -0,0 +1,241 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +// driver_snippet.c +// + +#define SET_DISPATCH 1 + +// Template. Not called in this test. +void top_level_call() {} + +#include + +/* +IRQL values: +PASSIVE_LEVEL +APC_LEVEL +DISPATCH_LEVEL +DIRQL +*/ + +/* +_IRQL_requires_max_(irql) //The irql is the maximum IRQL at which the function can be called. +_IRQL_requires_min_(irql) //The irql is the minimum IRQL at which the function can be called. +_IRQL_requires_(irql) //The function must be entered at the IRQL specified by irql. +_IRQL_raises_(irql) //The function exits at the specified irql, but it can only be called to raise (not lower) the current IRQL. +_IRQL_saves_ //The annotated parameter saves the current IRQL to restore later. +_IRQL_restores_ //The annotated parameter contains an IRQL value from IRQL_saves that is to be restored when the function returns. +_IRQL_saves_global_(kind, param) //The current IRQL is saved into a location that is internal to the code analysis tools from which the IRQL is to be restored. This annotation is used to annotate a function. The location is identified by kind and further refined by param. For example, OldIrql could be the kind, and FastMutex could be the parameter that held that old IRQL value. +_IRQL_restores_global_(kind, param) //The IRQL saved by the function annotated with IRQL_saves_global is restored from a location that is internal to the Code Analysis tools. +_IRQL_always_function_min_(value) //The IRQL value is the minimum value to which the function can lower the IRQL. +_IRQL_always_function_max_(value) //The IRQL value is the maximum value to which the function can raise the IRQL. +_IRQL_requires_same_ //The annotated function must enter and exit at the same IRQL. The function can change the IRQL, but it must restore the IRQL to its original value before exiting. +_IRQL_uses_cancel_ //The annotated parameter is the IRQL value that should be restored by a DRIVER_CANCEL callback function. In most cases, use the IRQL_is_cancel annotation instead. +*/ + +/* +Function which should only be called from max APC_LEVEL +*/ +_IRQL_requires_max_(APC_LEVEL) + VOID DoNothing_MaxAPC(void) +{ + __noop; +} + +/* +Function which should only be called from max DISPATCH_LEVEL +*/ +_IRQL_requires_max_(DISPATCH_LEVEL) + VOID DoNothing_MaxDispatch(void) +{ + __noop; +} + +/* +Function which should only be called from PASSIVE_LEVEL +*/ +_IRQL_requires_(PASSIVE_LEVEL) + VOID DoNothing_RequiresPassive(void) +{ + __noop; +} + +/* +Function which should only be called from DISPATCH_LEVEL +*/ +_IRQL_requires_(DISPATCH_LEVEL) + VOID DoNothing_RequiresDispatch(void) +{ + __noop; +} + +_IRQL_raises_(DISPATCH_LEVEL) + VOID IrqlSetHigherFromPassive_pass0(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(DISPATCH_LEVEL, &oldIRQL); +} + + +/* +Function can be called to raise the IRQL but needs to exit at DISPATCH_LEVEL. +*/ +_IRQL_raises_(DISPATCH_LEVEL) + VOID IrqlRaiseLevelExplicit_pass1(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(APC_LEVEL, &oldIRQL); // Raise level + DoNothing_MaxDispatch(); // call function with max DISPATCH_LEVEL. This is OK since we're at APC_LEVEL and that is less than DISPATCH_LEVEL + KeRaiseIrql(DISPATCH_LEVEL, &oldIRQL); // Raise level again + DoNothing_MaxDispatch(); // call function with max DISPATCH_LEVEL. This is OK since we're at DISPATCH_LEVEL + // Function Exits at DISPATCH_LEVEL +} + +/* +Function can be called to raise the IRQL but needs to exit at APC_LEVEL, but it raises the IRQL to DISPATCH_LEVEL. +*/ +_IRQL_raises_(APC_LEVEL) + VOID IrqlRaiseLevelExplicit_fail0(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(DISPATCH_LEVEL, &oldIRQL); +} + +/* +Function is annotated for max IRQL PASSIVE_LEVEL but raises the IRQL +*/ +_IRQL_always_function_max_(PASSIVE_LEVEL) + VOID IrqlRaiseLevelExplicit_fail3(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(DISPATCH_LEVEL, &oldIRQL); +} +/* +Function is annotated for max IRQL PASSIVE_LEVEL but raises the IRQL and then lowers it. Desipite lowering the IRQL, the function is still annotated for max IRQL PASSIVE_LEVEL +*/ +_IRQL_always_function_max_(PASSIVE_LEVEL) + VOID IrqlRaiseLevelExplicit_fail4(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(DISPATCH_LEVEL, &oldIRQL); + KeLowerIrql(oldIRQL); +} + +/* +Function is annotated for max IRQL PASSIVE_LEVEL, but it raises the IRQL to DISPATCH_LEVEL through another function call. +*/ +_IRQL_always_function_max_(PASSIVE_LEVEL) + VOID CallFunctionThatRaisesIRQL_fail5(void) +{ + IrqlSetHigherFromPassive_pass0(); +} + +/* +Function is annotated for max IRQL PASSIVE_LEVEL, but it raises the IRQL to DISPATCH_LEVEL through another function call. +*/ +// TODO what is the IRQL by default if not set? +_IRQL_requires_same_ + VOID CallFunctionThatRaisesIRQL_fail6(void) +{ + IrqlSetHigherFromPassive_pass0(); +} + + +/* +Function is annotated for max IRQL PASSIVE_LEVEL and does not raise the IRQL +*/ +_IRQL_always_function_max_(PASSIVE_LEVEL) + VOID IrqlDontChange_pass(void) +{ + DoNothing_MaxAPC(); +} + + +/* +Function must enter and exit at the same IRQL, but raises and does not lower the IRQL +*/ +_IRQL_requires_same_ + VOID + IrqlRequiresSame_fail7(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(DISPATCH_LEVEL, &oldIRQL); +} + +/* +Function must enter and exit at the same IRQL, but raises and does not lower the IRQL +*/ +_IRQL_requires_same_ + VOID + IrqlRequiresSame_notsupported(void) +{ + KIRQL oldIRQL = PASSIVE_LEVEL; + KeRaiseIrql(oldIRQL+1, &oldIRQL); +} + + + +/* +Funciton must enter and exit at the same IRQL. IRQL is set higher but then set lower before exiting. +*/ +_IRQL_requires_same_ + VOID + IrqlRequiresSame_pass(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(DISPATCH_LEVEL, &oldIRQL); + KeLowerIrql(oldIRQL); +} + +/* +Function that calls another function by reference which correctly raises the IRQL. +This should pass since IrqlInderectCall_pass0 is not annotated for max IRQL. +*/ +VOID IrqlIndirectCall_pass0(void) + +{ + void (*funcPtr)(void); + funcPtr = &IrqlSetHigherFromPassive_pass0; + funcPtr(); +} +/* +Function that calls another function by reference which correctly raises the IRQL. +This should pass since IrqlInderectCall_pass0 is annotated for max DISPATCH_LEVEL. +*/ +_IRQL_always_function_max_(DISPATCH_LEVEL) +VOID IrqlIndirectCall_pass1(void) + +{ + void (*funcPtr)(void); + funcPtr = &IrqlSetHigherFromPassive_pass0; + funcPtr(); +} + +/* +Function that calls another function by reference which incorrectly raises the IRQL. +This should fail because the function pointer points to a function that should fail. +*/ +VOID IrqlIndirectCall_fail0(void) + +{ + void (*funcPtr)(void); + funcPtr = &IrqlRaiseLevelExplicit_fail0; + funcPtr(); +} +/* +Function that calls another function by reference which incorrectly raises the IRQL. +This should fail because the function pointer points to a function that that raises the IRQL above PASSIVE_LEVEL. +*/ +_IRQL_always_function_max_(PASSIVE_LEVEL) +VOID IrqlIndirectCall_fail1(void) + +{ + void (*funcPtr)(void); + funcPtr = &IrqlSetHigherFromPassive_pass0; + funcPtr(); +} + + +// TODO multi-threaded tests +// function has max IRQL requirement, creates two threads where one is above that requirement and one is below diff --git a/src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.qhelp b/src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.qhelp new file mode 100644 index 00000000..440aca9c --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.qhelp @@ -0,0 +1,23 @@ + + + +

    + The function has lowered the IRQL to a level below what is allowed. +

    +     + +

    + A function has been annotated as having a minimum IRQL, but the execution of that function lowers the IRQL below that minimum. If you have applied custom IRQL annotations to your own functions, confirm that they are accurate. +

    +     + + + + +
  • + + C28124 + +
    • +     +     diff --git a/src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.ql b/src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.ql new file mode 100644 index 00000000..1a894fa4 --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.ql @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +/** + * @id cpp/drivers/irql-set-too-low + * @name IRQL set too low (C28124) + * @description A function annotated with a minimum IRQL for execution lowers the IRQL below that amount. + * @platform Desktop + * @security.severity Low + * @feature.area Multiple + * @impact Exploitable Design + * @repro.text The following statement exits at an IRQL too low for the function it is contained in. + * @owner.email sdat@microsoft.com + * @opaqueid CQLD-C28124 + * @kind problem + * @problem.severity warning + * @precision medium + * @tags correctness + * wddst + * @scope domainspecific + * @query-version v1 + */ + +import cpp +import drivers.libraries.Irql + +bindingset[irqlRequirement] +predicate tooLowForFunc( + IrqlRestrictsFunction irqlFunc, ControlFlowNode statement, int irqlRequirement +) { + statement.getControlFlowScope() = irqlFunc and + irqlRequirement > max(getPotentialExitIrqlAtCfn(statement)) and + irqlRequirement != -1 +} + +from IrqlRestrictsFunction irqlFunc, ControlFlowNode statement, int irqlRequirement +where + irqlFunc.(IrqlAlwaysMinFunction).getIrqlLevel() = irqlRequirement and + tooLowForFunc(irqlFunc, statement, irqlRequirement) and + // Only get the first node which is set too low + not exists(ControlFlowNode otherNode | + otherNode.getControlFlowScope() = irqlFunc and + otherNode = statement.getAPredecessor() and + tooLowForFunc(irqlFunc, otherNode, irqlRequirement) + ) +select statement, + "$@: IRQL potentially set too low at $@. Minimum IRQL for this function: " + irqlRequirement + + ", IRQL at statement: " + max(getPotentialExitIrqlAtCfn(statement)), irqlFunc, + irqlFunc.getQualifiedName(), statement, statement.toString() diff --git a/src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.sarif b/src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.sarif new file mode 100644 index 00000000..bc06b3b7 --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.sarif @@ -0,0 +1,362 @@ +{ + "$schema" : "https://json.schemastore.org/sarif-2.1.0.json", + "version" : "2.1.0", + "runs" : [ { + "tool" : { + "driver" : { + "name" : "CodeQL", + "organization" : "GitHub", + "semanticVersion" : "2.14.4", + "notifications" : [ { + "id" : "cpp/baseline/expected-extracted-files", + "name" : "cpp/baseline/expected-extracted-files", + "shortDescription" : { + "text" : "Expected extracted files" + }, + "fullDescription" : { + "text" : "Files appearing in the source archive that are expected to be extracted." + }, + "defaultConfiguration" : { + "enabled" : true + }, + "properties" : { + "tags" : [ "expected-extracted-files", "telemetry" ] + } + } ], + "rules" : [ { + "id" : "cpp/drivers/irql-set-too-low", + "name" : "cpp/drivers/irql-set-too-low", + "shortDescription" : { + "text" : "IRQL set too low (C28124)" + }, + "fullDescription" : { + "text" : "A function annotated with a minimum IRQL for execution lowers the IRQL below that amount." + }, + "defaultConfiguration" : { + "enabled" : true, + "level" : "warning" + }, + "properties" : { + "tags" : [ "correctness", "wddst" ], + "description" : "A function annotated with a minimum IRQL for execution lowers the IRQL below that amount.", + "feature.area" : "Multiple", + "id" : "cpp/drivers/irql-set-too-low", + "impact" : "Exploitable Design", + "kind" : "problem", + "name" : "IRQL set too low (C28124)", + "opaqueid" : "CQLD-C28124", + "owner.email" : "sdat@microsoft.com", + "platform" : "Desktop", + "precision" : "medium", + "problem.severity" : "warning", + "query-version" : "v1", + "repro.text" : "The following statement exits at an IRQL too low for the function it is contained in.", + "scope" : "domainspecific", + "security.severity" : "Low" + } + } ] + }, + "extensions" : [ { + "name" : "microsoft/windows-drivers", + "semanticVersion" : "0.2.0+9e5ae32394a3e411584e20e992a697add48b30b5", + "locations" : [ { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", + "description" : { + "text" : "The QL pack root directory." + } + }, { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/qlpack.yml", + "description" : { + "text" : "The QL pack definition file." + } + } ] + } ] + }, + "invocations" : [ { + "toolExecutionNotifications" : [ { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + } ], + "executionSuccessful" : true + } ], + "artifacts" : [ { + "location" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } ], + "results" : [ { + "ruleId" : "cpp/drivers/irql-set-too-low", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-set-too-low", + "index" : 0 + }, + "message" : { + "text" : "[IrqlAlwaysMinAPC_fail](1): IRQL potentially set too low at [call to KeLowerIrql](2). Minimum IRQL for this function: 1, IRQL at statement: 0" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 92, + "startColumn" : 5, + "endColumn" : 16 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "8a19ae2477ed23d3:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 90, + "startColumn" : 10, + "endColumn" : 31 + } + }, + "message" : { + "text" : "IrqlAlwaysMinAPC_fail" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 92, + "startColumn" : 5, + "endColumn" : 16 + } + }, + "message" : { + "text" : "call to KeLowerIrql" + } + } ] + }, { + "ruleId" : "cpp/drivers/irql-set-too-low", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-set-too-low", + "index" : 0 + }, + "message" : { + "text" : "[IrqlMinDispatchLowerIrql_fail1](1): IRQL potentially set too low at [call to KeLowerIrql](2). Minimum IRQL for this function: 2, IRQL at statement: 1" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 59, + "startColumn" : 5, + "endColumn" : 16 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "c6798a9b4760c05b:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 57, + "startColumn" : 10, + "endColumn" : 40 + } + }, + "message" : { + "text" : "IrqlMinDispatchLowerIrql_fail1" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 59, + "startColumn" : 5, + "endColumn" : 16 + } + }, + "message" : { + "text" : "call to KeLowerIrql" + } + } ] + }, { + "ruleId" : "cpp/drivers/irql-set-too-low", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/irql-set-too-low", + "index" : 0 + }, + "message" : { + "text" : "[IrqlMinDispatchLowerIrql_fail](1): IRQL potentially set too low at [{{ ... }}](2). Minimum IRQL for this function: 2, IRQL at statement: 1" + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 42, + "endLine" : 44, + "endColumn" : 2 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "83574f45ab0b5d97:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 41, + "startColumn" : 10, + "endColumn" : 39 + } + }, + "message" : { + "text" : "IrqlMinDispatchLowerIrql_fail" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 42, + "endLine" : 44, + "endColumn" : 2 + } + }, + "message" : { + "text" : "{{ ... }}" + } + } ] + } ], + "columnKind" : "utf16CodeUnits", + "properties" : { + "semmle.formatSpecifier" : "sarifv2.1.0" + } + } ] +} \ No newline at end of file diff --git a/src/drivers/general/queries/experimental/IrqlSetTooLow/driver_snippet.c b/src/drivers/general/queries/experimental/IrqlSetTooLow/driver_snippet.c new file mode 100644 index 00000000..04e388c2 --- /dev/null +++ b/src/drivers/general/queries/experimental/IrqlSetTooLow/driver_snippet.c @@ -0,0 +1,126 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +// driver_snippet.c +// + +#define SET_DISPATCH 1 + +// Template. Not called in this test. +void top_level_call() {} + +#include + +/* +IRQL values: +PASSIVE_LEVEL +APC_LEVEL +DISPATCH_LEVEL +DIRQL +*/ + +/* +_IRQL_requires_max_(irql) //The irql is the maximum IRQL at which the function can be called. +_IRQL_requires_min_(irql) //The irql is the minimum IRQL at which the function can be called. +_IRQL_requires_(irql) //The function must be entered at the IRQL specified by irql. +_IRQL_raises_(irql) //The function exits at the specified irql, but it can only be called to raise (not lower) the current IRQL. +_IRQL_saves_ //The annotated parameter saves the current IRQL to restore later. +_IRQL_restores_ //The annotated parameter contains an IRQL value from IRQL_saves that is to be restored when the function returns. +_IRQL_saves_global_(kind, param) //The current IRQL is saved into a location that is internal to the code analysis tools from which the IRQL is to be restored. This annotation is used to annotate a function. The location is identified by kind and further refined by param. For example, OldIrql could be the kind, and FastMutex could be the parameter that held that old IRQL value. +_IRQL_restores_global_(kind, param) //The IRQL saved by the function annotated with IRQL_saves_global is restored from a location that is internal to the Code Analysis tools. +_IRQL_always_function_min_(value) //The IRQL value is the minimum value to which the function can lower the IRQL. +_IRQL_always_function_max_(value) //The IRQL value is the maximum value to which the function can raise the IRQL. +_IRQL_requires_same_ //The annotated function must enter and exit at the same IRQL. The function can change the IRQL, but it must restore the IRQL to its original value before exiting. +_IRQL_uses_cancel_ //The annotated parameter is the IRQL value that should be restored by a DRIVER_CANCEL callback function. In most cases, use the IRQL_is_cancel annotation instead. +*/ + +/* +Call a function which should always be min DISPATCH_LEVEL but takes in an argument set to APC_LEVEL +*/ +_IRQL_always_function_min_(DISPATCH_LEVEL) + VOID IrqlMinDispatchLowerIrql_fail(KIRQL *oldIRQL) +{ + KeLowerIrql(*oldIRQL); +} + +VOID IrqlLowerWithFunctionCall(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(APC_LEVEL, &oldIRQL); + IrqlMinDispatchLowerIrql_fail(&oldIRQL); +} + +/* +Call a function which should always be min DISPATCH_LEVEL but takes in an argument set to APC_LEVEL +*/ +_IRQL_always_function_min_(DISPATCH_LEVEL) + VOID IrqlMinDispatchLowerIrql_fail1(KIRQL *oldIRQL) +{ + KeLowerIrql(*oldIRQL); +} + +VOID IrqlLowerWithFunctionCall1(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(APC_LEVEL, &oldIRQL); + KeRaiseIrql(DISPATCH_LEVEL, &oldIRQL); // oldIRQL should be APC_LEVEL + IrqlMinDispatchLowerIrql_fail1(&oldIRQL); +} + + +/* +Call a function which should always be min APC_LEVEL that takes in an argument set to DISPATCH_LEVEL +*/ +_IRQL_always_function_min_(APC_LEVEL) + VOID IrqlMinAPCLowerIrql(KIRQL *oldIRQL) +{ + KeLowerIrql(*oldIRQL); +} + +VOID IrqlLowerWithFunctionCall_pass(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(APC_LEVEL, &oldIRQL); + KeRaiseIrql(DISPATCH_LEVEL, &oldIRQL); // oldIRQL should be APC_LEVEL + IrqlMinAPCLowerIrql(&oldIRQL); +} + +/* Set IRQL to PASSIVE_LEVEL inside function which is min APC_LEVEL*/ +_IRQL_always_function_min_(APC_LEVEL) + VOID IrqlAlwaysMinAPC_fail(KIRQL *oldIRQL) +{ + KeLowerIrql(*oldIRQL); // lowers to PASSIVE_LEVEL which is lower than APC_LEVEL +} + +VOID IrqlCallAlwaysMinAPC(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(APC_LEVEL, &oldIRQL); + IrqlAlwaysMinAPC_fail(&oldIRQL); // oldIRQL should be PASSIVE_LEVEL +} + +_IRQL_requires_same_ + VOID + IrqlReqSame_pass(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(APC_LEVEL, &oldIRQL); + KeLowerIrql(oldIRQL); +} + +/* +Requires same but lowers IRQL +*/ +_IRQL_requires_same_ + VOID + IrqlReqSame_fail(KIRQL *oldIRQL) +{ + KeLowerIrql(*oldIRQL); +} + +VOID IrqlCallReqSame(void) +{ + KIRQL oldIRQL; + KeRaiseIrql(APC_LEVEL, &oldIRQL); + IrqlReqSame_fail(&oldIRQL); // oldIRQL should be APC_LEVEL +} diff --git a/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.ql b/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.ql index afd1a25a..fbec0213 100644 --- a/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.ql +++ b/src/drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.ql @@ -32,6 +32,7 @@ where or irqlFunc.(IrqlRequiresAnnotatedFunction).getIrqlLevel() = irqlRequirement ) and + irqlRequirement != -1 and irqlRequirement < min(getPotentialExitIrqlAtCfn(prior)) select call, "$@: IRQL potentially too high at call to $@. Maximum IRQL for this call: " + irqlRequirement + diff --git a/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.ql b/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.ql index 9b42ad7a..e0234179 100644 --- a/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.ql +++ b/src/drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.ql @@ -32,6 +32,7 @@ where or irqlFunc.(IrqlRequiresAnnotatedFunction).getIrqlLevel() = irqlRequirement ) and + irqlRequirement != -1 and irqlRequirement > max(getPotentialExitIrqlAtCfn(prior)) select call, "$@: IRQL potentially too low at call to $@. Minimum IRQL for this call: " + irqlRequirement + diff --git a/src/drivers/libraries/Irql.qll b/src/drivers/libraries/Irql.qll index 5d335316..a9106463 100644 --- a/src/drivers/libraries/Irql.qll +++ b/src/drivers/libraries/Irql.qll @@ -1,8 +1,21 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +/** + * Provides classes related to calculating and estimating the IRQL in a Windows device driver. + * + * For best results, this library expects to be used in tandem with IRQL annotations. A limited + * amount of functionality is still present even when no annotations are present, primarily around + * measuring direct calls to KeRaiseIrql and KeLowerIrql. + * + * Much of this library's analysis is intraprocedural or limited interprocedural, using a simple + * analysis based on call sites to a given function. Full interprocedural analysis that relies on the + * implicit behaviors of the WDM driver model, etc. is not yet supported. + */ + import cpp import drivers.libraries.SAL import drivers.wdm.libraries.WdmDrivers +import drivers.libraries.IrqlDataFlow /** * A macro in wdm.h that represents an IRQL level, @@ -60,21 +73,28 @@ class IrqlRestoresGlobalAnnotation extends SALAnnotation { } } -/** Standard IRQL annotations which apply to entire functions and manipulate or constrain the IRQL. */ +/** + * Standard IRQL annotations which apply to entire functions and manipulate or constrain the IRQL. + */ class IrqlFunctionAnnotation extends SALAnnotation { string irqlLevel; string irqlAnnotationName; - MacroInvocation irqlMacroInvocation; IrqlFunctionAnnotation() { - this.getMacroName() - .matches([ - "_IRQL_requires_", "_IRQL_requires_min_", "_IRQL_requires_max_", "_IRQL_raises_", - "_IRQL_saves_" - ]) and - irqlAnnotationName = this.getMacroName() and - irqlMacroInvocation.getParentInvocation() = this and - irqlLevel = irqlMacroInvocation.getMacro().getHead() + ( + this.getMacroName() + .matches([ + "_IRQL_requires_", "_IRQL_requires_min_", "_IRQL_requires_max_", "_IRQL_raises_", + "_IRQL_always_function_max_", "_IRQL_always_function_min_" + ]) and + irqlLevel = this.getUnexpandedArgument(0) + or + // Special case: _IRQL_saves_ annotations can apply to a whole function, + // but do not have an associated IRQL value. + this.getMacroName().matches("_IRQL_saves_") and + irqlLevel = "NA_IRQL_SAVES" + ) and + irqlAnnotationName = this.getMacroName() } /** Returns the raw text of the IRQL value used in this annotation. */ @@ -83,18 +103,30 @@ class IrqlFunctionAnnotation extends SALAnnotation { /** Returns the text of this annotation (i.e. \_IRQL\_requires\_, etc.) */ string getIrqlMacroName() { result = irqlAnnotationName } - /** Evaluate the IRQL specified in this annotation, if possible. */ + /** + * Evaluate the IRQL specified in this annotation, if possible. + * + * This will return -1 if the IRQL specified is anything other than a standard + * IRQL level (i.e. PASSIVE_LEVEL). This includes statements like "DPC_LEVEL - 1". + */ int getIrqlLevel() { - if exists(IrqlMacro im | im.getHead().matches(this.getIrqlLevelString())) - then - result = - any(int i | - exists(IrqlMacro im | - im.getIrqlLevel() = i and - im.getHead().matches(this.getIrqlLevelString()) + // Special case for DPC_LEVEL, which is not defined normally + if this.getIrqlLevelString().matches("DPC_LEVEL") + then result = 2 + else + if exists(IrqlMacro im | im.getHead().matches(this.getIrqlLevelString())) + then + result = + any(int i | + exists(IrqlMacro im | + im.getIrqlLevel() = i and + im.getHead().matches(this.getIrqlLevelString()) + ) ) - ) - else result = -1 + else + if exists(int i | i = this.getIrqlLevelString().toInt()) + then result = this.getIrqlLevelString().toInt() + else result = -1 } } @@ -130,6 +162,14 @@ class IrqlRequiresAnnotation extends IrqlFunctionAnnotation { IrqlRequiresAnnotation() { this.getMacroName().matches("_IRQL_requires_") } } +class IrqlAlwaysMaxAnnotation extends IrqlFunctionAnnotation { + IrqlAlwaysMaxAnnotation() { this.getMacroName().matches("_IRQL_always_function_max_") } +} + +class IrqlAlwaysMinAnnotation extends IrqlFunctionAnnotation { + IrqlAlwaysMinAnnotation() { this.getMacroName().matches("_IRQL_always_function_min_") } +} + /** * A SAL annotation indicating that the parameter in * question is used to store or restore the IRQL. @@ -252,6 +292,18 @@ class IrqlRaisesAnnotatedFunction extends IrqlRestrictsFunction, IrqlChangesFunc int getIrqlLevel() { result = irqlAnnotation.(IrqlRaisesAnnotation).getIrqlLevel() } } +class IrqlAlwaysMaxFunction extends IrqlRestrictsFunction { + IrqlAlwaysMaxFunction() { irqlAnnotation instanceof IrqlAlwaysMaxAnnotation } + + int getIrqlLevel() { result = irqlAnnotation.(IrqlAlwaysMaxAnnotation).getIrqlLevel() } +} + +class IrqlAlwaysMinFunction extends IrqlRestrictsFunction { + IrqlAlwaysMinFunction() { irqlAnnotation instanceof IrqlAlwaysMinAnnotation } + + int getIrqlLevel() { result = irqlAnnotation.(IrqlAlwaysMinAnnotation).getIrqlLevel() } +} + /** A function annotated to save the IRQL at the specified location upon entry. */ class IrqlSavesGlobalAnnotatedFunction extends IrqlChangesFunction { IrqlSavesGlobalAnnotation irqlAnnotation; @@ -418,11 +470,16 @@ class KeLowerIrqlCall extends FunctionCall { * "the IRQL before the most recent KeRaiseIrql call". */ int getIrqlLevel() { - result = any(getPotentialExitIrqlAtCfn(this.getMostRecentRaise().getAPredecessor())) + result = + any(getPotentialExitIrqlAtCfn(this.getMostRecentRaiseInterprocedural().getAPredecessor())) } /** - * Get the most recent call before this call that explicitly raised the IRQL. + * Get the most recent KeRaiseIrql call before this call. + * + * This performs a local (intraprocedural) analysis only. It is unused in the library today, + * but can be inserted in place of the interprocedural analysis by modifying the getIrqlLevel() + * function above. */ KeRaiseIrqlCall getMostRecentRaise() { result = @@ -431,6 +488,20 @@ class KeLowerIrqlCall extends FunctionCall { not exists(KeRaiseIrqlCall kric2 | kric2 != sgic and kric2.getAPredecessor*() = sgic) ) } + + /** + * Get the corresponding KeRaiseIrql call that preceded this KeLowerIrql call. + * + * This performs an interprocedural analysis using CodeQL's DataFlow classes. + */ + KeRaiseIrqlCall getMostRecentRaiseInterprocedural() { + result = + any(KeRaiseIrqlCall kric | + exists(IrqlRaiseLowerFlow irlf | + irlf.hasFlow(DataFlow::exprNode(kric), DataFlow::exprNode(this.getAnArgument())) + ) + ) + } } /** A call to a function that restores the IRQL from a specified state. */ @@ -450,7 +521,11 @@ class RestoresGlobalIrqlCall extends FunctionCall { result = any(getPotentialExitIrqlAtCfn(this.getMostRecentRaise().getAPredecessor())) } - /** Returns the matching call to a function that saved the IRQL to a global state. */ + /** + * Returns the matching call to a function that saved the IRQL to a global state. + * + * This is a strictly intraprocedural analysis. + */ SavesGlobalIrqlCall getMostRecentRaise() { result = any(SavesGlobalIrqlCall sgic | @@ -507,12 +582,13 @@ private predicate exprsMatchText(Expr e1, Expr e2) { * - If calling a function annotated to maintain the same IRQL, then the result is the IRQL at the previous CFN. * - If this node is calling a function with no annotations, the result is the IRQL that function exits at. * - If there is a prior CFN in the CFG, the result is the result for that prior CFN. - * - If there is no prior CFN, then the result is whatever the IRQL was at a statement prior to a function call to this function. - * - If there are no prior CFNs and no calls to this function, then the IRQL is determined by annotations. - * - If there is nothing else, then IRQL is 0. + * - If there is no prior CFN, then the result is whatever the IRQL was at a statement prior to a function call to this function (a lazy interprocedural analysis.) + * - If there are no prior CFNs and no calls to this function, then the IRQL is determined by annotations applied to this function. + * - Failing all this, we set the IRQL to 0. * * Not implemented: _IRQL_limited_to_ */ +pragma[assume_small_delta] cached int getPotentialExitIrqlAtCfn(ControlFlowNode cfn) { if cfn instanceof KeRaiseIrqlCall @@ -580,6 +656,8 @@ private ControlFlowNode getExitPointsOfFunction(Function f) { /** * Attempt to find the range of valid IRQL values when **entering** a given IRQL-annotated function. + * + * Note: we implicitly apply DISPATCH_LEVEL as the max when a max is not specified. */ cached int getAllowableIrqlLevel(IrqlRestrictsFunction irqlFunc) { @@ -600,12 +678,8 @@ int getAllowableIrqlLevel(IrqlRestrictsFunction irqlFunc) { then result = any([0 .. irqlFunc.(IrqlMaxAnnotatedFunction).getIrqlLevel()]) else if irqlFunc instanceof IrqlMinAnnotatedFunction - then - result = - any([irqlFunc.(IrqlMinAnnotatedFunction).getIrqlLevel() .. any(IrqlMacro im) - .getGlobalMaxIrqlLevel()] - ) + then result = any([irqlFunc.(IrqlMinAnnotatedFunction).getIrqlLevel() .. 2]) else - // No known restriction - result = any([0 .. any(IrqlMacro im).getGlobalMaxIrqlLevel()]) + // No known restriction. Default to between PASSIVE and DISPATCH. + result = any([0 .. 2]) } diff --git a/src/drivers/libraries/IrqlDataFlow.qll b/src/drivers/libraries/IrqlDataFlow.qll new file mode 100644 index 00000000..bf32d1bf --- /dev/null +++ b/src/drivers/libraries/IrqlDataFlow.qll @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +/** + * Provides data-flow analyses used in calculating the IRQL in a Windows driver. + */ + + +import cpp +import drivers.libraries.Irql +import semmle.code.cpp.dataflow.new.DataFlow + +/** + * A data-flow configuration describing flow from a + * KeRaiseIrqlCall to a KeLowerIrqlCall. + */ +class IrqlRaiseLowerFlow extends DataFlow::Configuration { + IrqlRaiseLowerFlow() { this = "IrqlRaiseLowerFlow" } + + override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof KeRaiseIrqlCall } + + override predicate isSink(DataFlow::Node sink) { + exists(KeLowerIrqlCall firf | + sink.asExpr() = firf.getArgument(firf.getTarget().(IrqlRestoreFunction).getIrqlIndex()) + ) + } +} + +/** + * A function that has at least one parameter annotated with "\_IRQL\_restores\_". + */ +class IrqlRestoreFunction extends Function { + Parameter p; + int irqlIndex; + + IrqlRestoreFunction() { + p = this.getParameter(irqlIndex) and + p instanceof IrqlRestoreParameter + } + + Parameter getRestoreParameter() { result = p } + + int getIrqlIndex() { result = irqlIndex } +} \ No newline at end of file diff --git a/src/drivers/test/build_create_analyze_test.cmd b/src/drivers/test/build_create_analyze_test.cmd index 9859d885..02ec00ae 100644 --- a/src/drivers/test/build_create_analyze_test.cmd +++ b/src/drivers/test/build_create_analyze_test.cmd @@ -1,3 +1,8 @@ +rd /s /q working >NUL 2>&1 +rd /s /q TestDB >NUL 2>&1 +rd /s /q AnalysisFiles >NUL 2>&1 + + call :test PendingStatusError WDMTestTemplate wdm queries call :test ExaminedValue WDMTestTemplate wdm queries call :test StrSafe KMDFTestTemplate kmdf queries @@ -9,6 +14,8 @@ call :test OpaqueMdlWrite WDMTestTemplate wdm queries call :test KeWaitLocal WDMTestTemplate wdm queries call :test IrqlTooHigh WDMTestTemplate general queries\experimental call :test IrqlTooLow WDMTestTemplate general queries\experimental +call :test IrqlSetTooHigh WDMTestTemplate general queries\experimental +call :test IrqlSetTooLow WDMTestTemplate general queries\experimental call :test WrongDispatchTableAssignment WDMTestTemplate wdm queries call :test ExtendedDeprecatedApis WDMTestTemplate general queries call :test WdkDeprecatedApis WDMTestTemplate general queries diff --git a/src/drivers/test/diff/IrqlSetTooHigh.sarif b/src/drivers/test/diff/IrqlSetTooHigh.sarif new file mode 100644 index 00000000..dea8275e --- /dev/null +++ b/src/drivers/test/diff/IrqlSetTooHigh.sarif @@ -0,0 +1,21 @@ +{ + "all": { + "+": 0, + "-": 0 + }, + "error": { + "+": 0, + "-": 0, + "codes": [] + }, + "warning": { + "+": 0, + "-": 0, + "codes": [] + }, + "note": { + "+": 0, + "-": 0, + "codes": [] + } +} \ No newline at end of file diff --git a/src/drivers/test/diff/IrqlSetTooLow.sarif b/src/drivers/test/diff/IrqlSetTooLow.sarif new file mode 100644 index 00000000..dea8275e --- /dev/null +++ b/src/drivers/test/diff/IrqlSetTooLow.sarif @@ -0,0 +1,21 @@ +{ + "all": { + "+": 0, + "-": 0 + }, + "error": { + "+": 0, + "-": 0, + "codes": [] + }, + "warning": { + "+": 0, + "-": 0, + "codes": [] + }, + "note": { + "+": 0, + "-": 0, + "codes": [] + } +} \ No newline at end of file diff --git a/src/suites/ported_driver_ca_checks.qls b/src/suites/ported_driver_ca_checks.qls index c4a43c99..1a9058b4 100644 --- a/src/suites/ported_driver_ca_checks.qls +++ b/src/suites/ported_driver_ca_checks.qls @@ -15,6 +15,8 @@ - drivers/kmdf/queries/StrSafe/StrSafe.ql - drivers/general/queries/experimental/IrqlTooHigh/IrqlTooHigh.ql - drivers/general/queries/experimental/IrqlTooLow/IrqlTooLow.ql + - drivers/general/queries/experimental/IrqlSetTooHigh/IrqlSetTooHigh.ql + - drivers/general/queries/experimental/IrqlSetTooLow/IrqlSetTooLow.ql - drivers/wdm/queries/ExaminedValue/ExaminedValue.ql - drivers/wdm/queries/IllegalFieldAccess/IllegalFieldAccess.ql - drivers/wdm/queries/IllegalFieldAccess2/IllegalFieldAccess2.ql From c4bf4f84ffdf930653ee2aebe15f803cfdbfdb37 Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Mon, 20 Nov 2023 18:47:50 -0800 Subject: [PATCH 10/11] Add KeSetEventIrql and KeSetEventPaged queries and update IRQL library. (#90) * WIP: query to find improper use of KeSetEvent * WIP: query to find improper use of KeSetEvent * Refactor KeSetEvent query to run significantly faster. * Create KeSetEventIrql and KeSetEventPaged queries. Also make various updates + fixes to the IRQL model. * Fix typo in debug library. * Improve string matching for IRQL The previous .matches() clauses were using "_" as wildcards rather than as literal underscores. While this didn't affect our results directly because of other restrictions we provide, it had some performance penalty. Rather than try and escape all the underscores (which would require lots of ugly double backslashes) we just switch to the = syntax where possible, which is similarly performant. * Remove test predicate left in by accident. * Cleanup imports and move KeSetEventPaged out of experimental * Add KeSetEventPaged to the Recommended suite. --------- Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Signed-off-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> Co-authored-by: jacob-ronstadt Co-authored-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> --- .../KeSetEventPaged/KeSetEventPaged.ql | 36 +++ .../KeSetEventPaged/KeSetEventPaged.qlhelp | 23 ++ .../KeSetEventPaged/KeSetEventPaged.sarif | 265 ++++++++++++++++++ .../queries/KeSetEventPaged/driver_snippet.c | 58 ++++ .../KeSetEventIrql/KeSetEventIrql.ql | 47 ++++ .../KeSetEventIrql/KeSetEventIrql.qlhelp | 23 ++ .../KeSetEventIrql/KeSetEventIrql.sarif | 264 +++++++++++++++++ .../KeSetEventIrql/driver_snippet.c | 60 ++++ src/drivers/libraries/Irql.qll | 216 ++++++++++---- src/drivers/libraries/IrqlDebug.qll | 63 +++++ .../test/build_create_analyze_test.cmd | 2 + src/drivers/test/diff/KeSetEventIrql.sarif | 21 ++ src/drivers/test/diff/KeSetEventPaged.sarif | 21 ++ src/suites/windows_driver_recommended.qls | 3 +- 14 files changed, 1040 insertions(+), 62 deletions(-) create mode 100644 src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.ql create mode 100644 src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.qlhelp create mode 100644 src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.sarif create mode 100644 src/drivers/general/queries/KeSetEventPaged/driver_snippet.c create mode 100644 src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.ql create mode 100644 src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.qlhelp create mode 100644 src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.sarif create mode 100644 src/drivers/general/queries/experimental/KeSetEventIrql/driver_snippet.c create mode 100644 src/drivers/libraries/IrqlDebug.qll create mode 100644 src/drivers/test/diff/KeSetEventIrql.sarif create mode 100644 src/drivers/test/diff/KeSetEventPaged.sarif diff --git a/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.ql b/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.ql new file mode 100644 index 00000000..e383301f --- /dev/null +++ b/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.ql @@ -0,0 +1,36 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +/** + * @id cpp/drivers/ke-set-event-irql + * @name KeSetEvent called in paged segment with wait + * @description Calles to KeSetEvent in a paged segment must not call with the Wait parameter set to true. This can cause a system crash if the segment is paged out. + * @platform Desktop + * @security.severity Low + * @feature.area Multiple + * @impact Exploitable Design + * @repro.text The following call to KeSetEvent has Wait set to true while in a paged segment. + * @owner.email sdat@microsoft.com + * @opaqueid CQLD-D0004 + * @kind problem + * @problem.severity error + * @precision high + * @tags correctness + * wddst + * @scope domainspecific + * @query-version v1 + */ + +import cpp +import drivers.libraries.Page + +class KeSetEventCall extends FunctionCall { + KeSetEventCall() { this.getTarget().getName().matches("KeSetEvent") } +} + +from KeSetEventCall ksec, PagedFunctionDeclaration enclosingFunc +where + enclosingFunc = ksec.getEnclosingFunction() and + ksec.getArgument(2).getValue() = "1" +select ksec, + "$@: KeSetEvent should not be called with the Wait parameter set to true when in a paged function.", + ksec.getControlFlowScope(), ksec.getControlFlowScope().getQualifiedName() diff --git a/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.qlhelp b/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.qlhelp new file mode 100644 index 00000000..af790f27 --- /dev/null +++ b/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.qlhelp @@ -0,0 +1,23 @@ + + + +

    + KeSetEvent must not be called in a paged segment when the Wait argument is set to TRUE. This can cause a system crash the segment is paged out. +

    +     + +

    + Adjust the KeSetEvent call to pass FALSE to the wait parameter. +

    +     + + + + +
  • + + KeSetEvent (MSDN) + +
    • +     +     diff --git a/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.sarif b/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.sarif new file mode 100644 index 00000000..26e196eb --- /dev/null +++ b/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.sarif @@ -0,0 +1,265 @@ +{ + "$schema" : "https://json.schemastore.org/sarif-2.1.0.json", + "version" : "2.1.0", + "runs" : [ { + "tool" : { + "driver" : { + "name" : "CodeQL", + "organization" : "GitHub", + "semanticVersion" : "2.15.1", + "notifications" : [ { + "id" : "cpp/baseline/expected-extracted-files", + "name" : "cpp/baseline/expected-extracted-files", + "shortDescription" : { + "text" : "Expected extracted files" + }, + "fullDescription" : { + "text" : "Files appearing in the source archive that are expected to be extracted." + }, + "defaultConfiguration" : { + "enabled" : true + }, + "properties" : { + "tags" : [ "expected-extracted-files", "telemetry" ] + } + } ], + "rules" : [ { + "id" : "cpp/drivers/ke-set-event-irql", + "name" : "cpp/drivers/ke-set-event-irql", + "shortDescription" : { + "text" : "KeSetEvent called in paged segment with wait" + }, + "fullDescription" : { + "text" : "Calles to KeSetEvent in a paged segment must not call with the Wait parameter set to true. This can cause a system crash if the segment is paged out." + }, + "defaultConfiguration" : { + "enabled" : true, + "level" : "warning" + }, + "properties" : { + "tags" : [ "correctness", "wddst" ], + "description" : "Calles to KeSetEvent in a paged segment must not call with the Wait parameter set to true. This can cause a system crash if the segment is paged out.", + "feature.area" : "Multiple", + "id" : "cpp/drivers/ke-set-event-irql", + "impact" : "Exploitable Design", + "kind" : "problem", + "name" : "KeSetEvent called in paged segment with wait", + "opaqueid" : "CQLD-D0004", + "owner.email" : "sdat@microsoft.com", + "platform" : "Desktop", + "precision" : "medium", + "problem.severity" : "warning", + "query-version" : "v1", + "repro.text" : "The following call to KeSetEvent has Wait set to true while in a paged segment.", + "scope" : "domainspecific", + "security.severity" : "Low" + } + } ] + }, + "extensions" : [ { + "name" : "microsoft/windows-drivers", + "semanticVersion" : "0.2.0+234ee9b709196a3a802b2c02ad7945ba0dfb0ac0", + "locations" : [ { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", + "description" : { + "text" : "The QL pack root directory." + } + }, { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/qlpack.yml", + "description" : { + "text" : "The QL pack definition file." + } + } ] + } ] + }, + "invocations" : [ { + "toolExecutionNotifications" : [ { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + } ], + "executionSuccessful" : true + } ], + "artifacts" : [ { + "location" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } ], + "results" : [ { + "ruleId" : "cpp/drivers/ke-set-event-irql", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/ke-set-event-irql", + "index" : 0 + }, + "message" : { + "text" : "[KeSetEventIrql_Fail2](1): KeSetEvent should not be called with the Wait parameter set to true when in a paged function." + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 40, + "startColumn" : 5, + "endColumn" : 15 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "1853d3bfdff8bc5c:2", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 36, + "startColumn" : 6, + "endColumn" : 26 + } + }, + "message" : { + "text" : "KeSetEventIrql_Fail2" + } + } ] + }, { + "ruleId" : "cpp/drivers/ke-set-event-irql", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/ke-set-event-irql", + "index" : 0 + }, + "message" : { + "text" : "[KeSetEventIrql_Fail1](1): KeSetEvent should not be called with the Wait parameter set to true when in a paged function." + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 33, + "startColumn" : 5, + "endColumn" : 15 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "1853d3bfdff8bc5c:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 29, + "startColumn" : 6, + "endColumn" : 26 + } + }, + "message" : { + "text" : "KeSetEventIrql_Fail1" + } + } ] + } ], + "columnKind" : "utf16CodeUnits", + "properties" : { + "semmle.formatSpecifier" : "sarifv2.1.0" + } + } ] +} \ No newline at end of file diff --git a/src/drivers/general/queries/KeSetEventPaged/driver_snippet.c b/src/drivers/general/queries/KeSetEventPaged/driver_snippet.c new file mode 100644 index 00000000..9f9ad7d0 --- /dev/null +++ b/src/drivers/general/queries/KeSetEventPaged/driver_snippet.c @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +// driver_snippet.c +// + +#define SET_DISPATCH 1 + +// Template. Not called in this test. +void top_level_call() {} + +#include + +void KeSetEventIrql_Fail1(PRKEVENT Event); + +_IRQL_always_function_min_(APC_LEVEL) +void KeSetEventIrql_Fail2(PRKEVENT Event); + +_IRQL_always_function_min_(PASSIVE_LEVEL) +void KeSetEventIrql_Pass1(PRKEVENT Event); + +_IRQL_always_function_min_(PASSIVE_LEVEL) +void KeSetEventIrql_Pass2(PRKEVENT Event); + +#pragma alloc_text(PAGE, KeSetEventIrql_Fail1) +#pragma alloc_text(PAGE, KeSetEventIrql_Fail2) +#pragma alloc_text(PAGE, KeSetEventIrql_Pass2) + +void KeSetEventIrql_Fail1(PRKEVENT Event) +{ + // This is a paged function. We assume a lower limit of PASSIVE_LEVEL and an upper limit of APC_LEVEL on the IRQL. + + KeSetEvent(Event, HIGH_PRIORITY, TRUE); // ERROR: Calling with wait set to TRUE in a pageable context +} + +void KeSetEventIrql_Fail2(PRKEVENT Event) +{ + // This is a paged function. Even though it runs at APC_LEVEL, not PASSIVE_LEVEL, that's still an error. + + KeSetEvent(Event, HIGH_PRIORITY, TRUE); // ERROR: Calling with wait set to TRUE in a pageable context +} + +void KeSetEventIrql_Pass1(PRKEVENT Event) +{ + // This function will potentially run at passive level but it's not pageable, so there's no issue. + + KeSetEvent(Event, HIGH_PRIORITY, TRUE); +} + +void KeSetEventIrql_Pass2(PRKEVENT Event) +{ + // This function will runs at passive level and is pageable, but correctly uses FALSE in its call to KeSetEvent. + + KeSetEvent(Event, HIGH_PRIORITY, FALSE); +} + +// TODO multi-threaded tests +// function has max IRQL requirement, creates two threads where one is above that requirement and one is below diff --git a/src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.ql b/src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.ql new file mode 100644 index 00000000..ca3f136c --- /dev/null +++ b/src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.ql @@ -0,0 +1,47 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +/** + * @id cpp/drivers/ke-set-event-irql + * @name KeSetEvent called at wrong IRQL + * @description KeSetEvent must be called at DISPATCH_LEVEL or below. If the Wait argument is set to TRUE, it must be called at APC_LEVEL or below. + * @platform Desktop + * @security.severity Low + * @feature.area Multiple + * @impact Exploitable Design + * @repro.text The following call to KeSetEvent occurs at too high of an IRQL. + * @owner.email sdat@microsoft.com + * @opaqueid CQLD-D0005 + * @kind problem + * @problem.severity warning + * @precision medium + * @tags correctness + * wddst + * @scope domainspecific + * @query-version v1 + */ + +import cpp +import drivers.libraries.Irql +import drivers.libraries.Page + +class KeSetEventCall extends FunctionCall { + KeSetEventCall() { this.getTarget().getName().matches("KeSetEvent") } +} + +from KeSetEventCall ksec, string message +where + ksec.getArgument(2).getValue() = "0" and + not exists(int i | + i = [0 .. 2] and + getPotentialExitIrqlAtCfn(ksec.getAPredecessor()) = i + ) and + message = "KeSetEvent should not be called above DISPATCH_LEVEL when Wait is set to false." + or + ksec.getArgument(2).getValue() = "1" and + not exists(int i | + i = [0 .. 1] and + getPotentialExitIrqlAtCfn(ksec.getAPredecessor()) = i + ) and + message = "KeSetEvent should not be called at or above DISPATCH_LEVEL when Wait is set to true." +select ksec, "$@: " + message, ksec.getControlFlowScope(), + ksec.getControlFlowScope().getQualifiedName() diff --git a/src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.qlhelp b/src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.qlhelp new file mode 100644 index 00000000..079526ce --- /dev/null +++ b/src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.qlhelp @@ -0,0 +1,23 @@ + + + +

    + KeSetEvent must be called at DISPATCH_LEVEL or below. If the Wait argument is set to TRUE, it must be called at APC_LEVEL or below. Failure to follow these guidelines can lead to system crashes. +

    +     + +

    + Ensure that the IRQL at this statement is low enough. If you are calling with Wait set to TRUE, consider setting it to FALSE instead. +

    +     + + + + +
  • + + KeSetEvent (MSDN) + +
    • +     +     diff --git a/src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.sarif b/src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.sarif new file mode 100644 index 00000000..2eac8126 --- /dev/null +++ b/src/drivers/general/queries/experimental/KeSetEventIrql/KeSetEventIrql.sarif @@ -0,0 +1,264 @@ +{ + "$schema" : "https://json.schemastore.org/sarif-2.1.0.json", + "version" : "2.1.0", + "runs" : [ { + "tool" : { + "driver" : { + "name" : "CodeQL", + "organization" : "GitHub", + "semanticVersion" : "2.15.1", + "notifications" : [ { + "id" : "cpp/baseline/expected-extracted-files", + "name" : "cpp/baseline/expected-extracted-files", + "shortDescription" : { + "text" : "Expected extracted files" + }, + "fullDescription" : { + "text" : "Files appearing in the source archive that are expected to be extracted." + }, + "defaultConfiguration" : { + "enabled" : true + }, + "properties" : { + "tags" : [ "expected-extracted-files", "telemetry" ] + } + } ], + "rules" : [ { + "id" : "cpp/drivers/ke-set-event-irql", + "name" : "cpp/drivers/ke-set-event-irql", + "shortDescription" : { + "text" : "KeSetEvent called at wrong IRQL" + }, + "fullDescription" : { + "text" : "KeSetEvent must be called at DISPATCH_LEVEL or below. If the Wait argument is set to TRUE, it must be called at APC_LEVEL or below." + }, + "defaultConfiguration" : { + "enabled" : true, + "level" : "warning" + }, + "properties" : { + "tags" : [ "correctness", "wddst" ], + "description" : "KeSetEvent must be called at DISPATCH_LEVEL or below. If the Wait argument is set to TRUE, it must be called at APC_LEVEL or below.", + "feature.area" : "Multiple", + "id" : "cpp/drivers/ke-set-event-irql", + "impact" : "Exploitable Design", + "kind" : "problem", + "name" : "KeSetEvent called at wrong IRQL", + "opaqueid" : "CQLD-D0005", + "owner.email" : "sdat@microsoft.com", + "platform" : "Desktop", + "precision" : "medium", + "problem.severity" : "warning", + "query-version" : "v1", + "repro.text" : "The following call to KeSetEvent occurs at too high of an IRQL.", + "scope" : "domainspecific", + "security.severity" : "Low" + } + } ] + }, + "extensions" : [ { + "name" : "microsoft/windows-drivers", + "semanticVersion" : "0.2.0+234ee9b709196a3a802b2c02ad7945ba0dfb0ac0", + "locations" : [ { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", + "description" : { + "text" : "The QL pack root directory." + } + }, { + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/qlpack.yml", + "description" : { + "text" : "The QL pack definition file." + } + } ] + } ] + }, + "invocations" : [ { + "toolExecutionNotifications" : [ { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + } ], + "executionSuccessful" : true + } ], + "artifacts" : [ { + "location" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } ], + "results" : [ { + "ruleId" : "cpp/drivers/ke-set-event-irql", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/ke-set-event-irql", + "index" : 0 + }, + "message" : { + "text" : "[KeSetEventIrql_Fail1](1): KeSetEvent should not be called above DISPATCH_LEVEL when Wait is set to false." + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 34, + "startColumn" : 5, + "endColumn" : 15 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "3f0c2c1bba9b015e:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 30, + "startColumn" : 6, + "endColumn" : 26 + } + }, + "message" : { + "text" : "KeSetEventIrql_Fail1" + } + } ] + }, { + "ruleId" : "cpp/drivers/ke-set-event-irql", + "ruleIndex" : 0, + "rule" : { + "id" : "cpp/drivers/ke-set-event-irql", + "index" : 0 + }, + "message" : { + "text" : "[CompletionRoutine](1): KeSetEvent should not be called at or above DISPATCH_LEVEL when Wait is set to true." + }, + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + }, + "region" : { + "startLine" : 324, + "startColumn" : 5, + "endColumn" : 15 + } + } + } ], + "partialFingerprints" : { + "primaryLocationLineHash" : "779dfb1bf8eb10c3:1", + "primaryLocationStartColumnFingerprint" : "0" + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + }, + "region" : { + "startLine" : 303, + "endColumn" : 18 + } + }, + "message" : { + "text" : "CompletionRoutine" + } + } ] + } ], + "columnKind" : "utf16CodeUnits", + "properties" : { + "semmle.formatSpecifier" : "sarifv2.1.0" + } + } ] +} \ No newline at end of file diff --git a/src/drivers/general/queries/experimental/KeSetEventIrql/driver_snippet.c b/src/drivers/general/queries/experimental/KeSetEventIrql/driver_snippet.c new file mode 100644 index 00000000..b42c6ced --- /dev/null +++ b/src/drivers/general/queries/experimental/KeSetEventIrql/driver_snippet.c @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +// driver_snippet.c +// + +#define SET_DISPATCH 1 + +// Template. Not called in this test. +void top_level_call() {} + +_IRQL_always_function_max_(HIGH_LEVEL) +_IRQL_always_function_min_(5) +void KeSetEventIrql_Fail1(PRKEVENT Event); + +_IRQL_always_function_min_(DISPATCH_LEVEL) +void KeSetEventIrql_Fail2(PRKEVENT Event); + +_IRQL_always_function_min_(PASSIVE_LEVEL) +void KeSetEventIrql_Pass1(PRKEVENT Event); + +_IRQL_always_function_min_(DISPATCH_LEVEL) +void KeSetEventIrql_Pass2(PRKEVENT Event); + +#pragma alloc_text(PAGE, KeSetEventIrql_Fail2) +#pragma alloc_text(PAGE, KeSetEventIrql_Pass2) + +#include + +void KeSetEventIrql_Fail1(PRKEVENT Event) +{ + // This function is running at a high IRQL. It should not call KeSetEvent. + + KeSetEvent(Event, HIGH_PRIORITY, FALSE); // Calling at too high of an IRQL +} + +void KeSetEventIrql_Fail2(PRKEVENT Event) +{ + // This function runs at DISPATCH_LEVEL, which is too high to call KeSetEvent with the Wait argument. + + KeSetEvent(Event, HIGH_PRIORITY, TRUE); // Calling at too high of an IRQL +} + +void KeSetEventIrql_Pass1(PRKEVENT Event) +{ + // This function runs at passive, so there's no problem. + + KeSetEvent(Event, HIGH_PRIORITY, TRUE); +} + +void KeSetEventIrql_Pass2(PRKEVENT Event) +{ + // This function runs at DISPATCH_LEVEL but is calling with Wait set to FALSE, so there is no issue. + + KeSetEvent(Event, HIGH_PRIORITY, FALSE); +} + +// TODO multi-threaded tests +// function has max IRQL requirement, creates two threads where one is above that requirement and one is below + diff --git a/src/drivers/libraries/Irql.qll b/src/drivers/libraries/Irql.qll index a9106463..c4b7c295 100644 --- a/src/drivers/libraries/Irql.qll +++ b/src/drivers/libraries/Irql.qll @@ -16,59 +16,76 @@ import cpp import drivers.libraries.SAL import drivers.wdm.libraries.WdmDrivers import drivers.libraries.IrqlDataFlow +import drivers.libraries.Page /** * A macro in wdm.h that represents an IRQL level, * such as PASSIVE_LEVEL, DISPATCH_LEVEL, etc. */ +cached class IrqlMacro extends Macro { int irqlLevelAsInt; + cached IrqlMacro() { this.getName().matches("%_LEVEL") and - this.getFile().getBaseName().matches("wdm.h") and + this.getFile().getBaseName() = "wdm.h" and this.getBody().toInt() = irqlLevelAsInt and irqlLevelAsInt >= 0 and irqlLevelAsInt <= 31 } /** Returns the integer value of this IRQL level. */ + cached int getIrqlLevel() { result = irqlLevelAsInt } +} - /** - * Returns the highest IRQL in wdm.h across this database. - * May cause incorrect results if database contains both 32-bit - * and 64-bit builds. - */ - int getGlobalMaxIrqlLevel() { - result = - any(int i | - exists(IrqlMacro im | - i = im.getIrqlLevel() and - not exists(IrqlMacro im2 | im2 != im and im2.getIrqlLevel() > im.getIrqlLevel()) - ) +/** + * Returns the highest IRQL in wdm.h across this database. + * May cause incorrect results if database contains both 32-bit + * and 64-bit builds. + */ +cached +int getGlobalMaxIrqlLevel() { + result = + any(int i | + exists(IrqlMacro im | + i = im.getIrqlLevel() and + not exists(IrqlMacro im2 | im2 != im and im2.getIrqlLevel() > im.getIrqlLevel()) ) - } + ) +} + +/** + * Represents a real (not -1) Irql level, between 0 and the max for the architecture this + * database was built to target. + */ +class IrqlValue extends int { + IrqlValue() { this = [0 .. getGlobalMaxIrqlLevel()] } } /** An \_IRQL\_saves\_global\_(parameter, kind) annotation. */ +cached class IrqlSavesGlobalAnnotation extends SALAnnotation { MacroInvocation irqlMacroInvocation; + cached IrqlSavesGlobalAnnotation() { // Needs to include other function and parameter annotations too - this.getMacroName().matches(["_IRQL_saves_global_"]) and + this.getMacroName() = ["__drv_savesIRQLGlobal", "_IRQL_saves_global_"] and irqlMacroInvocation.getParentInvocation() = this } } /** An \_IRQL\_restores\_global\_(parameter, kind) annotation. */ +cached class IrqlRestoresGlobalAnnotation extends SALAnnotation { MacroInvocation irqlMacroInvocation; + cached IrqlRestoresGlobalAnnotation() { // Needs to include other function and parameter annotations too - this.getMacroName().matches(["_IRQL_restores_global_"]) and + this.getMacroName() = ["__drv_restoresIRQLGlobal", "_IRQL_restores_global_"] and irqlMacroInvocation.getParentInvocation() = this } } @@ -76,31 +93,37 @@ class IrqlRestoresGlobalAnnotation extends SALAnnotation { /** * Standard IRQL annotations which apply to entire functions and manipulate or constrain the IRQL. */ +cached class IrqlFunctionAnnotation extends SALAnnotation { string irqlLevel; string irqlAnnotationName; + cached IrqlFunctionAnnotation() { ( - this.getMacroName() - .matches([ - "_IRQL_requires_", "_IRQL_requires_min_", "_IRQL_requires_max_", "_IRQL_raises_", - "_IRQL_always_function_max_", "_IRQL_always_function_min_" - ]) and + this.getMacroName() = + [ + "__drv_requiresIRQL", "_IRQL_requires_", "_drv_minIRQL", "_IRQL_requires_min_", + "_drv_maxIRQL", "_IRQL_requires_max_", "__drv_raisesIRQL", "_IRQL_raises_", + "__drv_maxFunctionIRQL", "_IRQL_always_function_max_", "__drv_minFunctionIRQL", + "_IRQL_always_function_min_" + ] and irqlLevel = this.getUnexpandedArgument(0) or // Special case: _IRQL_saves_ annotations can apply to a whole function, // but do not have an associated IRQL value. - this.getMacroName().matches("_IRQL_saves_") and + this.getMacroName() = ["__drv_savesIRQL", "_IRQL_saves_"] and irqlLevel = "NA_IRQL_SAVES" ) and irqlAnnotationName = this.getMacroName() } /** Returns the raw text of the IRQL value used in this annotation. */ + cached string getIrqlLevelString() { result = irqlLevel } /** Returns the text of this annotation (i.e. \_IRQL\_requires\_, etc.) */ + cached string getIrqlMacroName() { result = irqlAnnotationName } /** @@ -109,9 +132,10 @@ class IrqlFunctionAnnotation extends SALAnnotation { * This will return -1 if the IRQL specified is anything other than a standard * IRQL level (i.e. PASSIVE_LEVEL). This includes statements like "DPC_LEVEL - 1". */ + cached int getIrqlLevel() { // Special case for DPC_LEVEL, which is not defined normally - if this.getIrqlLevelString().matches("DPC_LEVEL") + if this.getIrqlLevelString() = "DPC_LEVEL" then result = 2 else if exists(IrqlMacro im | im.getHead().matches(this.getIrqlLevelString())) @@ -135,7 +159,7 @@ class IrqlSameAnnotation extends SALAnnotation { string irqlAnnotationName; IrqlSameAnnotation() { - this.getMacroName().matches(["_IRQL_requires_same_"]) and + this.getMacroName() = ["__drv_sameIRQL", "_IRQL_requires_same_"] and irqlAnnotationName = this.getMacroName() } @@ -144,30 +168,36 @@ class IrqlSameAnnotation extends SALAnnotation { /** An "\_IRQL\_requires\_max\_" annotation. */ class IrqlMaxAnnotation extends IrqlFunctionAnnotation { - IrqlMaxAnnotation() { this.getMacroName().matches("_IRQL_requires_max_") } + IrqlMaxAnnotation() { this.getMacroName() = ["_drv_maxIRQL", "_IRQL_requires_max_"] } } /** An "\_IRQL\_raises\_" annotation. */ class IrqlRaisesAnnotation extends IrqlFunctionAnnotation { - IrqlRaisesAnnotation() { this.getMacroName().matches("_IRQL_raises_") } + IrqlRaisesAnnotation() { this.getMacroName() = ["__drv_raisesIRQL", "_IRQL_raises_"] } } /** An "\_IRQL\_requires\_min\_" annotation. */ class IrqlMinAnnotation extends IrqlFunctionAnnotation { - IrqlMinAnnotation() { this.getMacroName().matches("_IRQL_requires_min_") } + IrqlMinAnnotation() { this.getMacroName() = ["_drv_minIRQL", "_IRQL_requires_min_"] } } /** An "\_IRQL\_requires\_" annotation. */ class IrqlRequiresAnnotation extends IrqlFunctionAnnotation { - IrqlRequiresAnnotation() { this.getMacroName().matches("_IRQL_requires_") } + IrqlRequiresAnnotation() { this.getMacroName() = ["__drv_requiresIRQL", "_IRQL_requires_"] } } +/** An "\_IRQL\_always\_function\_max\_" annotation. */ class IrqlAlwaysMaxAnnotation extends IrqlFunctionAnnotation { - IrqlAlwaysMaxAnnotation() { this.getMacroName().matches("_IRQL_always_function_max_") } + IrqlAlwaysMaxAnnotation() { + this.getMacroName() = ["__drv_maxFunctionIRQL", "_IRQL_always_function_max_"] + } } +/** An "\_IRQL\_always\_function\_min\_" annotation. */ class IrqlAlwaysMinAnnotation extends IrqlFunctionAnnotation { - IrqlAlwaysMinAnnotation() { this.getMacroName().matches("_IRQL_always_function_min_") } + IrqlAlwaysMinAnnotation() { + this.getMacroName() = ["__drv_minFunctionIRQL", "_IRQL_always_function_min_"] + } } /** @@ -178,7 +208,8 @@ class IrqlParameterAnnotation extends SALAnnotation { string irqlAnnotationName; IrqlParameterAnnotation() { - this.getMacroName().matches(["_IRQL_restores_", "_IRQL_saves_"]) and + this.getMacroName() = + ["__drv_restoresIRQL", "_IRQL_restores_", "__drv_savesIRQL", "_IRQL_saves_"] and irqlAnnotationName = this.getMacroName() and exists(MacroInvocation mi | mi.getParentInvocation() = this) } @@ -192,7 +223,7 @@ class IrqlParameterAnnotation extends SALAnnotation { * question contains an IRQL value that the system will be set to. */ class IrqlRestoreAnnotation extends IrqlParameterAnnotation { - IrqlRestoreAnnotation() { this.getMacroName().matches(["_IRQL_restores_"]) } + IrqlRestoreAnnotation() { this.getMacroName() = ["__drv_restoresIRQL", "_IRQL_restores_"] } } /** @@ -201,7 +232,7 @@ class IrqlRestoreAnnotation extends IrqlParameterAnnotation { * - If applied to a parameter, the function saves the IRQL to the parameter. */ class IrqlSaveAnnotation extends IrqlFunctionAnnotation { - IrqlSaveAnnotation() { this.getMacroName().matches(["_IRQL_saves_"]) } + IrqlSaveAnnotation() { this.getMacroName() = ["__drv_savesIRQL", "_IRQL_saves_"] } } /** A parameter that is annotated with "\_IRQL\_restores\_". */ @@ -292,12 +323,14 @@ class IrqlRaisesAnnotatedFunction extends IrqlRestrictsFunction, IrqlChangesFunc int getIrqlLevel() { result = irqlAnnotation.(IrqlRaisesAnnotation).getIrqlLevel() } } +/** A function that is never allowed to run with the IRQL above a given value. */ class IrqlAlwaysMaxFunction extends IrqlRestrictsFunction { IrqlAlwaysMaxFunction() { irqlAnnotation instanceof IrqlAlwaysMaxAnnotation } int getIrqlLevel() { result = irqlAnnotation.(IrqlAlwaysMaxAnnotation).getIrqlLevel() } } +/** A function that is never allowed to run with the IRQL above a given value. */ class IrqlAlwaysMinFunction extends IrqlRestrictsFunction { IrqlAlwaysMinFunction() { irqlAnnotation instanceof IrqlAlwaysMinAnnotation } @@ -446,16 +479,12 @@ class IrqlSaveCall extends FunctionCall { /** A call to a KeRaiseIRQL API that directly raises the IRQL. */ class KeRaiseIrqlCall extends FunctionCall { KeRaiseIrqlCall() { - this.getTarget() - .getName() - .matches(any([ - "KeRaiseIrql", "KfRaiseIrql", "KeRaiseIrqlToDPCLevel", "KfRaiseIrqlToDPCLevel" - ] - )) + this.getTarget().getName() = + ["KeRaiseIrql", "KfRaiseIrql", "KeRaiseIrqlToDPCLevel", "KfRaiseIrqlToDPCLevel"] } int getIrqlLevel() { - if this.getTarget().getName().matches(any(["KeRaiseIrqlToDPCLevel", "KfRaiseIrqlToDPCLevel"])) + if this.getTarget().getName() = ["KeRaiseIrqlToDPCLevel", "KfRaiseIrqlToDPCLevel"] then result = 2 else result = this.getArgument(0).(Literal).getValue().toInt() } @@ -463,7 +492,7 @@ class KeRaiseIrqlCall extends FunctionCall { /** A direct call to a function that lowers the IRQL. */ class KeLowerIrqlCall extends FunctionCall { - KeLowerIrqlCall() { this.getTarget().getName().matches(any(["KeLowerIrql", "KfLowerIrql"])) } + KeLowerIrqlCall() { this.getTarget().getName() = ["KeLowerIrql", "KfLowerIrql"] } /** * A heuristic evaluation of the IRQL that the system is lowering to. This is defined as @@ -560,6 +589,8 @@ class RestoresGlobalIrqlCall extends FunctionCall { * * This function is obviously _not_ a guarantee that two expressions refer to the same thing. * Use this locally and with caution. + * + * TODO: Compare with global value numbering (both accuracy and performance) */ pragma[inline] private predicate exprsMatchText(Expr e1, Expr e2) { @@ -588,7 +619,6 @@ private predicate exprsMatchText(Expr e1, Expr e2) { * * Not implemented: _IRQL_limited_to_ */ -pragma[assume_small_delta] cached int getPotentialExitIrqlAtCfn(ControlFlowNode cfn) { if cfn instanceof KeRaiseIrqlCall @@ -629,6 +659,9 @@ int getPotentialExitIrqlAtCfn(ControlFlowNode cfn) { cfn2.getASuccessor() = fc ) then + // TODO: Check that this node is actually a function entry point and not just + // an isolated part of the CFN. Sometimes we get nodes that are "in" a function's + // CFN, but have no predecessors, but are not function entry. (Why?) result = any(getPotentialExitIrqlAtCfn(any(ControlFlowNode cfn2 | cfn2.getASuccessor().(FunctionCall).getTarget() = @@ -656,30 +689,91 @@ private ControlFlowNode getExitPointsOfFunction(Function f) { /** * Attempt to find the range of valid IRQL values when **entering** a given IRQL-annotated function. + * This is used as a heuristic when no other IRQL information is available (i.e. we are at the top + * of a call stack.) * - * Note: we implicitly apply DISPATCH_LEVEL as the max when a max is not specified. + * Note: we implicitly apply DISPATCH_LEVEL as the max when a max is not specified but a minimum is, + * and the global max if the minimum is > DISPATCH_LEVEL. */ cached -int getAllowableIrqlLevel(IrqlRestrictsFunction irqlFunc) { - if irqlFunc instanceof IrqlRequiresAnnotatedFunction - then result = irqlFunc.(IrqlRequiresAnnotatedFunction).getIrqlLevel() +int getAllowableIrqlLevel(Function func) { + if + exists(IrqlValue lowerBound, IrqlValue upperBound | + hasLowerBound(func, lowerBound) and hasUpperBound(func, upperBound) + ) + then + result = + [any(IrqlValue lowerBound | hasLowerBound(func, lowerBound)) .. any(IrqlValue upperBound | + hasUpperBound(func, upperBound) + )] else if - irqlFunc instanceof IrqlMaxAnnotatedFunction and - irqlFunc instanceof IrqlMinAnnotatedFunction - then - result = - any([irqlFunc.(IrqlMinAnnotatedFunction).getIrqlLevel() .. irqlFunc - .(IrqlMaxAnnotatedFunction) - .getIrqlLevel()] - ) + exists(IrqlValue upperBound | hasUpperBound(func, upperBound)) and + not exists(IrqlValue lowerBound | hasLowerBound(func, lowerBound)) + then result = [0 .. any(IrqlValue upperBound | hasUpperBound(func, upperBound))] else - if irqlFunc instanceof IrqlMaxAnnotatedFunction - then result = any([0 .. irqlFunc.(IrqlMaxAnnotatedFunction).getIrqlLevel()]) + if + not exists(IrqlValue upperBound | hasUpperBound(func, upperBound)) and + exists(IrqlValue lowerBound | hasLowerBound(func, lowerBound) and lowerBound > 2) + then + result = + [any(IrqlValue lowerBound | hasLowerBound(func, lowerBound)) .. getGlobalMaxIrqlLevel()] else - if irqlFunc instanceof IrqlMinAnnotatedFunction - then result = any([irqlFunc.(IrqlMinAnnotatedFunction).getIrqlLevel() .. 2]) - else - // No known restriction. Default to between PASSIVE and DISPATCH. - result = any([0 .. 2]) + if + not exists(IrqlValue upperBound | hasUpperBound(func, upperBound)) and + exists(IrqlValue lowerBound | hasLowerBound(func, lowerBound) and lowerBound <= 2) + then result = [any(IrqlValue lowerBound | hasLowerBound(func, lowerBound)) .. 2] + else result = -1 +} + +/** Attempts to find an upper bound on the expected entry IRQL for a function. */ +private predicate hasUpperBound(Function func, IrqlValue upperBound) { + // The instanceof checks may seem redundant, but in fact they let us + // implement a "priority" if there are multiple, possibly conflicting annotations. + ( + func.(IrqlMaxAnnotatedFunction).getIrqlLevel() = upperBound + or + not func instanceof IrqlMaxAnnotatedFunction and + func.(IrqlAlwaysMaxFunction).getIrqlLevel() = upperBound + or + ( + not func instanceof IrqlMaxAnnotatedFunction and + not func instanceof IrqlAlwaysMaxFunction + ) and + func.(IrqlRequiresAnnotatedFunction).getIrqlLevel() = upperBound + or + ( + not func instanceof IrqlMaxAnnotatedFunction and + not func instanceof IrqlAlwaysMaxFunction and + not func instanceof IrqlRequiresAnnotatedFunction + ) and + func instanceof PagedFunctionDeclaration and + upperBound = 1 + ) +} + +/** Attempts to find a lower bound on the expected entry IRQL for a function. */ +private predicate hasLowerBound(Function func, IrqlValue lowerBound) { + // The instanceof checks may seem redundant, but in fact they let us + // implement a "priority" if there are multiple, possibly conflicting annotations. + ( + func.(IrqlMinAnnotatedFunction).getIrqlLevel() = lowerBound + or + not func instanceof IrqlMinAnnotatedFunction and + func.(IrqlAlwaysMinFunction).getIrqlLevel() = lowerBound + or + ( + not func instanceof IrqlMinAnnotatedFunction and + not func instanceof IrqlAlwaysMinFunction + ) and + func.(IrqlRequiresAnnotatedFunction).getIrqlLevel() = lowerBound + or + ( + not func instanceof IrqlMinAnnotatedFunction and + not func instanceof IrqlAlwaysMinFunction and + not func instanceof IrqlRequiresAnnotatedFunction + ) and + func instanceof PagedFunctionDeclaration and + lowerBound = 0 + ) } diff --git a/src/drivers/libraries/IrqlDebug.qll b/src/drivers/libraries/IrqlDebug.qll new file mode 100644 index 00000000..609ccc9d --- /dev/null +++ b/src/drivers/libraries/IrqlDebug.qll @@ -0,0 +1,63 @@ +import cpp +import drivers.libraries.Irql + +/** + * A debugging function used to print the rationale for why a given CFN has the IRQL value it does. + */ +cached +string getIrqlDebugInfoAtCfn(ControlFlowNode cfn) { + if cfn instanceof KeRaiseIrqlCall + then result = "This is a KeRaiseIRQL call" + else + if cfn instanceof KeLowerIrqlCall + then result = "This is a KeLowerIRQL call" + else + if cfn instanceof RestoresGlobalIrqlCall + then result = "This is a RestoresGlobalIrqlCall" + else + if cfn instanceof IrqlRestoreCall + then result = "This is an IrqlRestoreCall" + else + if + cfn instanceof FunctionCall and + cfn.(FunctionCall).getTarget() instanceof IrqlRaisesAnnotatedFunction + then result = "This is a function call, and the target is annotated _irql_raises_" + else + if + cfn instanceof FunctionCall and + cfn.(FunctionCall).getTarget() instanceof IrqlRequiresSameAnnotatedFunction + then result = "This is a function call, and the target is annotated _irql_same_" + else + if cfn instanceof FunctionCall + then result = "This is a function call, and we took the result exiting the function" + else + if exists(ControlFlowNode cfn2 | cfn2 = cfn.getAPredecessor()) + then result = "This is the IRQL from the prior CFN in this function" + else + if + exists(FunctionCall fc, ControlFlowNode cfn2 | + fc.getTarget() = cfn.getControlFlowScope() and + cfn2.getASuccessor() = fc + ) + then + result = + "There is no preceding node. This is the IRQL from a function call to this function. Calilng function: " + + + any(string s | + exists(ControlFlowNode call | + call.getASuccessor().(FunctionCall).getTarget() = + cfn.getControlFlowScope() and + s = + " (" + call.getControlFlowScope().getName() + ": " + + call.getControlFlowScope().getFile().getBaseName() + ") " + ) + ) + else + if + cfn.getControlFlowScope() instanceof IrqlRestrictsFunction and + getAllowableIrqlLevel(cfn.getControlFlowScope()) != -1 + then + result = + "This function has no callers but is annotated to have IRQL restrictions on it, so we use those restrictions" + else result = "We could not find any usable IRQL info" +} diff --git a/src/drivers/test/build_create_analyze_test.cmd b/src/drivers/test/build_create_analyze_test.cmd index 02ec00ae..a5b6ebf3 100644 --- a/src/drivers/test/build_create_analyze_test.cmd +++ b/src/drivers/test/build_create_analyze_test.cmd @@ -31,6 +31,8 @@ call :test IrqlNotSaved WDMTestTemplate general queries call :test IllegalFieldWrite WDMTestTemplate wdm queries call :test IllegalFieldAccess2 WDMTestTemplate wdm queries call :test RoutineFunctionTypeNotExpected WDMTestTemplate general queries +call :test KeSetEventIrql WDMTestTemplate general queries\experimental +call :test KeSetEventPaged WDMTestTemplate general queries exit /b 0 diff --git a/src/drivers/test/diff/KeSetEventIrql.sarif b/src/drivers/test/diff/KeSetEventIrql.sarif new file mode 100644 index 00000000..dea8275e --- /dev/null +++ b/src/drivers/test/diff/KeSetEventIrql.sarif @@ -0,0 +1,21 @@ +{ + "all": { + "+": 0, + "-": 0 + }, + "error": { + "+": 0, + "-": 0, + "codes": [] + }, + "warning": { + "+": 0, + "-": 0, + "codes": [] + }, + "note": { + "+": 0, + "-": 0, + "codes": [] + } +} \ No newline at end of file diff --git a/src/drivers/test/diff/KeSetEventPaged.sarif b/src/drivers/test/diff/KeSetEventPaged.sarif new file mode 100644 index 00000000..dea8275e --- /dev/null +++ b/src/drivers/test/diff/KeSetEventPaged.sarif @@ -0,0 +1,21 @@ +{ + "all": { + "+": 0, + "-": 0 + }, + "error": { + "+": 0, + "-": 0, + "codes": [] + }, + "warning": { + "+": 0, + "-": 0, + "codes": [] + }, + "note": { + "+": 0, + "-": 0, + "codes": [] + } +} \ No newline at end of file diff --git a/src/suites/windows_driver_recommended.qls b/src/suites/windows_driver_recommended.qls index 6c707ced..9559419b 100644 --- a/src/suites/windows_driver_recommended.qls +++ b/src/suites/windows_driver_recommended.qls @@ -30,4 +30,5 @@ - microsoft/Likely Bugs/Conversion/InfiniteLoop.ql - microsoft/Likely Bugs/Memory Management/UseAfterFree/UseAfterFree.ql - microsoft/Likely Bugs/UninitializedPtrField.ql - - microsoft/Security/Crytpography/HardcodedIVCNG.ql \ No newline at end of file + - microsoft/Security/Crytpography/HardcodedIVCNG.ql + - drivers/general/queries/KeSetEventPaged/KeSetEventPaged.ql \ No newline at end of file From 0976a3ce8738b4cc1b2612d5435ce2455d836993 Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Tue, 21 Nov 2023 14:37:41 -0800 Subject: [PATCH 11/11] Cleanup of various issues before RI. - Update name/message of KeSetEventPaged to KeSetEventPageable - Add missing diff for RoutineFunctionTypeNotExpected - Update baseline for KeWaitLocal - Add RoutineFunctionTypeNotExpected to ported_driver_ca_checks suite --- .../KeSetEventPageable.ql} | 10 +- .../KeSetEventPageable.qlhelp} | 0 .../KeSetEventPageable.sarif} | 36 ++-- .../driver_snippet.c | 0 .../test/build_create_analyze_test.cmd | 2 +- ...ntPaged.sarif => KeSetEventPageable.sarif} | 0 src/drivers/test/diff/KeWaitLocal.sarif | 21 +-- .../diff/RoutineFunctionTypeNotExpected.sarif | 21 +++ .../wdm/queries/KeWaitLocal/KeWaitLocal.sarif | 154 ++++++++++++++++-- src/suites/ported_driver_ca_checks.qls | 1 + src/suites/windows_driver_recommended.qls | 2 +- 11 files changed, 190 insertions(+), 57 deletions(-) rename src/drivers/general/queries/{KeSetEventPaged/KeSetEventPaged.ql => KeSetEventPageable/KeSetEventPageable.ql} (73%) rename src/drivers/general/queries/{KeSetEventPaged/KeSetEventPaged.qlhelp => KeSetEventPageable/KeSetEventPageable.qlhelp} (100%) rename src/drivers/general/queries/{KeSetEventPaged/KeSetEventPaged.sarif => KeSetEventPageable/KeSetEventPageable.sarif} (84%) rename src/drivers/general/queries/{KeSetEventPaged => KeSetEventPageable}/driver_snippet.c (100%) rename src/drivers/test/diff/{KeSetEventPaged.sarif => KeSetEventPageable.sarif} (100%) create mode 100644 src/drivers/test/diff/RoutineFunctionTypeNotExpected.sarif diff --git a/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.ql b/src/drivers/general/queries/KeSetEventPageable/KeSetEventPageable.ql similarity index 73% rename from src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.ql rename to src/drivers/general/queries/KeSetEventPageable/KeSetEventPageable.ql index e383301f..88690dee 100644 --- a/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.ql +++ b/src/drivers/general/queries/KeSetEventPageable/KeSetEventPageable.ql @@ -1,14 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. /** - * @id cpp/drivers/ke-set-event-irql - * @name KeSetEvent called in paged segment with wait - * @description Calles to KeSetEvent in a paged segment must not call with the Wait parameter set to true. This can cause a system crash if the segment is paged out. + * @id cpp/drivers/ke-set-event-pageable + * @name KeSetEvent called in pageable segment with wait + * @description Calls to KeSetEvent in a pageable segment must not call with the Wait parameter set to true. This can cause a system crash if the segment is paged out. * @platform Desktop * @security.severity Low * @feature.area Multiple * @impact Exploitable Design - * @repro.text The following call to KeSetEvent has Wait set to true while in a paged segment. + * @repro.text The following call to KeSetEvent has Wait set to true while in a pageable segment. * @owner.email sdat@microsoft.com * @opaqueid CQLD-D0004 * @kind problem @@ -32,5 +32,5 @@ where enclosingFunc = ksec.getEnclosingFunction() and ksec.getArgument(2).getValue() = "1" select ksec, - "$@: KeSetEvent should not be called with the Wait parameter set to true when in a paged function.", + "$@: KeSetEvent should not be called with the Wait parameter set to true when in a pageable segment.", ksec.getControlFlowScope(), ksec.getControlFlowScope().getQualifiedName() diff --git a/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.qlhelp b/src/drivers/general/queries/KeSetEventPageable/KeSetEventPageable.qlhelp similarity index 100% rename from src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.qlhelp rename to src/drivers/general/queries/KeSetEventPageable/KeSetEventPageable.qlhelp diff --git a/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.sarif b/src/drivers/general/queries/KeSetEventPageable/KeSetEventPageable.sarif similarity index 84% rename from src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.sarif rename to src/drivers/general/queries/KeSetEventPageable/KeSetEventPageable.sarif index 26e196eb..177d11f1 100644 --- a/src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.sarif +++ b/src/drivers/general/queries/KeSetEventPageable/KeSetEventPageable.sarif @@ -24,33 +24,33 @@ } } ], "rules" : [ { - "id" : "cpp/drivers/ke-set-event-irql", - "name" : "cpp/drivers/ke-set-event-irql", + "id" : "cpp/drivers/ke-set-event-pageable", + "name" : "cpp/drivers/ke-set-event-pageable", "shortDescription" : { - "text" : "KeSetEvent called in paged segment with wait" + "text" : "KeSetEvent called in pageable segment with wait" }, "fullDescription" : { - "text" : "Calles to KeSetEvent in a paged segment must not call with the Wait parameter set to true. This can cause a system crash if the segment is paged out." + "text" : "Calls to KeSetEvent in a pageable segment must not call with the Wait parameter set to true. This can cause a system crash if the segment is paged out." }, "defaultConfiguration" : { "enabled" : true, - "level" : "warning" + "level" : "error" }, "properties" : { "tags" : [ "correctness", "wddst" ], - "description" : "Calles to KeSetEvent in a paged segment must not call with the Wait parameter set to true. This can cause a system crash if the segment is paged out.", + "description" : "Calls to KeSetEvent in a pageable segment must not call with the Wait parameter set to true. This can cause a system crash if the segment is paged out.", "feature.area" : "Multiple", - "id" : "cpp/drivers/ke-set-event-irql", + "id" : "cpp/drivers/ke-set-event-pageable", "impact" : "Exploitable Design", "kind" : "problem", - "name" : "KeSetEvent called in paged segment with wait", + "name" : "KeSetEvent called in pageable segment with wait", "opaqueid" : "CQLD-D0004", "owner.email" : "sdat@microsoft.com", "platform" : "Desktop", - "precision" : "medium", - "problem.severity" : "warning", + "precision" : "high", + "problem.severity" : "error", "query-version" : "v1", - "repro.text" : "The following call to KeSetEvent has Wait set to true while in a paged segment.", + "repro.text" : "The following call to KeSetEvent has Wait set to true while in a pageable segment.", "scope" : "domainspecific", "security.severity" : "Low" } @@ -58,7 +58,7 @@ }, "extensions" : [ { "name" : "microsoft/windows-drivers", - "semanticVersion" : "0.2.0+234ee9b709196a3a802b2c02ad7945ba0dfb0ac0", + "semanticVersion" : "0.2.0+143fe74d66f4093412a7b21390672217b557bba2", "locations" : [ { "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", "description" : { @@ -165,14 +165,14 @@ } } ], "results" : [ { - "ruleId" : "cpp/drivers/ke-set-event-irql", + "ruleId" : "cpp/drivers/ke-set-event-pageable", "ruleIndex" : 0, "rule" : { - "id" : "cpp/drivers/ke-set-event-irql", + "id" : "cpp/drivers/ke-set-event-pageable", "index" : 0 }, "message" : { - "text" : "[KeSetEventIrql_Fail2](1): KeSetEvent should not be called with the Wait parameter set to true when in a paged function." + "text" : "[KeSetEventIrql_Fail2](1): KeSetEvent should not be called with the Wait parameter set to true when in a pageable segment." }, "locations" : [ { "physicalLocation" : { @@ -211,14 +211,14 @@ } } ] }, { - "ruleId" : "cpp/drivers/ke-set-event-irql", + "ruleId" : "cpp/drivers/ke-set-event-pageable", "ruleIndex" : 0, "rule" : { - "id" : "cpp/drivers/ke-set-event-irql", + "id" : "cpp/drivers/ke-set-event-pageable", "index" : 0 }, "message" : { - "text" : "[KeSetEventIrql_Fail1](1): KeSetEvent should not be called with the Wait parameter set to true when in a paged function." + "text" : "[KeSetEventIrql_Fail1](1): KeSetEvent should not be called with the Wait parameter set to true when in a pageable segment." }, "locations" : [ { "physicalLocation" : { diff --git a/src/drivers/general/queries/KeSetEventPaged/driver_snippet.c b/src/drivers/general/queries/KeSetEventPageable/driver_snippet.c similarity index 100% rename from src/drivers/general/queries/KeSetEventPaged/driver_snippet.c rename to src/drivers/general/queries/KeSetEventPageable/driver_snippet.c diff --git a/src/drivers/test/build_create_analyze_test.cmd b/src/drivers/test/build_create_analyze_test.cmd index a5b6ebf3..b2c27cb6 100644 --- a/src/drivers/test/build_create_analyze_test.cmd +++ b/src/drivers/test/build_create_analyze_test.cmd @@ -32,7 +32,7 @@ call :test IllegalFieldWrite WDMTestTemplate wdm queries call :test IllegalFieldAccess2 WDMTestTemplate wdm queries call :test RoutineFunctionTypeNotExpected WDMTestTemplate general queries call :test KeSetEventIrql WDMTestTemplate general queries\experimental -call :test KeSetEventPaged WDMTestTemplate general queries +call :test KeSetEventPageable WDMTestTemplate general queries exit /b 0 diff --git a/src/drivers/test/diff/KeSetEventPaged.sarif b/src/drivers/test/diff/KeSetEventPageable.sarif similarity index 100% rename from src/drivers/test/diff/KeSetEventPaged.sarif rename to src/drivers/test/diff/KeSetEventPageable.sarif diff --git a/src/drivers/test/diff/KeWaitLocal.sarif b/src/drivers/test/diff/KeWaitLocal.sarif index 4ac852d9..dea8275e 100644 --- a/src/drivers/test/diff/KeWaitLocal.sarif +++ b/src/drivers/test/diff/KeWaitLocal.sarif @@ -1,7 +1,7 @@ { "all": { - "+": 1, - "-": 1 + "+": 0, + "-": 0 }, "error": { "+": 0, @@ -9,20 +9,9 @@ "codes": [] }, "warning": { - "+": 1, - "-": 1, - "codes": [ - [ - "cpp/drivers/kewaitlocal-requires-kernel-mode [good_use](1): KeWaitForSingleObject should have a KernelMode AccessMode when the [first argument](2) is local", - 0, - 1 - ], - [ - "cpp/drivers/kewaitlocal-requires-kernel-mode KeWaitForSingleObject should have a KernelMode AccessMode when the first argument is local", - 1, - 0 - ] - ] + "+": 0, + "-": 0, + "codes": [] }, "note": { "+": 0, diff --git a/src/drivers/test/diff/RoutineFunctionTypeNotExpected.sarif b/src/drivers/test/diff/RoutineFunctionTypeNotExpected.sarif new file mode 100644 index 00000000..dea8275e --- /dev/null +++ b/src/drivers/test/diff/RoutineFunctionTypeNotExpected.sarif @@ -0,0 +1,21 @@ +{ + "all": { + "+": 0, + "-": 0 + }, + "error": { + "+": 0, + "-": 0, + "codes": [] + }, + "warning": { + "+": 0, + "-": 0, + "codes": [] + }, + "note": { + "+": 0, + "-": 0, + "codes": [] + } +} \ No newline at end of file diff --git a/src/drivers/wdm/queries/KeWaitLocal/KeWaitLocal.sarif b/src/drivers/wdm/queries/KeWaitLocal/KeWaitLocal.sarif index 400a26f4..b8c9ff70 100644 --- a/src/drivers/wdm/queries/KeWaitLocal/KeWaitLocal.sarif +++ b/src/drivers/wdm/queries/KeWaitLocal/KeWaitLocal.sarif @@ -6,7 +6,23 @@ "driver" : { "name" : "CodeQL", "organization" : "GitHub", - "semanticVersion" : "2.11.5", + "semanticVersion" : "2.15.1", + "notifications" : [ { + "id" : "cpp/baseline/expected-extracted-files", + "name" : "cpp/baseline/expected-extracted-files", + "shortDescription" : { + "text" : "Expected extracted files" + }, + "fullDescription" : { + "text" : "Files appearing in the source archive that are expected to be extracted." + }, + "defaultConfiguration" : { + "enabled" : true + }, + "properties" : { + "tags" : [ "expected-extracted-files", "telemetry" ] + } + } ], "rules" : [ { "id" : "cpp/drivers/kewaitlocal-requires-kernel-mode", "name" : "cpp/drivers/kewaitlocal-requires-kernel-mode", @@ -42,7 +58,7 @@ }, "extensions" : [ { "name" : "microsoft/windows-drivers", - "semanticVersion" : "0.1.0+c5706b97bc9d314a2dabbf2421005784dd3c1303", + "semanticVersion" : "0.2.0+143fe74d66f4093412a7b21390672217b557bba2", "locations" : [ { "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", "description" : { @@ -54,28 +70,99 @@ "text" : "The QL pack definition file." } } ] + } ] + }, + "invocations" : [ { + "toolExecutionNotifications" : [ { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } }, { - "name" : "legacy-upgrades", - "semanticVersion" : "0.0.0", "locations" : [ { - "uri" : "file:///C:/codeql-home/codeql/legacy-upgrades/", - "description" : { - "text" : "The QL pack root directory." + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } } - }, { - "uri" : "file:///C:/codeql-home/codeql/legacy-upgrades/qlpack.yml", - "description" : { - "text" : "The QL pack definition file." + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" } - } ] - } ] - }, + } + }, { + "locations" : [ { + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } + } + } ], + "message" : { + "text" : "" + }, + "level" : "none", + "descriptor" : { + "id" : "cpp/baseline/expected-extracted-files", + "index" : 0 + }, + "properties" : { + "formattedMessage" : { + "text" : "" + } + } + } ], + "executionSuccessful" : true + } ], "artifacts" : [ { "location" : { "uri" : "driver/driver_snippet.c", "uriBaseId" : "%SRCROOT%", "index" : 0 } + }, { + "location" : { + "uri" : "driver/fail_driver1.c", + "uriBaseId" : "%SRCROOT%", + "index" : 1 + } + }, { + "location" : { + "uri" : "driver/fail_driver1.h", + "uriBaseId" : "%SRCROOT%", + "index" : 2 + } } ], "results" : [ { "ruleId" : "cpp/drivers/kewaitlocal-requires-kernel-mode", @@ -85,7 +172,7 @@ "index" : 0 }, "message" : { - "text" : "KeWaitForSingleObject should have a KernelMode AccessMode when the first argument is local" + "text" : "[good_use](1): KeWaitForSingleObject should have a KernelMode AccessMode when the [first argument](2) is local" }, "locations" : [ { "physicalLocation" : { @@ -104,7 +191,42 @@ "partialFingerprints" : { "primaryLocationLineHash" : "61bc3c7079348327:1", "primaryLocationStartColumnFingerprint" : "0" - } + }, + "relatedLocations" : [ { + "id" : 1, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 7, + "startColumn" : 6, + "endColumn" : 14 + } + }, + "message" : { + "text" : "good_use" + } + }, { + "id" : 2, + "physicalLocation" : { + "artifactLocation" : { + "uri" : "driver/driver_snippet.c", + "uriBaseId" : "%SRCROOT%", + "index" : 0 + }, + "region" : { + "startLine" : 9, + "startColumn" : 12, + "endColumn" : 19 + } + }, + "message" : { + "text" : "first argument" + } + } ] } ], "columnKind" : "utf16CodeUnits", "properties" : { diff --git a/src/suites/ported_driver_ca_checks.qls b/src/suites/ported_driver_ca_checks.qls index 1a9058b4..c30d978b 100644 --- a/src/suites/ported_driver_ca_checks.qls +++ b/src/suites/ported_driver_ca_checks.qls @@ -10,6 +10,7 @@ - drivers/general/queries/ExtendedDeprecatedApis/ExtendedDeprecatedApis.ql - drivers/general/queries/IrqlNotSaved/IrqlNotSaved.ql - drivers/general/queries/IrqlNotUsed/IrqlNotUsed.ql + - drivers/general/queries/RoutineFunctionTypeNotExpected/RoutineFunctionTypeNotExpected.ql - drivers/general/queries/PoolTagIntegral/PoolTagIntegral.ql - drivers/general/queries/WdkDeprecatedApis/wdk-deprecated-api.ql - drivers/kmdf/queries/StrSafe/StrSafe.ql diff --git a/src/suites/windows_driver_recommended.qls b/src/suites/windows_driver_recommended.qls index 9559419b..06725b5b 100644 --- a/src/suites/windows_driver_recommended.qls +++ b/src/suites/windows_driver_recommended.qls @@ -31,4 +31,4 @@ - microsoft/Likely Bugs/Memory Management/UseAfterFree/UseAfterFree.ql - microsoft/Likely Bugs/UninitializedPtrField.ql - microsoft/Security/Crytpography/HardcodedIVCNG.ql - - drivers/general/queries/KeSetEventPaged/KeSetEventPaged.ql \ No newline at end of file + - drivers/general/queries/KeSetEventPaged/KeSetEventPageable.ql \ No newline at end of file