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