Skip to content

Commit

Permalink
Modernize dataflow configurations
Browse files Browse the repository at this point in the history
  • Loading branch information
jketema committed Dec 1, 2023
1 parent d74222a commit bb8a497
Show file tree
Hide file tree
Showing 54 changed files with 511 additions and 501 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,14 @@
import cpp
import codingstandards.c.cert
import codingstandards.cpp.dataflow.DataFlow
import DataFlow::PathGraph
import NonArrayPointerToArrayIndexingExprFlow::PathGraph

/**
* A data-flow configuration that tracks flow from an `AddressOfExpr` of a variable
* of `PointerType` that is not also an `ArrayType` to a `PointerArithmeticOrArrayExpr`
*/
class NonArrayPointerToArrayIndexingExprConfig extends DataFlow::Configuration {
NonArrayPointerToArrayIndexingExprConfig() { this = "ArrayToArrayIndexConfig" }

override predicate isSource(DataFlow::Node source) {
module NonArrayPointerToArrayIndexingExprConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(AddressOfExpr ao, Type t |
source.asExpr() = ao and
not ao.getOperand() instanceof ArrayExpr and
Expand All @@ -35,15 +33,15 @@ class NonArrayPointerToArrayIndexingExprConfig extends DataFlow::Configuration {
)
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(PointerArithmeticOrArrayExpr ae |
sink.asExpr() = ae.getPointerOperand() and
not sink.asExpr() instanceof Literal and
not ae.isNonPointerOperandZero()
)
}

override predicate isBarrierOut(DataFlow::Node node) {
predicate isBarrierOut(DataFlow::Node node) {
// the default interprocedural data-flow model flows through any field or array assignment
// expressions to the qualifier (array base, pointer dereferenced, or qualifier) instead of the
// individual element or field that the assignment modifies. this default behaviour causes
Expand All @@ -63,6 +61,9 @@ class NonArrayPointerToArrayIndexingExprConfig extends DataFlow::Configuration {
}
}

module NonArrayPointerToArrayIndexingExprFlow =
DataFlow::Global<NonArrayPointerToArrayIndexingExprConfig>;

class PointerArithmeticOrArrayExpr extends Expr {
Expr operand;

Expand Down Expand Up @@ -101,9 +102,11 @@ class PointerArithmeticOrArrayExpr extends Expr {
predicate isNonPointerOperandZero() { operand.(Literal).getValue().toInt() = 0 }
}

from DataFlow::PathNode source, DataFlow::PathNode sink
from
NonArrayPointerToArrayIndexingExprFlow::PathNode source,
NonArrayPointerToArrayIndexingExprFlow::PathNode sink
where
not isExcluded(sink.getNode().asExpr(),
InvalidMemory2Package::doNotUsePointerArithmeticOnNonArrayObjectPointersQuery()) and
any(NonArrayPointerToArrayIndexingExprConfig cfg).hasFlowPath(source, sink)
NonArrayPointerToArrayIndexingExprFlow::flowPath(source, sink)
select sink, source, sink, "Pointer arithmetic on non-array object pointer."
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import cpp
import codingstandards.c.cert
import codingstandards.c.Pointers
import codingstandards.cpp.dataflow.TaintTracking
import DataFlow::PathGraph
import ScaledIntegerPointerArithmeticFlow::PathGraph

/**
* An expression which invokes the `offsetof` macro or `__builtin_offsetof` operation.
Expand Down Expand Up @@ -69,12 +69,10 @@ class ScaledIntegerExpr extends Expr {
* A data-flow configuration modeling data-flow from a `ScaledIntegerExpr` to a
* `PointerArithmeticExpr` where the pointer does not point to a 1-byte type.
*/
class ScaledIntegerPointerArithmeticConfig extends DataFlow::Configuration {
ScaledIntegerPointerArithmeticConfig() { this = "ScaledIntegerPointerArithmeticConfig" }
module ScaledIntegerPointerArithmeticConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ScaledIntegerExpr }

override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ScaledIntegerExpr }

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(PointerArithmeticExpr pa |
// exclude pointers to 1-byte types as they do not scale
pa.getPointer().getFullyConverted().getType().(DerivedType).getBaseType().getSize() != 1 and
Expand All @@ -83,9 +81,13 @@ class ScaledIntegerPointerArithmeticConfig extends DataFlow::Configuration {
}
}

from ScaledIntegerPointerArithmeticConfig config, DataFlow::PathNode src, DataFlow::PathNode sink
module ScaledIntegerPointerArithmeticFlow = DataFlow::Global<ScaledIntegerPointerArithmeticConfig>;

from
ScaledIntegerPointerArithmeticFlow::PathNode src,
ScaledIntegerPointerArithmeticFlow::PathNode sink
where
not isExcluded(sink.getNode().asExpr(),
Pointers2Package::doNotAddOrSubtractAScaledIntegerToAPointerQuery()) and
config.hasFlowPath(src, sink)
ScaledIntegerPointerArithmeticFlow::flowPath(src, sink)
select sink, src, sink, "Scaled integer used in pointer arithmetic."
16 changes: 8 additions & 8 deletions c/cert/src/rules/CON30-C/CleanUpThreadSpecificStorage.ql
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ import codingstandards.cpp.Concurrency
import codingstandards.cpp.dataflow.TaintTracking
import codingstandards.cpp.dataflow.DataFlow

class TssCreateToTssDeleteDataFlowConfiguration extends DataFlow::Configuration {
TssCreateToTssDeleteDataFlowConfiguration() { this = "TssCreateToTssDeleteDataFlowConfiguration" }

override predicate isSource(DataFlow::Node node) {
module TssCreateToTssDeleteConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node node) {
exists(TSSCreateFunctionCall tsc, Expr e |
// the only requirement of the source is that at some point
// it refers to the key of a create statement
Expand All @@ -30,7 +28,7 @@ class TssCreateToTssDeleteDataFlowConfiguration extends DataFlow::Configuration
)
}

override predicate isSink(DataFlow::Node node) {
predicate isSink(DataFlow::Node node) {
exists(TSSDeleteFunctionCall tsd, Expr e |
// the only requirement of a sink is that at some point
// it references the key of a delete call.
Expand All @@ -40,15 +38,17 @@ class TssCreateToTssDeleteDataFlowConfiguration extends DataFlow::Configuration
}
}

module TssCreateToTssDeleteFlow = DataFlow::Global<TssCreateToTssDeleteConfig>;

from TSSCreateFunctionCall tcfc
where
not isExcluded(tcfc, Concurrency4Package::cleanUpThreadSpecificStorageQuery()) and
// all calls to `tss_create` must be bookended by calls to tss_delete
// even if a thread is not created.
not exists(TssCreateToTssDeleteDataFlowConfiguration config |
config.hasFlow(DataFlow::definitionByReferenceNodeFromArgument(tcfc.getKey()), _)
not (
TssCreateToTssDeleteFlow::flow(DataFlow::definitionByReferenceNodeFromArgument(tcfc.getKey()), _)
or
config.hasFlow(DataFlow::exprNode(tcfc.getKey()), _)
TssCreateToTssDeleteFlow::flow(DataFlow::exprNode(tcfc.getKey()), _)
)
or
// if a thread is created, we must check additional items
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import codingstandards.cpp.Alignment
import codingstandards.cpp.dataflow.DataFlow
import codingstandards.cpp.dataflow.DataFlow2
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
import DataFlow::PathGraph
import ExprWithAlignmentToCStyleCastFlow::PathGraph

/**
* An expression with a type that has defined alignment requirements
Expand Down Expand Up @@ -96,8 +96,7 @@ class UnconvertedCastFromNonVoidPointerExpr extends Expr {
*/
class DefaultAlignedPointerExpr extends UnconvertedCastFromNonVoidPointerExpr, ExprWithAlignment {
DefaultAlignedPointerExpr() {
not any(AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig config)
.hasFlowTo(DataFlow::exprNode(this))
not AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprFlow::flowTo(DataFlow::exprNode(this))
}

override int getAlignment() { result = this.getType().(PointerType).getBaseType().getAlignment() }
Expand All @@ -118,43 +117,37 @@ class DefaultAlignedPointerExpr extends UnconvertedCastFromNonVoidPointerExpr, E
* to exclude an `DefaultAlignedPointerAccessExpr` as a source if a preceding source
* defined by this configuration provides more accurate alignment information.
*/
class AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig extends DataFlow2::Configuration
module AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig implements
DataFlow::ConfigSig
{
AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig() {
this = "AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig"
}

override predicate isSource(DataFlow::Node source) {
predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof AddressOfAlignedVariableExpr or
source.asExpr() instanceof DefinedAlignmentAllocationExpr
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
sink.asExpr() instanceof UnconvertedCastFromNonVoidPointerExpr
}
}

module AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprFlow =
DataFlow::Global<AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig>;

/**
* A data-flow configuration for analysing the flow of `ExprWithAlignment` pointer expressions
* to casts which perform pointer type conversions and potentially create pointer alignment issues.
*/
class ExprWithAlignmentToCStyleCastConfiguration extends DataFlow::Configuration {
ExprWithAlignmentToCStyleCastConfiguration() {
this = "ExprWithAlignmentToCStyleCastConfiguration"
}
module ExprWithAlignmentToCStyleCastConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source.asExpr() instanceof ExprWithAlignment }

override predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof ExprWithAlignment
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(CStyleCast cast |
cast.getUnderlyingType() instanceof PointerType and
cast.getUnconverted() = sink.asExpr()
)
}

override predicate isBarrierOut(DataFlow::Node node) {
predicate isBarrierOut(DataFlow::Node node) {
// the default interprocedural data-flow model flows through any array assignment expressions
// to the qualifier (array base or pointer dereferenced) instead of the individual element
// that the assignment modifies. this default behaviour causes false positives for any future
Expand All @@ -169,12 +162,15 @@ class ExprWithAlignmentToCStyleCastConfiguration extends DataFlow::Configuration
}
}

module ExprWithAlignmentToCStyleCastFlow = DataFlow::Global<ExprWithAlignmentToCStyleCastConfig>;

from
DataFlow::PathNode source, DataFlow::PathNode sink, ExprWithAlignment expr, CStyleCast cast,
ExprWithAlignmentToCStyleCastFlow::PathNode source,
ExprWithAlignmentToCStyleCastFlow::PathNode sink, ExprWithAlignment expr, CStyleCast cast,
Type toBaseType, int alignmentFrom, int alignmentTo
where
not isExcluded(cast, Pointers3Package::doNotCastPointerToMoreStrictlyAlignedPointerTypeQuery()) and
any(ExprWithAlignmentToCStyleCastConfiguration config).hasFlowPath(source, sink) and
ExprWithAlignmentToCStyleCastFlow::flowPath(source, sink) and
source.getNode().asExpr() = expr and
sink.getNode().asExpr() = cast.getUnconverted() and
toBaseType = cast.getActualType().(PointerType).getBaseType() and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import cpp
import codingstandards.c.cert
import codingstandards.cpp.dataflow.DataFlow
import DataFlow::PathGraph
import SuspectFunctionPointerToCallFlow::PathGraph

/**
* An expression of type `FunctionPointer` which is the unconverted expression of a cast
Expand All @@ -37,26 +37,26 @@ class SuspiciousFunctionPointerCastExpr extends Expr {
* Data-flow configuration for flow from a `SuspiciousFunctionPointerCastExpr`
* to a call of the function pointer resulting from the function pointer cast
*/
class SuspectFunctionPointerToCallConfig extends DataFlow::Configuration {
SuspectFunctionPointerToCallConfig() { this = "SuspectFunctionPointerToCallConfig" }

override predicate isSource(DataFlow::Node src) {
module SuspectFunctionPointerToCallConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) {
src.asExpr() instanceof SuspiciousFunctionPointerCastExpr
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(VariableCall call | sink.asExpr() = call.getExpr().(VariableAccess))
}
}

module SuspectFunctionPointerToCallFlow = DataFlow::Global<SuspectFunctionPointerToCallConfig>;

from
SuspectFunctionPointerToCallConfig config, DataFlow::PathNode src, DataFlow::PathNode sink,
SuspectFunctionPointerToCallFlow::PathNode src, SuspectFunctionPointerToCallFlow::PathNode sink,
Access access
where
not isExcluded(src.getNode().asExpr(),
ExpressionsPackage::doNotCallFunctionPointerWithIncompatibleTypeQuery()) and
access = src.getNode().asExpr() and
config.hasFlowPath(src, sink)
SuspectFunctionPointerToCallFlow::flowPath(src, sink)
select src, src, sink,
"Incompatible function $@ assigned to function pointer is eventually called through the pointer.",
access.getTarget(), access.getTarget().getName()
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import cpp
import codingstandards.c.cert
import codingstandards.cpp.dataflow.DataFlow
import semmle.code.cpp.controlflow.Dominance
import DataFlow::PathGraph
import IndirectCastFlow::PathGraph

/**
* The standard function `memset` and its assorted variants
Expand Down Expand Up @@ -62,15 +62,15 @@ class IndirectCastReallocatedFlowState extends DataFlow::FlowState {
* other cast expressions or to dereferences of pointers reallocated with a call
* to `realloc` but not cleared via a function call to `memset`.
*/
class IndirectCastConfiguration extends DataFlow::Configuration {
IndirectCastConfiguration() { this = "CastToIncompatibleTypeConfiguration" }
module IndirectCastConfig implements DataFlow::StateConfigSig {
class FlowState = DataFlow::FlowState;

override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
predicate isSource(DataFlow::Node source, FlowState state) {
state instanceof IndirectCastDefaultFlowState and
source.asExpr() instanceof IndirectCastAnalysisUnconvertedCastExpr
}

override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
predicate isSink(DataFlow::Node sink, FlowState state) {
sink.asExpr() instanceof IndirectCastAnalysisUnconvertedCastExpr and
state instanceof IndirectCastDefaultFlowState
or
Expand Down Expand Up @@ -103,17 +103,16 @@ class IndirectCastConfiguration extends DataFlow::Configuration {
)
}

override predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) {
predicate isBarrier(DataFlow::Node node, FlowState state) {
state instanceof IndirectCastReallocatedFlowState and
exists(FunctionCall fc |
fc.getTarget() instanceof MemsetFunction and
fc.getArgument(0) = node.asExpr()
)
}

override predicate isAdditionalFlowStep(
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
DataFlow::FlowState state2
predicate isAdditionalFlowStep(
DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
) {
// track pointer flow through realloc calls and update state to `IndirectCastReallocatedFlowState`
state1 instanceof IndirectCastDefaultFlowState and
Expand All @@ -135,6 +134,8 @@ class IndirectCastConfiguration extends DataFlow::Configuration {
}
}

module IndirectCastFlow = DataFlow::GlobalWithState<IndirectCastConfig>;

pragma[inline]
predicate areTypesSameExceptForConstSpecifiers(Type a, Type b) {
a.stripType() = b.stripType() and
Expand Down Expand Up @@ -190,12 +191,14 @@ Type compatibleTypes(Type type) {
)
}

from DataFlow::PathNode source, DataFlow::PathNode sink, Cast cast, Type fromType, Type toType
from
IndirectCastFlow::PathNode source, IndirectCastFlow::PathNode sink, Cast cast, Type fromType,
Type toType
where
not isExcluded(sink.getNode().asExpr(),
Pointers3Package::doNotAccessVariableViaPointerOfIncompatibleTypeQuery()) and
cast.getFile().compiledAsC() and
any(IndirectCastConfiguration config).hasFlowPath(source, sink) and
IndirectCastFlow::flowPath(source, sink) and
// include only sinks which are not a compatible type to the associated source
source.getNode().asExpr() = cast.getUnconverted() and
fromType = cast.getUnconverted().getType().(PointerType).getBaseType() and
Expand Down
16 changes: 8 additions & 8 deletions c/cert/src/rules/EXP40-C/DoNotModifyConstantObjects.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import cpp
import codingstandards.c.cert
import codingstandards.cpp.dataflow.DataFlow
import DataFlow::PathGraph
import CastFlow::PathGraph
import codingstandards.cpp.SideEffect

class ConstRemovingCast extends Cast {
Expand All @@ -32,23 +32,23 @@ class MaybeReturnsStringLiteralFunctionCall extends FunctionCall {
}
}

class MyDataFlowConfCast extends DataFlow::Configuration {
MyDataFlowConfCast() { this = "MyDataFlowConfCast" }

override predicate isSource(DataFlow::Node source) {
module CastConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source.asExpr().getFullyConverted() instanceof ConstRemovingCast
or
source.asExpr().getFullyConverted() = any(MaybeReturnsStringLiteralFunctionCall c)
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
sink.asExpr() = any(Assignment a).getLValue().(PointerDereferenceExpr).getOperand()
}
}

from MyDataFlowConfCast conf, DataFlow::PathNode src, DataFlow::PathNode sink
module CastFlow = DataFlow::Global<CastConfig>;

from CastFlow::PathNode src, CastFlow::PathNode sink
where
conf.hasFlowPath(src, sink)
CastFlow::flowPath(src, sink)
or
sink.getNode()
.asExpr()
Expand Down
Loading

0 comments on commit bb8a497

Please sign in to comment.