From 3fa482761e94d69977cd96d107c99f045d2ab807 Mon Sep 17 00:00:00 2001 From: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> Date: Mon, 18 Mar 2024 14:57:30 -0700 Subject: [PATCH 1/3] Update README.md to fix typo Signed-off-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8939ef93..dfae57e1 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,7 @@ This repository contains open-source components for supplemental use in developi Specific versions, queries, or suites can be specified using the format `codeql database analyze /@x.x.x:`. For futher information, see the [CodeQL documentation](https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/publishing-and-using-codeql-packs#using-a-codeql-pack-to-analyze-a-codeql-database). - Example: `codeql database analyze --download D:\DriverDatabase suites/windows-all-recommended.qls --format=sarifv2.1.0 --output=D:\DriverAnalysis1.sarif ` + Example: `codeql database analyze --download D:\DriverDatabase suites/windows_driver_recommended.qls --format=sarifv2.1.0 --output=D:\DriverAnalysis1.sarif ` _(Parameters: path to new database, query pack, format, output sarif file)_ From eddc6fd651bb078f0afe0498fad3720ed93e9517 Mon Sep 17 00:00:00 2001 From: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> Date: Tue, 26 Mar 2024 13:40:41 -0700 Subject: [PATCH 2/3] Updates to multithreadedavcondition query (port of CA28616) (#136) * add example to qhelp file * update doc * update query ID * Update MultithreadedAVCondition.qhelp Signed-off-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> --------- Signed-off-by: Jacob Ronstadt <147542405+jacob-ronstadt@users.noreply.github.com> --- .../MultithreadedAVCondition.qhelp | 55 +++++++++++++++++++ .../MultithreadedAVCondition.ql | 2 +- .../MultithreadedAVCondition.qlhelp | 23 -------- 3 files changed, 56 insertions(+), 24 deletions(-) create mode 100644 src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.qhelp delete mode 100644 src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.qlhelp diff --git a/src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.qhelp b/src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.qhelp new file mode 100644 index 00000000..3f7bf527 --- /dev/null +++ b/src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.qhelp @@ -0,0 +1,55 @@ + + + +

+ This warning indicates that a thread has potential to access deleted objects if preempted. Unlike the Code Analysis version of this query, this query does not currently verify the use of any synchronization mechanisms, so it may produce false positives. +

+ + +

+ There should be no access to a reference-counted object after the reference count is at zero +

+ + +

+ In this example, m_cRef is a member of this. A thread T1 executes the if condition, decrements m_cRef to 1, and is then preempted. Another thread T2 executes the if condition, decrements m_cRef to 0, executes the if body (where this is deleted), and returns NULL. +

+ + +

+ The following code does not reference any heap memory after the object is deleted. +

+ + + + +
  • + + Warning C28616 + +
    • +     +     diff --git a/src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.ql b/src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.ql index 1a3cbcd4..eae80e29 100644 --- a/src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.ql +++ b/src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.ql @@ -10,7 +10,7 @@ * @impact Exploitable Design * @repro.text There should be no access to a reference-counted object after the reference count is at zero * @owner.email sdat@microsoft.com - * @opaqueid CQLD-D0006 + * @opaqueid CQLD-C28616 * @kind problem * @problem.severity warning * @precision medium diff --git a/src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.qlhelp b/src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.qlhelp deleted file mode 100644 index 3d05a279..00000000 --- a/src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.qlhelp +++ /dev/null @@ -1,23 +0,0 @@ - - - -

    - This warning indicates that a thread has potential to access deleted objects if preempted. -

    -     - -

    - There should be no access to a reference-counted object after the reference count is at zero -

    -     - - - - -
  • - - Warning C28616 - -
    • -     -     From 5dc2b928dffec2ca7d3e2622a5861e9f5943bede Mon Sep 17 00:00:00 2001 From: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Date: Fri, 29 Mar 2024 11:59:29 -0700 Subject: [PATCH 3/3] Update suppression library to use new query IDs. (#137) * Update suppression IDs [in-progress] * Finish adding/updating suppression IDs. * Fix opaque query IDs for a few queies. * Clean up some accidental duplicates in Suppression.qll. --- .../queries/IrqlTooHigh/IrqlTooHigh.ql | 4 +- .../queries/IrqlTooHigh/IrqlTooHigh.sarif | 10 +-- .../general/queries/IrqlTooLow/IrqlTooLow.ql | 4 +- .../queries/IrqlTooLow/IrqlTooLow.sarif | 10 +-- .../RoleTypeCorrectlyUsed.ql | 2 +- .../RoleTypeCorrectlyUsed.sarif | 18 ++--- src/drivers/libraries/Suppression.qll | 66 +++++++++++-------- 7 files changed, 61 insertions(+), 53 deletions(-) diff --git a/src/drivers/general/queries/IrqlTooHigh/IrqlTooHigh.ql b/src/drivers/general/queries/IrqlTooHigh/IrqlTooHigh.ql index fbec0213..d89c1966 100644 --- a/src/drivers/general/queries/IrqlTooHigh/IrqlTooHigh.ql +++ b/src/drivers/general/queries/IrqlTooHigh/IrqlTooHigh.ql @@ -2,7 +2,7 @@ // Licensed under the MIT license. /** * @id cpp/drivers/irql-too-high - * @name IRQL too high (C28120) + * @name IRQL too high (C28121) * @description A function annotated with IRQL requirements was called at an IRQL too high for the requirements. * @platform Desktop * @security.severity Low @@ -10,7 +10,7 @@ * @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 + * @opaqueid CQLD-C28121 * @kind problem * @problem.severity warning * @precision medium diff --git a/src/drivers/general/queries/IrqlTooHigh/IrqlTooHigh.sarif b/src/drivers/general/queries/IrqlTooHigh/IrqlTooHigh.sarif index 52a2f7a3..8e663932 100644 --- a/src/drivers/general/queries/IrqlTooHigh/IrqlTooHigh.sarif +++ b/src/drivers/general/queries/IrqlTooHigh/IrqlTooHigh.sarif @@ -6,7 +6,7 @@ "driver" : { "name" : "CodeQL", "organization" : "GitHub", - "semanticVersion" : "2.14.4", + "semanticVersion" : "2.15.4", "notifications" : [ { "id" : "cpp/baseline/expected-extracted-files", "name" : "cpp/baseline/expected-extracted-files", @@ -27,7 +27,7 @@ "id" : "cpp/drivers/irql-too-high", "name" : "cpp/drivers/irql-too-high", "shortDescription" : { - "text" : "IRQL too high (C28120)" + "text" : "IRQL too high (C28121)" }, "fullDescription" : { "text" : "A function annotated with IRQL requirements was called at an IRQL too high for the requirements." @@ -43,8 +43,8 @@ "id" : "cpp/drivers/irql-too-high", "impact" : "Exploitable Design", "kind" : "problem", - "name" : "IRQL too high (C28120)", - "opaqueid" : "CQLD-C28120", + "name" : "IRQL too high (C28121)", + "opaqueid" : "CQLD-C28121", "owner.email" : "sdat@microsoft.com", "platform" : "Desktop", "precision" : "medium", @@ -58,7 +58,7 @@ }, "extensions" : [ { "name" : "microsoft/windows-drivers", - "semanticVersion" : "0.2.0+4842fd4116871d3b47eede85c2c4497b43c34d57", + "semanticVersion" : "1.1.0+2affc3c634804dac7504a483a378cc9ba22a0f0b", "locations" : [ { "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", "description" : { diff --git a/src/drivers/general/queries/IrqlTooLow/IrqlTooLow.ql b/src/drivers/general/queries/IrqlTooLow/IrqlTooLow.ql index e0234179..24f31d1a 100644 --- a/src/drivers/general/queries/IrqlTooLow/IrqlTooLow.ql +++ b/src/drivers/general/queries/IrqlTooLow/IrqlTooLow.ql @@ -2,7 +2,7 @@ // Licensed under the MIT license. /** * @id cpp/drivers/irql-too-low - * @name IRQL too low (C28121) + * @name IRQL too low (C28120) * @description A function annotated with IRQL requirements was called at an IRQL too low for the requirements. * @platform Desktop * @security.severity Low @@ -10,7 +10,7 @@ * @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 + * @opaqueid CQLD-C28120 * @kind problem * @problem.severity warning * @precision medium diff --git a/src/drivers/general/queries/IrqlTooLow/IrqlTooLow.sarif b/src/drivers/general/queries/IrqlTooLow/IrqlTooLow.sarif index 3c5d989e..a1783990 100644 --- a/src/drivers/general/queries/IrqlTooLow/IrqlTooLow.sarif +++ b/src/drivers/general/queries/IrqlTooLow/IrqlTooLow.sarif @@ -6,7 +6,7 @@ "driver" : { "name" : "CodeQL", "organization" : "GitHub", - "semanticVersion" : "2.14.4", + "semanticVersion" : "2.15.4", "notifications" : [ { "id" : "cpp/baseline/expected-extracted-files", "name" : "cpp/baseline/expected-extracted-files", @@ -27,7 +27,7 @@ "id" : "cpp/drivers/irql-too-low", "name" : "cpp/drivers/irql-too-low", "shortDescription" : { - "text" : "IRQL too low (C28121)" + "text" : "IRQL too low (C28120)" }, "fullDescription" : { "text" : "A function annotated with IRQL requirements was called at an IRQL too low for the requirements." @@ -43,8 +43,8 @@ "id" : "cpp/drivers/irql-too-low", "impact" : "Exploitable Design", "kind" : "problem", - "name" : "IRQL too low (C28121)", - "opaqueid" : "CQLD-C28121", + "name" : "IRQL too low (C28120)", + "opaqueid" : "CQLD-C28120", "owner.email" : "sdat@microsoft.com", "platform" : "Desktop", "precision" : "medium", @@ -58,7 +58,7 @@ }, "extensions" : [ { "name" : "microsoft/windows-drivers", - "semanticVersion" : "0.2.0+4842fd4116871d3b47eede85c2c4497b43c34d57", + "semanticVersion" : "1.1.0+2affc3c634804dac7504a483a378cc9ba22a0f0b", "locations" : [ { "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", "description" : { diff --git a/src/drivers/general/queries/RoleTypeCorrectlyUsed/RoleTypeCorrectlyUsed.ql b/src/drivers/general/queries/RoleTypeCorrectlyUsed/RoleTypeCorrectlyUsed.ql index 64c77279..27c21d79 100644 --- a/src/drivers/general/queries/RoleTypeCorrectlyUsed/RoleTypeCorrectlyUsed.ql +++ b/src/drivers/general/queries/RoleTypeCorrectlyUsed/RoleTypeCorrectlyUsed.ql @@ -10,7 +10,7 @@ * @impact Insecure Coding Practice * @repro.text * @owner.email: sdat@microsoft.com - * @opaqueid CQLD-C28158 + * @opaqueid CQLD-D0007 * @problem.severity warning * @precision medium * @tags correctness diff --git a/src/drivers/general/queries/RoleTypeCorrectlyUsed/RoleTypeCorrectlyUsed.sarif b/src/drivers/general/queries/RoleTypeCorrectlyUsed/RoleTypeCorrectlyUsed.sarif index 2d6f2459..aab47630 100644 --- a/src/drivers/general/queries/RoleTypeCorrectlyUsed/RoleTypeCorrectlyUsed.sarif +++ b/src/drivers/general/queries/RoleTypeCorrectlyUsed/RoleTypeCorrectlyUsed.sarif @@ -44,12 +44,12 @@ "impact" : "Insecure Coding Practice", "kind" : "problem", "name" : "Incorrect Role Type Use", - "opaqueid" : "CQLD-C28158", + "opaqueid" : "CQLD-D0007", "owner.email:" : "sdat@microsoft.com", "platform" : "Desktop", "precision" : "medium", "problem.severity" : "warning", - "query-version" : "v1", + "query-version" : "v2", "repro.text" : "", "scope" : "domainspecific" } @@ -57,14 +57,14 @@ }, "extensions" : [ { "name" : "microsoft/windows-drivers", - "semanticVersion" : "1.0.12+54db165bcee31f7827c56bf2bb9a408d8a4db4fe", + "semanticVersion" : "1.1.0+2affc3c634804dac7504a483a378cc9ba22a0f0b", "locations" : [ { - "uri" : "file:///C:/codeql-home/WDDST/src/", + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/", "description" : { "text" : "The QL pack root directory." } }, { - "uri" : "file:///C:/codeql-home/WDDST/src/qlpack.yml", + "uri" : "file:///C:/codeql-home/Windows-Driver-Developer-Supplemental-Tools/src/qlpack.yml", "description" : { "text" : "The QL pack definition file." } @@ -76,9 +76,9 @@ "locations" : [ { "physicalLocation" : { "artifactLocation" : { - "uri" : "driver/driver_snippet.c", + "uri" : "driver/fail_driver1.c", "uriBaseId" : "%SRCROOT%", - "index" : 0 + "index" : 1 } } } ], @@ -99,9 +99,9 @@ "locations" : [ { "physicalLocation" : { "artifactLocation" : { - "uri" : "driver/fail_driver1.c", + "uri" : "driver/driver_snippet.c", "uriBaseId" : "%SRCROOT%", - "index" : 1 + "index" : 0 } } } ], diff --git a/src/drivers/libraries/Suppression.qll b/src/drivers/libraries/Suppression.qll index 1059a807..34d9c084 100644 --- a/src/drivers/libraries/Suppression.qll +++ b/src/drivers/libraries/Suppression.qll @@ -1,17 +1,17 @@ import cpp // Reference: https://learn.microsoft.com/en-us/cpp/preprocessor/warning?view=msvc-170 - -/** Represents a Code Analysis-style suppression using #pragma commands. - * +/** + * Represents a Code Analysis-style suppression using #pragma commands. + * * In this library we support two styles: * #pragma prefast (suppress:XXXX) which suppresses rule XXXX on the following line of code, and * #pragma prefast (disable:XXXX) which suppresses rule XXXX until the pragma stack is adjusted using #pragma (push/pop). - * + * * More details can be found at https://learn.microsoft.com/en-us/cpp/preprocessor/warning?view=msvc-170. * Please note that at present, pragma commands combining disable and suppress commands in a single line are * not supported. -*/ + */ abstract class CASuppression extends PreprocessorPragma { abstract predicate matchesRuleName(string name); @@ -34,15 +34,11 @@ abstract class CASuppression extends PreprocessorPragma { "__WARNING_BANNED_API_USAGE_LSTRLEN", "28750" ] ) and - result = "lgtm[cpp/windows/drivers/queries/extended-deprecated-apis]" + result = "lgtm[cpp/drivers/extended-deprecated-apis]" or this.getRuleName() = any(["__WARNING_UNHELPFUL_TAG", "28147"]) and result = - any([ - "lgtm[cpp/windows/drivers/queries/default-pool-tag]", - "lgtm[cpp/windows/drivers/queries/default-pool-tag-extended]" - ] - ) + any(["lgtm[cpp/drivers/default-pool-tag]", "lgtm[cpp/drivers/default-pool-tag-extended]"]) or this.getRuleName() = any(["__WARNING_IRQL_NOT_SET", "28158"]) and result = "lgtm[cpp/drivers/irql-not-saved]" @@ -51,56 +47,68 @@ abstract class CASuppression extends PreprocessorPragma { result = "lgtm[cpp/drivers/irql-not-used]" or this.getRuleName() = any(["__WARNING_POOL_TAG", "28134"]) and - result = "lgtm[cpp/windows/drivers/queries/pool-tag-integral]" + result = "lgtm[cpp/drivers/pool-tag-integral]" or this.getRuleName() = any(["__WARNING_STRSAFE_H", "28146"]) and - result = "lgtm[cpp/portedqueries/str-safe]" + result = "lgtm[cpp/drivers/str-safe]" or this.getRuleName() = any(["__WARNING_MUST_USE", "28193"]) and - result = "lgtm[cpp/portedqueries/examined-value]" - or - this.getRuleName() = any(["__WARNING_IRQ_TOO_LOW", "28120"]) and - result = "lgtm[cpp/portedqueries/irq-too-low]" + result = "lgtm[cpp/drivers/examined-value]" or this.getRuleName() = any(["__WARNING_IRQ_TOO_HIGH", "28121"]) and - result = "lgtm[cpp/portedqueries/irq-too-high]" + result = "lgtm[cpp/drivers/irql-too-high]" + or + this.getRuleName() = any(["__WARNING_IRQ_TOO_LOW", "28120"]) and + result = "lgtm[cpp/drivers/irql-too-low]" or this.getRuleName() = any(["__WARNING_FUNCTION_ASSIGNMENT", "28128"]) and - result = "lgtm[cpp/windows/drivers/queries/illegal-field-access]" + result = "lgtm[cpp/drivers/illegal-field-access]" or this.getRuleName() = any(["__WARNING_INACCESSIBLE_MEMBER", "28175"]) and - result = "lgtm[cpp/windows/drivers/queries/illegal-field-access-2]" + result = "lgtm[cpp/drivers/illegal-field-access-2]" or this.getRuleName() = any(["__WARNING_READ_ONLY_MEMBER", "28176"]) and - result = "lgtm[cpp/windows/drivers/queries/illegal-field-write]" + result = "lgtm[cpp/drivers/illegal-field-write]" or this.getRuleName() = any(["__WARNING_INIT_NOT_CLEARED", "28152"]) and - result = "lgtm[cpp/windows/drivers/queries/init-not-cleared]" + result = "lgtm[cpp/drivers/init-not-cleared]" or this.getRuleName() = any(["__WARNING_KE_WAIT_LOCAL", "28135"]) and result = "lgtm[cpp/drivers/kewaitlocal-requires-kernel-mode]" or this.getRuleName() = any(["__WARNING_MULTIPLE_PAGED_CODE", "28171"]) and - result = "lgtm[cpp/portedqueries/multiple-paged-code]" + result = "lgtm[cpp/drivers/multiple-paged-code]" or this.getRuleName() = any(["__WARNING_NO_PAGED_CODE", "28170"]) and - result = "lgtm[cpp/portedqueries/no-paged-code]" + result = "lgtm[cpp/drivers/no-paged-code]" or this.getRuleName() = any(["__WARNING_NO_PAGING_SEGMENT", "28172"]) and - result = "lgtm[cpp/portedqueries/no-paging-segment]" + result = "lgtm[cpp/drivers/no-paging-segment]" or this.getRuleName() = any(["__WARNING_OBJ_REFERENCE_MODE", "28126"]) and - result = "lgtm[cpp/windows/drivers/queries/ob-reference-mode]" + result = "lgtm[cpp/drivers/ob-reference-mode]" or this.getRuleName() = any(["__WARNING_MODIFYING_MDL", "28145"]) and - result = "lgtm[cpp/windows/drivers/queries/opaquemdlwrite]" + result = "lgtm[cpp/drivers/opaque-mdl-write]" or this.getRuleName() = any(["__WARNING_PENDING_STATUS_ERROR", "28143"]) and - result = "lgtm[cpp/portedqueries/pending-status-error]" + result = "lgtm[cpp/drivers/pending-status-error]" or this.getRuleName() = any(["__WARNING_DISPATCH_MISMATCH", "28168", "__WARNING_DISPATCH_MISSING", "28169"]) and - result = "lgtm[cpp/portedqueries/wrong-dispatch-table-assignment]" + result = "lgtm[cpp/drivers/wrong-dispatch-table-assignment]" + or + this.getRuleName() = any(["__WARNING_IRQ_SET_TOO_HIGH", "28150"]) and + result = "lgtm[cpp/drivers/irql-set-too-high]" + or + this.getRuleName() = any(["__WARNING_IRQ_SET_TOO_LOW", "28124"]) and + result = "lgtm[cpp/drivers/irql-set-too-low]" + or + this.getRuleName() = any(["__WARNING_INTERLOCKEDDECREMENT_MISUSE1", "28616"]) and + result = "lgtm[cpp/drivers/multithreaded-av-condition]" + or + this.getRuleName() = any(["__WARNING_PROTOTYPE_MISMATCH", "28127"]) and + result = "lgtm[cpp/drivers/routine-function-type-not-expected]" or result = "lgtm[" + this.getRuleName() + "]" }