Skip to content

Commit

Permalink
update uses of dataflow to use latest library (#155)
Browse files Browse the repository at this point in the history
* update uses of dataflow to use latest library

* fix dataflow use

* bug fix

* Update build-codeql.yaml

Signed-off-by: Jacob Ronstadt <[email protected]>

* update pack versions to fix false positive

---------

Signed-off-by: Jacob Ronstadt <[email protected]>
  • Loading branch information
jacob-ronstadt authored Jan 10, 2025
1 parent 3ef295d commit 2a7c167
Show file tree
Hide file tree
Showing 21 changed files with 185 additions and 198 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build-codeql.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ on:
workflow_dispatch:

env:
CODEQL_VERSION: 2.15.4
CODEQL_VERSION: 2.20.1

jobs:
build:
Expand Down
20 changes: 13 additions & 7 deletions src/codeql-pack.lock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,23 @@
lockVersion: 1.0.0
dependencies:
codeql/cpp-all:
version: 0.12.1
version: 3.1.0
codeql/dataflow:
version: 0.1.4
version: 1.1.8
codeql/mad:
version: 1.0.14
codeql/rangeanalysis:
version: 0.0.3
version: 1.0.14
codeql/ssa:
version: 0.2.4
version: 1.0.14
codeql/tutorial:
version: 0.2.4
version: 1.0.14
codeql/typeflow:
version: 1.0.14
codeql/typetracking:
version: 0.2.4
version: 1.0.14
codeql/util:
version: 0.2.4
version: 2.0.1
codeql/xml:
version: 1.0.14
compiled: false
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class KernelFloatAnnotatedTypedef extends TypedefType {
class KernelFloatAnnotatedFunction extends Function {
KernelFloatFunctionAnnotation kernelFloatAnnotation;

cached

KernelFloatAnnotatedFunction() {
(
// this.hasCLinkage() and
Expand Down
2 changes: 1 addition & 1 deletion src/drivers/general/queries/FloatSafeExit/FloatSafeExit.ql
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class KernelFloatAnnotatedTypedef extends TypedefType {
class KernelFloatAnnotatedFunction extends Function {
KernelFloatFunctionAnnotation kernelFloatAnnotation;

cached

KernelFloatAnnotatedFunction() {
(
// this.hasCLinkage() and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class KernelFloatAnnotatedTypedef extends TypedefType {
class KernelFloatAnnotatedFunction extends Function {
KernelFloatFunctionAnnotation kernelFloatAnnotation;

cached

KernelFloatAnnotatedFunction() {
(
// this.hasCLinkage() and
Expand Down
40 changes: 20 additions & 20 deletions src/drivers/general/queries/IrqlNotSaved/IrqlNotSaved.ql
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,19 @@
import cpp
import drivers.libraries.Irql
import semmle.code.cpp.dataflow.new.DataFlow
import semmle.code.cpp.dataflow.new.DataFlow2

/**
* A data-flow configuration describing flow from an
* \_IRQL\_saves\_-annotated parameter to an OS function that restores
* the IRQL.
*/
class IrqlFlowConfiguration extends DataFlow::Configuration {
IrqlFlowConfiguration() { this = "IrqlFlowConfiguration" }
module IrqlFlowConfigurationConfig implements DataFlow::ConfigSig {

override predicate isSource(DataFlow::Node source) {
predicate isSource(DataFlow::Node source) {
source.asParameter() instanceof IrqlSaveParameter
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(FunctionCall fc, FundamentalIrqlSaveFunction fisf |
fc.getTarget() = fisf and
(
Expand All @@ -50,6 +48,8 @@ class IrqlFlowConfiguration extends DataFlow::Configuration {
}
}

module IrqlFlowConfiguration = DataFlow::Global<IrqlFlowConfigurationConfig>;

/**
* A function that we know will restore the IRQL, i.e. one defined
* by the Windows OS itself. This is in general in a Windows Kits header. For
Expand All @@ -75,38 +75,38 @@ class FundamentalIrqlSaveFunction extends IrqlSavesFunction {
/**
* A simple data flow from any IrqlSaveParameter.
*/
class IrqlSaveParameterFlowConfiguration extends DataFlow2::Configuration {
IrqlSaveParameterFlowConfiguration() { this = "IrqlSaveParameterFlowConfiguration" }
module IrqlSaveParameterFlowConfigurationConfig implements DataFlow::ConfigSig {

override predicate isSource(DataFlow::Node source) {
predicate isSource(DataFlow::Node source) {
source.asParameter() instanceof IrqlSaveParameter
}

override predicate isSink(DataFlow::Node sink) { sink instanceof DataFlow::Node }
predicate isSink(DataFlow::Node sink) { sink instanceof DataFlow::Node }
}
module IrqlSaveParameterFlowConfiguration = DataFlow::Global<IrqlSaveParameterFlowConfigurationConfig>;


/**
* A data-flow configuration representing flow from an
* OS function that returns an IRQL to be saved to a parameter marked
* \_IRQL\_saves\_ (or a variable aliasing that parameter.)
*/
class IrqlAssignmentFlowConfiguration extends DataFlow::Configuration {
IrqlAssignmentFlowConfiguration() { this = "IrqlAssignmentFlowConfiguration" }
module IrqlAssignmentFlowConfigurationConfig implements DataFlow::ConfigSig {

override predicate isSource(DataFlow::Node source) {
predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof FunctionCall and
source.asExpr().(FunctionCall).getTarget() instanceof FundamentalIrqlSaveFunction and
source.asExpr().(FunctionCall).getTarget() instanceof IrqlSavesViaReturnFunction
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(Assignment a |
a.getLValue().getAChild*().(VariableAccess).getTarget() instanceof IrqlSaveVariableFlowedTo and
a.getRValue() = sink.asExpr()
)
}
}

module IrqlAssignmentFlowConfiguration = DataFlow::Global<IrqlAssignmentFlowConfigurationConfig>;
/**
* A variable that is either a parameter annotated \_IRQL\_saves\_
* or a variable which contains the value from a parameter annotated as such.
Expand All @@ -116,14 +116,14 @@ class IrqlSaveVariableFlowedTo extends Variable {

IrqlSaveVariableFlowedTo() {
exists(
IrqlSaveParameterFlowConfiguration ispfc, DataFlow::Node parameter, DataFlow::Node assignment
DataFlow::Node parameter, DataFlow::Node assignment
|
(
this.getAnAssignedValue() = assignment.asExpr() or
this = assignment.asParameter()
) and
parameter.asParameter() = isp and
ispfc.hasFlow(parameter, assignment)
IrqlSaveParameterFlowConfiguration::flow(parameter, assignment)
)
or
this = isp
Expand All @@ -142,19 +142,19 @@ where
*/

not exists(
DataFlow::Node node, IrqlSaveVariableFlowedTo isvft, IrqlAssignmentFlowConfiguration iafc
DataFlow::Node node, IrqlSaveVariableFlowedTo isvft
|
isvft.getSaveParameter() = isp and
exists(Assignment a |
a.getLValue().getAChild*().(VariableAccess).getTarget() = isvft and
a.getRValue() = node.asExpr()
) and
iafc.hasFlow(_, node)
IrqlAssignmentFlowConfiguration::flow(_, 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 |
not exists(DataFlow::Node node |
node.asParameter() = isp and
ifc.hasFlow(node, _)
IrqlFlowConfiguration::flow(node, _)
)
select isp, "The parameter $@ is annotated \"_IRQL_saves_\" but never has the IRQL saved to it.",
isp, isp.getName()
22 changes: 10 additions & 12 deletions src/drivers/general/queries/IrqlNotUsed/IrqlNotUsed.ql
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import cpp
import drivers.libraries.Irql
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\_".
Expand Down Expand Up @@ -61,14 +60,12 @@ class FundamentalIrqlRestoreFunction extends IrqlRestoreFunction {
* _IRQL_restores_-annotated parameter to an OS function that restores
* the IRQL.
*/
class IrqlFlowConfiguration extends DataFlow::Configuration {
IrqlFlowConfiguration() { this = "IrqlFlowConfiguration" }

override predicate isSource(DataFlow::Node source) {
module IrqlFlowConfigurationConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source.asParameter() instanceof IrqlRestoreParameter
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(FunctionCall fc, FundamentalIrqlRestoreFunction firf |
fc.getTarget() = firf and
(
Expand All @@ -79,21 +76,22 @@ class IrqlFlowConfiguration extends DataFlow::Configuration {
)
}
}
module IrqlFlowConfiguration = DataFlow::Global<IrqlFlowConfigurationConfig>;

from IrqlRestoreFunction irf, IrqlFlowConfiguration ifc
from IrqlRestoreFunction irf
where
// Exclude OS functions
not irf instanceof FundamentalIrqlRestoreFunction and
(
// Account for case where parameter is touched but has no path to restore the IRQL
exists(DataFlow::PathNode source |
source.getNode().asParameter() = irf.getRestoreParameter() and
not ifc.hasFlowPath(source, _)
exists(DataFlow::Node source |
source.asParameter() = irf.getRestoreParameter() and
not IrqlFlowConfiguration::flow(source, _)
)
or
// Account for case where parameter is totally untouched
not exists(DataFlow::PathNode source |
source.getNode().asParameter() = irf.getRestoreParameter()
not exists(DataFlow::Node source |
source.asParameter() = irf.getRestoreParameter()
)
)
select irf,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
* @scope domainspecific
* @query-version v1
*/

import cpp
import semmle.code.cpp.dataflow.DataFlow
import semmle.code.cpp.dataflow.new.DataFlow

/** A pool allocation function (has a ULONG "Tag" field, a "Flags" field, and a size parameter.) */
class PoolTypeFunction extends Function {
Expand All @@ -28,8 +29,8 @@ class PoolTypeFunction extends Function {
p.getName().matches("Tag") and
p.getType().getName().matches("ULONG")
) and
this.getAParameter().getName().matches("Flags")
and this.getAParameter().getType().getName().matches("SIZE_T")
this.getAParameter().getName().matches("Flags") and
this.getAParameter().getType().getName().matches("SIZE_T")
}
}

Expand All @@ -47,25 +48,24 @@ class GlobalDefaultPoolTag extends GlobalVariable {
}

/** An interprocedural data-flow analysis looking for flow from bad (default) pool tags. */
class DefaultPoolTagFlow extends DataFlow::Configuration {
DefaultPoolTagFlow() { this = "DefaultPoolTagFlow" }

override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof DefaultPoolTag }
module DefaultPoolTagFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source.asExpr() instanceof DefaultPoolTag }

override predicate isSink(DataFlow::Node sink) { sink instanceof DataFlow::ExprNode }
predicate isSink(DataFlow::Node sink) { sink instanceof DataFlow::ExprNode }
}
module DefaultPoolTagFlow = DataFlow::Global<DefaultPoolTagFlowConfig>;

/** An interprocedural data-flow analysis looking for flow from good pool tags. */
class ValidPoolTagFlow extends DataFlow::Configuration {
ValidPoolTagFlow() { this = "ValidPoolTagFlow" }

override predicate isSource(DataFlow::Node source) {
/** An interprocedural data-flow analysis looking for flow from good pool tags. */
module ValidPoolTagFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof Literal and
not source.asExpr() instanceof DefaultPoolTag
}

override predicate isSink(DataFlow::Node sink) { sink instanceof DataFlow::ExprNode }
predicate isSink(DataFlow::Node sink) { sink instanceof DataFlow::ExprNode }
}
module ValidPoolTagFlow = DataFlow::Global<ValidPoolTagFlowConfig>;

from FunctionCall fc, int i, GlobalDefaultPoolTag gdpt
where
Expand All @@ -76,17 +76,17 @@ where
// A bad tag is directly passed in
fc.getArgument(i) instanceof DefaultPoolTag
or
// A global tag variable is being passed in, and no path exists
// A global tag variable is being passed in, and no path exists
// where a good tag has been assigned instead
fc.getArgument(i).(VariableAccess).getTarget() = gdpt and
not exists(ValidPoolTagFlow dataFlow, DataFlow::Node source, DataFlow::Node sink |
not exists(DataFlow::Node source, DataFlow::Node sink |
sink.asExpr() = fc.getArgument(i) and
dataFlow.hasFlow(source, sink)
ValidPoolTagFlow::flow(source, sink)
)
or
// A local variable with a bad tag is being passed in
exists(DefaultPoolTagFlow dataFlow, DataFlow::Node source, DataFlow::Node sink |
exists(DataFlow::Node source, DataFlow::Node sink |
sink.asExpr() = fc.getArgument(i) and
dataFlow.hasFlow(source, sink)
DefaultPoolTagFlow::flow(source, sink)
)
select fc.getArgument(i), "Default pool tag used in function call"
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
*/

import drivers.kmdf.libraries.KmdfDrivers
import semmle.code.cpp.dataflow.DataFlow
import DataFlow::PathGraph
import semmle.code.cpp.dataflow.new.DataFlow

/** A function that initializes or changes a WDFDEVICE_INIT struct, and which must not be called after WDFDeviceCreate. */
class WdfInitializationApi extends Function {
Expand Down Expand Up @@ -73,24 +72,23 @@ predicate isChildExpr(Expr e, FunctionCall func) {
* A data-flow model to determine if a use of a WDFDEVICE_INIT struct is
* used in an initialization function after WdfDeviceCreate is called.
*/
class InitAPIDataFlow extends DataFlow::Configuration {
InitAPIDataFlow() { this = "KMDFDeviceInitApiFlow" }
module InitAPIDataFlowConfig implements DataFlow::ConfigSig {

override predicate isSource(DataFlow::Node source) {
predicate isSource(DataFlow::Node source) {
exists(FunctionCall fc |
fc.getTarget().getName().matches("WdfDeviceCreate") and
fc.getArgument(0).getAChild*() = source.asExpr()
)
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(FunctionCall fc |
fc.getTarget() instanceof WdfInitializationApi and
fc.getArgument(0).getAChild*() = sink.asExpr()
)
}

override predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node sink) {
predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node sink) {
exists(FunctionCall fc |
fc.getTarget().getName().matches("WdfDeviceCreate") and
fc.getTarget() = sink.getFunction() and
Expand All @@ -99,13 +97,16 @@ class InitAPIDataFlow extends DataFlow::Configuration {
}
}

from InitAPIDataFlow iadf, DataFlow::PathNode e1, DataFlow::PathNode e2
module InitAPIDataFlow = DataFlow::Global<InitAPIDataFlowConfig>;
import InitAPIDataFlow::PathGraph

from InitAPIDataFlow::PathNode e1, InitAPIDataFlow::PathNode e2
where
exists(FunctionCall driverCreateCall, WdfInitiailzationApiCall apiCall |
driverCreateCall.getAChild*() = e1.getNode().asExpr() and
isChildExpr(e2.getNode().asExpr(), apiCall) and
driverCreateCall.getASuccessor*() = apiCall
) and
iadf.hasFlowPath(e1, e2)
InitAPIDataFlow::flowPath(e1, e2)
select e1.getNode(), e1, e2,
"A WDF device object initialization method was called after WdfDeviceCreate was called on the same WDFDEVICE_INIT struct. This can lead to system instability."
Loading

0 comments on commit 2a7c167

Please sign in to comment.