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