-
Notifications
You must be signed in to change notification settings - Fork 28
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add KeSetEventIrql and KeSetEventPaged queries and update IRQL librar…
…y. (#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 <[email protected]> Signed-off-by: Jacob Ronstadt <[email protected]> Co-authored-by: jacob-ronstadt <[email protected]> Co-authored-by: Jacob Ronstadt <[email protected]>
- Loading branch information
1 parent
984e2ad
commit c4bf4f8
Showing
14 changed files
with
1,040 additions
and
62 deletions.
There are no files selected for viewing
36 changes: 36 additions & 0 deletions
36
src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.ql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 [email protected] | ||
* @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() |
23 changes: 23 additions & 0 deletions
23
src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.qlhelp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd"> | ||
<qhelp> | ||
<overview> | ||
<p> | ||
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. | ||
</p> | ||
</overview> | ||
<recommendation> | ||
<p> | ||
Adjust the KeSetEvent call to pass FALSE to the wait parameter. | ||
</p> | ||
</recommendation> | ||
<example> | ||
<sample src="driver_snippet.c" /> | ||
</example> | ||
<references> | ||
<li> | ||
<a href="https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-kesetevent"> | ||
KeSetEvent (MSDN) | ||
</a> | ||
</li> | ||
</references> | ||
</qhelp> |
265 changes: 265 additions & 0 deletions
265
src/drivers/general/queries/KeSetEventPaged/KeSetEventPaged.sarif
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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" : "[email protected]", | ||
"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" | ||
} | ||
} ] | ||
} |
58 changes: 58 additions & 0 deletions
58
src/drivers/general/queries/KeSetEventPaged/driver_snippet.c
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 <wdm.h> | ||
|
||
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 |
Oops, something went wrong.