From bb8a49793c831eb7c8019cc4628a7eedc5481906 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Fri, 1 Dec 2023 21:25:45 +0100 Subject: [PATCH] Modernize dataflow configurations --- ...interArithmeticOnNonArrayObjectPointers.ql | 21 ++++++---- ...otAddOrSubtractAScaledIntegerToAPointer.ql | 18 ++++---- .../CON30-C/CleanUpThreadSpecificStorage.ql | 16 +++---- ...PointerToMoreStrictlyAlignedPointerType.ql | 40 ++++++++---------- ...CallFunctionPointerWithIncompatibleType.ql | 16 +++---- ...essVariableViaPointerOfIncompatibleType.ql | 25 ++++++----- .../EXP40-C/DoNotModifyConstantObjects.ql | 16 +++---- ...sAliasedPointerToRestrictQualifiedParam.ql | 24 ++++++----- ...trictPointerReferencesOverlappingObject.ql | 18 ++++---- ...uesForFsetposThatAreReturnedFromFgetpos.ql | 12 +++--- ...DoNotModifyAlignmentOfMemoryWithRealloc.ql | 16 +++---- ...oNotPassInvalidDataToTheAsctimeFunction.ql | 14 +++---- ...ArgOnAVaListThatHasAnIndeterminateValue.ql | 16 +++---- .../DoNotAttemptToModifyStringLiterals.ql | 18 ++++---- .../src/codingstandards/c/OutOfBounds.qll | 21 +++++----- ...PointersAddressingDifferentArrays.expected | 16 ++++--- ...ionalOperatorsWithDifferingArrays.expected | 24 +++++++---- .../ArrayFunctionArgumentNumberOfElements.ql | 17 ++++---- ...emcmpUsedToCompareNullTerminatedStrings.ql | 27 ++++++------ .../AttemptToWriteToAReadOnlyStream.ql | 14 +++---- ...allBeComparedWithUnmodifiedReturnValues.ql | 16 +++---- ...AnElementOfAnArrayPassedToASmartPointer.ql | 25 ++++++----- ...hmeticUsedWithPointersToNonFinalClasses.ql | 22 +++++----- .../rules/A5-1-7/LambdaPassedToDecltype.ql | 15 +++---- .../src/rules/A5-1-7/LambdaPassedToTypeid.ql | 18 ++++---- .../PointerSubtractionOnDifferentArrays.ql | 25 +++++------ ...entOfAnArrayPassedToASmartPointer.expected | 4 +- ...sePointerArithmeticOnPolymorphicObjects.ql | 22 +++++----- ...nArrayThroughAPointerOfTheIncorrectType.ql | 16 +++---- .../DetectAndHandleMemoryAllocationErrors.ql | 14 +++---- .../MEM53-CPP/ManuallyManagedLifetime.qll | 27 ++++++------ ...ConstructorCallForManuallyManagedObject.ql | 6 +-- ...gDestructorCallForManuallyManagedObject.ql | 8 ++-- .../src/codingstandards/cpp/Concurrency.qll | 14 +++---- .../src/codingstandards/cpp/Nullness.qll | 24 +++++------ .../cpp/allocations/PlacementNew.qll | 12 +++--- ...essOfUndefinedMemberThroughNullPointer.qll | 10 +++-- ...emberThroughUninitializedStaticPointer.qll | 4 +- .../ConstLikeReturnValue.qll | 20 ++++----- ...tractPointersAddressingDifferentArrays.qll | 29 +++++++------ ...nterArithmeticToAddressDifferentArrays.qll | 16 ++++--- ...RelationalOperatorsWithDifferingArrays.qll | 29 +++++++------ .../nonconstantformat/NonConstantFormat.qll | 16 +++---- ...lyFreeMemoryAllocatedDynamicallyShared.qll | 42 ++++++++----------- ...nterValueStoredInUnrelatedSmartPointer.qll | 26 ++++++------ .../PlacementNewInsufficientStorage.qll | 19 ++++----- .../PlacementNewNotProperlyAligned.qll | 27 ++++++------ ...tringNumberConversionMissingErrorCheck.qll | 16 +++---- .../ThrowingOperatorNewReturnsNull.qll | 16 +++---- ...eOnlyArrayIndexingForPointerArithmetic.qll | 15 +++---- .../cpp/standardlibrary/FileStreams.qll | 18 ++++---- ...PointersAddressingDifferentArrays.expected | 16 ++++--- ...ionalOperatorsWithDifferingArrays.expected | 24 +++++++---- ...alueStoredInUnrelatedSmartPointer.expected | 12 +++++- 54 files changed, 511 insertions(+), 501 deletions(-) diff --git a/c/cert/src/rules/ARR37-C/DoNotUsePointerArithmeticOnNonArrayObjectPointers.ql b/c/cert/src/rules/ARR37-C/DoNotUsePointerArithmeticOnNonArrayObjectPointers.ql index 1abc2ad882..2f8ecec25d 100644 --- a/c/cert/src/rules/ARR37-C/DoNotUsePointerArithmeticOnNonArrayObjectPointers.ql +++ b/c/cert/src/rules/ARR37-C/DoNotUsePointerArithmeticOnNonArrayObjectPointers.ql @@ -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 @@ -35,7 +33,7 @@ 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 @@ -43,7 +41,7 @@ class NonArrayPointerToArrayIndexingExprConfig extends DataFlow::Configuration { ) } - 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 @@ -63,6 +61,9 @@ class NonArrayPointerToArrayIndexingExprConfig extends DataFlow::Configuration { } } +module NonArrayPointerToArrayIndexingExprFlow = + DataFlow::Global; + class PointerArithmeticOrArrayExpr extends Expr { Expr operand; @@ -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." diff --git a/c/cert/src/rules/ARR39-C/DoNotAddOrSubtractAScaledIntegerToAPointer.ql b/c/cert/src/rules/ARR39-C/DoNotAddOrSubtractAScaledIntegerToAPointer.ql index 5606c8485f..c641c17124 100644 --- a/c/cert/src/rules/ARR39-C/DoNotAddOrSubtractAScaledIntegerToAPointer.ql +++ b/c/cert/src/rules/ARR39-C/DoNotAddOrSubtractAScaledIntegerToAPointer.ql @@ -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. @@ -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 @@ -83,9 +81,13 @@ class ScaledIntegerPointerArithmeticConfig extends DataFlow::Configuration { } } -from ScaledIntegerPointerArithmeticConfig config, DataFlow::PathNode src, DataFlow::PathNode sink +module ScaledIntegerPointerArithmeticFlow = DataFlow::Global; + +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." diff --git a/c/cert/src/rules/CON30-C/CleanUpThreadSpecificStorage.ql b/c/cert/src/rules/CON30-C/CleanUpThreadSpecificStorage.ql index 4b31b89023..59fab6e455 100644 --- a/c/cert/src/rules/CON30-C/CleanUpThreadSpecificStorage.ql +++ b/c/cert/src/rules/CON30-C/CleanUpThreadSpecificStorage.ql @@ -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 @@ -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. @@ -40,15 +38,17 @@ class TssCreateToTssDeleteDataFlowConfiguration extends DataFlow::Configuration } } +module TssCreateToTssDeleteFlow = DataFlow::Global; + 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 diff --git a/c/cert/src/rules/EXP36-C/DoNotCastPointerToMoreStrictlyAlignedPointerType.ql b/c/cert/src/rules/EXP36-C/DoNotCastPointerToMoreStrictlyAlignedPointerType.ql index b161beac1b..cada60d10f 100644 --- a/c/cert/src/rules/EXP36-C/DoNotCastPointerToMoreStrictlyAlignedPointerType.ql +++ b/c/cert/src/rules/EXP36-C/DoNotCastPointerToMoreStrictlyAlignedPointerType.ql @@ -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 @@ -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() } @@ -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; + /** * 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 @@ -169,12 +162,15 @@ class ExprWithAlignmentToCStyleCastConfiguration extends DataFlow::Configuration } } +module ExprWithAlignmentToCStyleCastFlow = DataFlow::Global; + 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 diff --git a/c/cert/src/rules/EXP37-C/DoNotCallFunctionPointerWithIncompatibleType.ql b/c/cert/src/rules/EXP37-C/DoNotCallFunctionPointerWithIncompatibleType.ql index b68cfa8ce1..e28dbddaaf 100644 --- a/c/cert/src/rules/EXP37-C/DoNotCallFunctionPointerWithIncompatibleType.ql +++ b/c/cert/src/rules/EXP37-C/DoNotCallFunctionPointerWithIncompatibleType.ql @@ -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 @@ -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; + 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() diff --git a/c/cert/src/rules/EXP39-C/DoNotAccessVariableViaPointerOfIncompatibleType.ql b/c/cert/src/rules/EXP39-C/DoNotAccessVariableViaPointerOfIncompatibleType.ql index 784fb54b2f..1962c5b0b0 100644 --- a/c/cert/src/rules/EXP39-C/DoNotAccessVariableViaPointerOfIncompatibleType.ql +++ b/c/cert/src/rules/EXP39-C/DoNotAccessVariableViaPointerOfIncompatibleType.ql @@ -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 @@ -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 @@ -103,7 +103,7 @@ 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 @@ -111,9 +111,8 @@ class IndirectCastConfiguration extends DataFlow::Configuration { ) } - 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 @@ -135,6 +134,8 @@ class IndirectCastConfiguration extends DataFlow::Configuration { } } +module IndirectCastFlow = DataFlow::GlobalWithState; + pragma[inline] predicate areTypesSameExceptForConstSpecifiers(Type a, Type b) { a.stripType() = b.stripType() and @@ -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 diff --git a/c/cert/src/rules/EXP40-C/DoNotModifyConstantObjects.ql b/c/cert/src/rules/EXP40-C/DoNotModifyConstantObjects.ql index dbeffd8153..d79224435f 100644 --- a/c/cert/src/rules/EXP40-C/DoNotModifyConstantObjects.ql +++ b/c/cert/src/rules/EXP40-C/DoNotModifyConstantObjects.ql @@ -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 { @@ -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; + +from CastFlow::PathNode src, CastFlow::PathNode sink where - conf.hasFlowPath(src, sink) + CastFlow::flowPath(src, sink) or sink.getNode() .asExpr() diff --git a/c/cert/src/rules/EXP43-C/DoNotPassAliasedPointerToRestrictQualifiedParam.ql b/c/cert/src/rules/EXP43-C/DoNotPassAliasedPointerToRestrictQualifiedParam.ql index 2e96e25f9f..a4cc4e8944 100644 --- a/c/cert/src/rules/EXP43-C/DoNotPassAliasedPointerToRestrictQualifiedParam.ql +++ b/c/cert/src/rules/EXP43-C/DoNotPassAliasedPointerToRestrictQualifiedParam.ql @@ -115,25 +115,24 @@ int getPointerArithmeticOperandStatedValue(CallToFunctionWithRestrictParametersA result = 0 } -class PointerValueToRestrictArgConfig extends DataFlow::Configuration { - PointerValueToRestrictArgConfig() { this = "PointerValueToRestrictArgConfig" } +module PointerValueToRestrictArgConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { pointerValue(source.asExpr()) } - override predicate isSource(DataFlow::Node source) { pointerValue(source.asExpr()) } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(CallToFunctionWithRestrictParameters call | sink.asExpr() = call.getAPtrArg(_).getAChild*() ) } - override predicate isBarrierIn(DataFlow::Node node) { + predicate isBarrierIn(DataFlow::Node node) { exists(AddressOfExpr a | node.asExpr() = a.getOperand().getAChild*()) } } +module PointerValueToRestrictArgFlow = DataFlow::Global; + from - PointerValueToRestrictArgConfig config, CallToFunctionWithRestrictParameters call, - CallToFunctionWithRestrictParametersArgExpr arg1, + CallToFunctionWithRestrictParameters call, CallToFunctionWithRestrictParametersArgExpr arg1, CallToFunctionWithRestrictParametersArgExpr arg2, int argOffset1, int argOffset2, Expr source1, Expr source2, string sourceMessage1, string sourceMessage2 where @@ -144,17 +143,20 @@ where (not arg2 = call.getARestrictPtrArg() or arg2.getParamIndex() > arg1.getParamIndex()) and ( // check if two pointers address the same object - config.hasFlow(DataFlow::exprNode(source1), DataFlow::exprNode(arg1.getAChild*())) and + PointerValueToRestrictArgFlow::flow(DataFlow::exprNode(source1), + DataFlow::exprNode(arg1.getAChild*())) and ( // one pointer value flows to both args - config.hasFlow(DataFlow::exprNode(source1), DataFlow::exprNode(arg2.getAChild*())) and + PointerValueToRestrictArgFlow::flow(DataFlow::exprNode(source1), + DataFlow::exprNode(arg2.getAChild*())) and sourceMessage1 = "$@" and sourceMessage2 = "source" and source1 = source2 or // there are two separate values that flow from an AddressOfExpr of the same target getAddressOfExprTargetBase(source1) = getAddressOfExprTargetBase(source2) and - config.hasFlow(DataFlow::exprNode(source2), DataFlow::exprNode(arg2.getAChild*())) and + PointerValueToRestrictArgFlow::flow(DataFlow::exprNode(source2), + DataFlow::exprNode(arg2.getAChild*())) and sourceMessage1 = "a pair of address-of expressions ($@, $@)" and sourceMessage2 = "addressof1" and not source1 = source2 diff --git a/c/cert/src/rules/EXP43-C/RestrictPointerReferencesOverlappingObject.ql b/c/cert/src/rules/EXP43-C/RestrictPointerReferencesOverlappingObject.ql index 727bda754e..bbe41259b8 100644 --- a/c/cert/src/rules/EXP43-C/RestrictPointerReferencesOverlappingObject.ql +++ b/c/cert/src/rules/EXP43-C/RestrictPointerReferencesOverlappingObject.ql @@ -39,22 +39,20 @@ class AssignmentOrInitializationToRestrictPtrValueExpr extends Expr { * A data-flow configuration for tracking flow from an assignment or initialization to * an assignment to an `AssignmentOrInitializationToRestrictPtrValueExpr`. */ -class AssignedValueToRestrictPtrValueConfiguration extends DataFlow::Configuration { - AssignedValueToRestrictPtrValueConfiguration() { - this = "AssignmentOrInitializationToRestrictPtrValueConfiguration" - } - - override predicate isSource(DataFlow::Node source) { +module AssignedValueToRestrictPtrValueConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { exists(Variable v | source.asExpr() = v.getAnAssignedValue()) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof AssignmentOrInitializationToRestrictPtrValueExpr } } +module AssignedValueToRestrictPtrValueFlow = + DataFlow::Global; + from - AssignedValueToRestrictPtrValueConfiguration config, AssignmentOrInitializationToRestrictPtrValueExpr expr, DataFlow::Node sourceValue, string sourceMessage where @@ -71,8 +69,8 @@ where exists(AssignmentOrInitializationToRestrictPtrValueExpr pre_expr | expr.getEnclosingBlock() = pre_expr.getEnclosingBlock() and ( - config.hasFlow(sourceValue, DataFlow::exprNode(pre_expr)) and - config.hasFlow(sourceValue, DataFlow::exprNode(expr)) and + AssignedValueToRestrictPtrValueFlow::flow(sourceValue, DataFlow::exprNode(pre_expr)) and + AssignedValueToRestrictPtrValueFlow::flow(sourceValue, DataFlow::exprNode(expr)) and sourceMessage = "the same source value" or // Expressions referring to the address of the same variable can also result in aliasing diff --git a/c/cert/src/rules/FIO44-C/OnlyUseValuesForFsetposThatAreReturnedFromFgetpos.ql b/c/cert/src/rules/FIO44-C/OnlyUseValuesForFsetposThatAreReturnedFromFgetpos.ql index 94f3238f26..33a906136f 100644 --- a/c/cert/src/rules/FIO44-C/OnlyUseValuesForFsetposThatAreReturnedFromFgetpos.ql +++ b/c/cert/src/rules/FIO44-C/OnlyUseValuesForFsetposThatAreReturnedFromFgetpos.ql @@ -22,24 +22,24 @@ class FsetposCall extends FunctionCall { FsetposCall() { this.getTarget().hasGlobalOrStdName("fsetpos") } } -class FposDFConf extends DataFlow::Configuration { - FposDFConf() { this = "FposDFConf" } - - override predicate isSource(DataFlow::Node source) { +module FposDFConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { // source must be the second parameter of a FgetposCall call source = DataFlow::definitionByReferenceNodeFromArgument(any(FgetposCall c).getArgument(1)) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { // sink must be the second parameter of a FsetposCall call sink.asExpr() = any(FsetposCall c).getArgument(1) } } +module FposDFFlow = DataFlow::Global; + from FsetposCall fsetpos where not isExcluded(fsetpos.getArgument(1), IO2Package::onlyUseValuesForFsetposThatAreReturnedFromFgetposQuery()) and - not any(FposDFConf dfConf).hasFlowToExpr(fsetpos.getArgument(1)) + not FposDFFlow::flowToExpr(fsetpos.getArgument(1)) select fsetpos.getArgument(1), "The position argument of a call to `fsetpos()` should be obtained from a call to `fgetpos()`." diff --git a/c/cert/src/rules/MEM36-C/DoNotModifyAlignmentOfMemoryWithRealloc.ql b/c/cert/src/rules/MEM36-C/DoNotModifyAlignmentOfMemoryWithRealloc.ql index 0d334a89f8..512b783030 100644 --- a/c/cert/src/rules/MEM36-C/DoNotModifyAlignmentOfMemoryWithRealloc.ql +++ b/c/cert/src/rules/MEM36-C/DoNotModifyAlignmentOfMemoryWithRealloc.ql @@ -16,7 +16,7 @@ import cpp import codingstandards.c.cert import codingstandards.cpp.Alignment import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import AlignedAllocToReallocFlow::PathGraph int getStatedValue(Expr e) { // `upperBound(e)` defaults to `exprMaxVal(e)` when `e` isn't analyzable. So to get a meaningful @@ -37,22 +37,22 @@ class ReallocCall extends FunctionCall { ReallocCall() { this.getTarget().hasName("realloc") } } -class AlignedAllocToReallocConfig extends DataFlow::Configuration { - AlignedAllocToReallocConfig() { this = "AlignedAllocToReallocConfig" } - - override predicate isSource(DataFlow::Node source) { +module AlignedAllocToReallocConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NonDefaultAlignedAllocCall } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(ReallocCall realloc | sink.asExpr() = realloc.getArgument(0)) } } -from AlignedAllocToReallocConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink +module AlignedAllocToReallocFlow = DataFlow::Global; + +from AlignedAllocToReallocFlow::PathNode source, AlignedAllocToReallocFlow::PathNode sink where not isExcluded(sink.getNode().asExpr(), Memory2Package::doNotModifyAlignmentOfMemoryWithReallocQuery()) and - cfg.hasFlowPath(source, sink) + AlignedAllocToReallocFlow::flowPath(source, sink) select sink, source, sink, "Memory allocated with $@ but reallocated with realloc.", source.getNode().asExpr(), "aligned_alloc" diff --git a/c/cert/src/rules/MSC33-C/DoNotPassInvalidDataToTheAsctimeFunction.ql b/c/cert/src/rules/MSC33-C/DoNotPassInvalidDataToTheAsctimeFunction.ql index f9e2c605ae..52dd0b1046 100644 --- a/c/cert/src/rules/MSC33-C/DoNotPassInvalidDataToTheAsctimeFunction.ql +++ b/c/cert/src/rules/MSC33-C/DoNotPassInvalidDataToTheAsctimeFunction.ql @@ -30,22 +30,22 @@ class AsctimeArg extends Expr { * Dataflow configuration for flow from a library function * to a call of function `asctime` */ -class TmStructSafeConfig extends DataFlow::Configuration { - TmStructSafeConfig() { this = "TmStructSafeConfig" } - - override predicate isSource(DataFlow::Node src) { +module TmStructSafeConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() .(FunctionCall) .getTarget() .hasGlobalName(["localtime", "localtime_r", "localtime_s", "gmtime", "gmtime_r", "gmtime_s"]) } - override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof AsctimeArg } + predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof AsctimeArg } } -from AsctimeArg fc, TmStructSafeConfig config +module TmStructSafeFlow = DataFlow::Global; + +from AsctimeArg fc where not isExcluded(fc, Contracts7Package::doNotPassInvalidDataToTheAsctimeFunctionQuery()) and - not config.hasFlowToExpr(fc) + not TmStructSafeFlow::flowToExpr(fc) select fc, "The function `asctime` and `asctime_r` should be discouraged. Unsanitized input can overflow the output buffer." diff --git a/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql b/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql index 457c1803ba..821b79c8e4 100644 --- a/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql +++ b/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql @@ -35,17 +35,17 @@ class VaEndArg extends VaAccess { * Dataflow configuration for flow from a library function * to a call of function `asctime` */ -class VaArgConfig extends DataFlow::Configuration { - VaArgConfig() { this = "VaArgConfig" } - - override predicate isSource(DataFlow::Node src) { +module VaArgConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asUninitialized() = any(VariableDeclarationEntry m | m.getType().hasName("va_list")).getVariable() } - override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof VaAccess } + predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof VaAccess } } +module VaArgFlow = DataFlow::Global; + /** * Controlflow nodes preceeding a call to `va_arg` */ @@ -65,9 +65,9 @@ ControlFlowNode preceedsFC(VaAccess va_arg) { } predicate sameSource(VaAccess e1, VaAccess e2) { - exists(VaArgConfig config, DataFlow::Node source | - config.hasFlow(source, DataFlow::exprNode(e1)) and - config.hasFlow(source, DataFlow::exprNode(e2)) + exists(DataFlow::Node source | + VaArgFlow::flow(source, DataFlow::exprNode(e1)) and + VaArgFlow::flow(source, DataFlow::exprNode(e2)) ) } diff --git a/c/cert/src/rules/STR30-C/DoNotAttemptToModifyStringLiterals.ql b/c/cert/src/rules/STR30-C/DoNotAttemptToModifyStringLiterals.ql index 7fbdc276c5..40f19ed4a0 100644 --- a/c/cert/src/rules/STR30-C/DoNotAttemptToModifyStringLiterals.ql +++ b/c/cert/src/rules/STR30-C/DoNotAttemptToModifyStringLiterals.ql @@ -39,12 +39,8 @@ class ModifiesFirstArgFunction extends BufferWrite, FunctionCall { * literal that is assigned to a non modifiable type or wherein the string * literal arises as a argument to a function that may modify its argument. */ -class ImplicitOrExplicitStringLiteralModifiedConfiguration extends DataFlow::Configuration { - ImplicitOrExplicitStringLiteralModifiedConfiguration() { - this = "ImplicitOrExplicitStringLiteralModifiedConfiguration" - } - - override predicate isSource(DataFlow::Node node) { +module ImplicitOrExplicitStringLiteralModifiedConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { // usage through variables exists(Variable v | v.getAnAssignedValue() = node.asExpr() and @@ -65,7 +61,7 @@ class ImplicitOrExplicitStringLiteralModifiedConfiguration extends DataFlow::Con ) } - override predicate isSink(DataFlow::Node node) { + predicate isSink(DataFlow::Node node) { // it's either a buffer write of some kind that we // know about exists(BufferWrite bw | bw.getDest() = node.asExpr()) @@ -77,6 +73,9 @@ class ImplicitOrExplicitStringLiteralModifiedConfiguration extends DataFlow::Con } } +module ImplicitOrExplicitStringLiteralModifiedFlow = + DataFlow::Global; + class MaybeReturnsStringLiteralFunctionCall extends FunctionCall { MaybeReturnsStringLiteralFunctionCall() { getTarget().getName() in [ @@ -144,11 +143,12 @@ class ImplicitStringLiteralBase extends Expr { } } -from Expr literal, Expr literalWrite, ImplicitOrExplicitStringLiteralModifiedConfiguration config +from Expr literal, Expr literalWrite where not isExcluded(literal, Strings1Package::doNotAttemptToModifyStringLiteralsQuery()) and not isExcluded(literalWrite, Strings1Package::doNotAttemptToModifyStringLiteralsQuery()) and - config.hasFlow(DataFlow::exprNode(literal), DataFlow::exprNode(literalWrite)) + ImplicitOrExplicitStringLiteralModifiedFlow::flow(DataFlow::exprNode(literal), + DataFlow::exprNode(literalWrite)) select literalWrite, "This operation may write to a string that may be a string literal that was $@.", literal, "created here" diff --git a/c/common/src/codingstandards/c/OutOfBounds.qll b/c/common/src/codingstandards/c/OutOfBounds.qll index d6d68d04d5..87c7c17870 100644 --- a/c/common/src/codingstandards/c/OutOfBounds.qll +++ b/c/common/src/codingstandards/c/OutOfBounds.qll @@ -906,13 +906,10 @@ module OOB { override predicate isNotNullTerminated() { none() } } - private class PointerToObjectSourceOrSizeToBufferAccessFunctionConfig extends DataFlow::Configuration + private module PointerToObjectSourceOrSizeToBufferAccessFunctionConfig implements + DataFlow::ConfigSig { - PointerToObjectSourceOrSizeToBufferAccessFunctionConfig() { - this = "PointerToObjectSourceOrSizeToBufferAccessFunctionConfig" - } - - override predicate isSource(DataFlow::Node source) { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof PointerToObjectSource or exists(PointerToObjectSource ptr | @@ -921,7 +918,7 @@ module OOB { ) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(BufferAccess ba, Expr arg | ( arg = ba.(BufferAccessLibraryFunctionCall).getAnArgument() or @@ -934,7 +931,7 @@ module OOB { ) } - 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 @@ -955,10 +952,14 @@ module OOB { } } + private module PointerToObjectSourceOrSizeToBufferAccessFunctionFlow = + DataFlow::Global; + private predicate hasFlowFromBufferOrSizeExprToUse(Expr source, Expr use) { - exists(PointerToObjectSourceOrSizeToBufferAccessFunctionConfig config, Expr useOrChild | + exists(Expr useOrChild | exists(getArithmeticOffsetValue(use, useOrChild)) and - config.hasFlow(DataFlow::exprNode(source), DataFlow::exprNode(useOrChild)) + PointerToObjectSourceOrSizeToBufferAccessFunctionFlow::flow(DataFlow::exprNode(source), + DataFlow::exprNode(useOrChild)) ) } diff --git a/c/common/test/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.expected b/c/common/test/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.expected index 0011556fd0..d0ba3bdb5c 100644 --- a/c/common/test/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.expected +++ b/c/common/test/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.expected @@ -4,15 +4,19 @@ problems | test.c:13:10:13:11 | p4 | test.c:5:14:5:15 | l2 | test.c:13:10:13:11 | p4 | Subtraction between left operand pointing to array $@ and other operand pointing to array $@. | test.c:3:7:3:8 | l2 | l2 | test.c:2:7:2:8 | l1 | l1 | | test.c:13:15:13:16 | l1 | test.c:13:15:13:16 | l1 | test.c:13:15:13:16 | l1 | Subtraction between right operand pointing to array $@ and other operand pointing to array $@. | test.c:2:7:2:8 | l1 | l1 | test.c:3:7:3:8 | l2 | l2 | edges -| test.c:4:14:4:15 | l1 | test.c:10:10:10:11 | p1 | -| test.c:4:14:4:15 | l1 | test.c:12:10:12:11 | p1 | -| test.c:5:14:5:15 | l2 | test.c:11:10:11:11 | p2 | -| test.c:5:14:5:15 | l2 | test.c:12:15:12:16 | p2 | -| test.c:5:14:5:15 | l2 | test.c:13:10:13:11 | p4 | -| test.c:5:14:5:15 | l2 | test.c:14:10:14:11 | p4 | +| test.c:4:14:4:15 | l1 | test.c:4:14:4:18 | access to array | +| test.c:4:14:4:18 | access to array | test.c:10:10:10:11 | p1 | +| test.c:4:14:4:18 | access to array | test.c:12:10:12:11 | p1 | +| test.c:5:14:5:15 | l2 | test.c:5:14:5:19 | access to array | +| test.c:5:14:5:19 | access to array | test.c:11:10:11:11 | p2 | +| test.c:5:14:5:19 | access to array | test.c:12:15:12:16 | p2 | +| test.c:5:14:5:19 | access to array | test.c:13:10:13:11 | p4 | +| test.c:5:14:5:19 | access to array | test.c:14:10:14:11 | p4 | nodes | test.c:4:14:4:15 | l1 | semmle.label | l1 | +| test.c:4:14:4:18 | access to array | semmle.label | access to array | | test.c:5:14:5:15 | l2 | semmle.label | l2 | +| test.c:5:14:5:19 | access to array | semmle.label | access to array | | test.c:10:10:10:11 | p1 | semmle.label | p1 | | test.c:10:15:10:16 | l1 | semmle.label | l1 | | test.c:11:10:11:11 | p2 | semmle.label | p2 | diff --git a/c/common/test/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.expected b/c/common/test/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.expected index 5431867345..8db569a98d 100644 --- a/c/common/test/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.expected +++ b/c/common/test/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.expected @@ -11,20 +11,26 @@ problems | test.c:25:7:25:14 | ... >= ... | test.c:25:13:25:14 | l3 | test.c:25:13:25:14 | l3 | Compare operation >= comparing right operand pointing to array $@ and other operand pointing to array $@. | test.c:4:7:4:8 | l3 | l3 | test.c:2:7:2:8 | l1 | l1 | edges | test.c:6:13:6:14 | l1 | test.c:13:12:13:13 | p0 | -| test.c:7:14:7:15 | l1 | test.c:11:7:11:8 | p1 | -| test.c:7:14:7:15 | l1 | test.c:13:7:13:8 | p1 | -| test.c:7:14:7:15 | l1 | test.c:15:13:15:14 | p1 | -| test.c:7:14:7:15 | l1 | test.c:17:7:17:8 | p1 | -| test.c:7:14:7:15 | l1 | test.c:23:13:23:14 | p1 | -| test.c:7:14:7:15 | l1 | test.c:25:7:25:8 | p1 | -| test.c:8:14:8:15 | l1 | test.c:11:12:11:13 | p2 | -| test.c:8:14:8:15 | l1 | test.c:21:7:21:8 | p2 | -| test.c:9:14:9:15 | l2 | test.c:21:12:21:13 | p3 | +| test.c:7:14:7:15 | l1 | test.c:7:14:7:18 | access to array | +| test.c:7:14:7:18 | access to array | test.c:11:7:11:8 | p1 | +| test.c:7:14:7:18 | access to array | test.c:13:7:13:8 | p1 | +| test.c:7:14:7:18 | access to array | test.c:15:13:15:14 | p1 | +| test.c:7:14:7:18 | access to array | test.c:17:7:17:8 | p1 | +| test.c:7:14:7:18 | access to array | test.c:23:13:23:14 | p1 | +| test.c:7:14:7:18 | access to array | test.c:25:7:25:8 | p1 | +| test.c:8:14:8:15 | l1 | test.c:8:14:8:18 | access to array | +| test.c:8:14:8:18 | access to array | test.c:11:12:11:13 | p2 | +| test.c:8:14:8:18 | access to array | test.c:21:7:21:8 | p2 | +| test.c:9:14:9:15 | l2 | test.c:9:14:9:18 | access to array | +| test.c:9:14:9:18 | access to array | test.c:21:12:21:13 | p3 | nodes | test.c:6:13:6:14 | l1 | semmle.label | l1 | | test.c:7:14:7:15 | l1 | semmle.label | l1 | +| test.c:7:14:7:18 | access to array | semmle.label | access to array | | test.c:8:14:8:15 | l1 | semmle.label | l1 | +| test.c:8:14:8:18 | access to array | semmle.label | access to array | | test.c:9:14:9:15 | l2 | semmle.label | l2 | +| test.c:9:14:9:18 | access to array | semmle.label | access to array | | test.c:11:7:11:8 | p1 | semmle.label | p1 | | test.c:11:12:11:13 | p2 | semmle.label | p2 | | test.c:13:7:13:8 | p1 | semmle.label | p1 | diff --git a/c/misra/src/rules/RULE-17-5/ArrayFunctionArgumentNumberOfElements.ql b/c/misra/src/rules/RULE-17-5/ArrayFunctionArgumentNumberOfElements.ql index 6a0ff9833a..208e8153d6 100644 --- a/c/misra/src/rules/RULE-17-5/ArrayFunctionArgumentNumberOfElements.ql +++ b/c/misra/src/rules/RULE-17-5/ArrayFunctionArgumentNumberOfElements.ql @@ -44,21 +44,22 @@ class ArrayParameter extends Parameter { */ int countElements(ArrayAggregateLiteral l) { result = count(l.getAnElementExpr(_)) } -class SmallArrayConfig extends DataFlow::Configuration { - SmallArrayConfig() { this = "SmallArrayConfig" } +module SmallArrayConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ArrayAggregateLiteral } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ArrayAggregateLiteral } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(ArrayParameter p).getAMatchingArgument() } } +module SmallArrayFlow = DataFlow::Global; + from Expr arg, ArrayParameter p where not isExcluded(arg, Contracts6Package::arrayFunctionArgumentNumberOfElementsQuery()) and - exists(SmallArrayConfig config | arg = p.getAMatchingArgument() | - // the argument is a value and not an arrey + arg = p.getAMatchingArgument() and + ( + // the argument is a value and not an array not arg.getType() instanceof DerivedType or // the argument is an array too small @@ -67,7 +68,7 @@ where // the argument is a pointer and its value does not come from a literal of the correct arg.getType() instanceof PointerType and not exists(ArrayAggregateLiteral l | - config.hasFlow(DataFlow::exprNode(l), DataFlow::exprNode(arg)) and + SmallArrayFlow::flow(DataFlow::exprNode(l), DataFlow::exprNode(arg)) and countElements(l) >= p.getArraySize() ) ) diff --git a/c/misra/src/rules/RULE-21-14/MemcmpUsedToCompareNullTerminatedStrings.ql b/c/misra/src/rules/RULE-21-14/MemcmpUsedToCompareNullTerminatedStrings.ql index 96d6dedcb3..44e21d14db 100644 --- a/c/misra/src/rules/RULE-21-14/MemcmpUsedToCompareNullTerminatedStrings.ql +++ b/c/misra/src/rules/RULE-21-14/MemcmpUsedToCompareNullTerminatedStrings.ql @@ -16,13 +16,11 @@ import cpp import codingstandards.c.misra import codingstandards.c.misra.EssentialTypes import codingstandards.cpp.dataflow.TaintTracking -import DataFlow::PathGraph +import NullTerminatedStringToMemcmpFlow::PathGraph // Data flow from a StringLiteral or from an array of characters, to a memcmp call -class NullTerminatedStringToMemcmpConfiguration extends TaintTracking::Configuration { - NullTerminatedStringToMemcmpConfiguration() { this = "NullTerminatedStringToMemcmpConfiguration" } - - override predicate isSource(DataFlow::Node source) { +module NullTerminatedStringToMemcmpConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof StringLiteral or exists(Variable v, ArrayAggregateLiteral aal | @@ -48,7 +46,7 @@ class NullTerminatedStringToMemcmpConfiguration extends TaintTracking::Configura ) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(FunctionCall memcmp | memcmp.getTarget().hasGlobalOrStdName("memcmp") and sink.asExpr() = memcmp.getArgument([0, 1]) @@ -56,20 +54,23 @@ class NullTerminatedStringToMemcmpConfiguration extends TaintTracking::Configura } } +module NullTerminatedStringToMemcmpFlow = TaintTracking::Global; + from - FunctionCall memcmp, DataFlow::PathNode source, DataFlow::PathNode sink, - DataFlow::PathNode source1, DataFlow::PathNode arg1, DataFlow::PathNode source2, - DataFlow::PathNode arg2 + FunctionCall memcmp, NullTerminatedStringToMemcmpFlow::PathNode source, + NullTerminatedStringToMemcmpFlow::PathNode sink, + NullTerminatedStringToMemcmpFlow::PathNode source1, + NullTerminatedStringToMemcmpFlow::PathNode arg1, + NullTerminatedStringToMemcmpFlow::PathNode source2, + NullTerminatedStringToMemcmpFlow::PathNode arg2 where not isExcluded(memcmp, EssentialTypesPackage::memcmpUsedToCompareNullTerminatedStringsQuery()) and memcmp.getTarget().hasGlobalOrStdName("memcmp") and arg1.getNode().asExpr() = memcmp.getArgument(0) and arg2.getNode().asExpr() = memcmp.getArgument(1) and // There is a path from a null-terminated string to each argument - exists(NullTerminatedStringToMemcmpConfiguration cfg | - cfg.hasFlowPath(source1, arg1) and - cfg.hasFlowPath(source2, arg2) - ) and + NullTerminatedStringToMemcmpFlow::flowPath(source1, arg1) and + NullTerminatedStringToMemcmpFlow::flowPath(source2, arg2) and // Produce multiple paths for each result, one for each source/arg pair ( source = source1 and sink = arg1 diff --git a/c/misra/src/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.ql b/c/misra/src/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.ql index 58d46176c2..6dc3b3ee71 100644 --- a/c/misra/src/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.ql +++ b/c/misra/src/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.ql @@ -15,22 +15,22 @@ import codingstandards.c.misra import codingstandards.cpp.standardlibrary.FileAccess import codingstandards.cpp.dataflow.DataFlow -class FileDFConf extends DataFlow::Configuration { - FileDFConf() { this = "FileDFConf" } - - override predicate isSource(DataFlow::Node source) { +module FileDFConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { // source is the return value of a call to fopen source.asExpr().(FOpenCall).isReadOnlyMode() } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { // sink must be the second parameter of a FsetposCall call sink.asExpr() = any(FileWriteFunctionCall write).getFileExpr() } } -from FileDFConf dfConf, DataFlow::Node source, FileWriteFunctionCall sink +module FileDFFlow = DataFlow::Global; + +from DataFlow::Node source, FileWriteFunctionCall sink where not isExcluded(sink, IO3Package::attemptToWriteToAReadOnlyStreamQuery()) and - dfConf.hasFlow(source, DataFlow::exprNode(sink.getFileExpr())) + FileDFFlow::flow(source, DataFlow::exprNode(sink.getFileExpr())) select sink, "Attempt to write to a $@ opened as read-only.", source, "stream" diff --git a/c/misra/src/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.ql b/c/misra/src/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.ql index 22499946a0..307357a93a 100644 --- a/c/misra/src/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.ql +++ b/c/misra/src/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.ql @@ -19,14 +19,12 @@ import codingstandards.cpp.ReadErrorsAndEOF * The getchar() return value propagates directly to a check against EOF macro * type conversions are not allowed */ -class DFConf extends DataFlow::Configuration { - DFConf() { this = "DFConf" } - - override predicate isSource(DataFlow::Node source) { +module DFConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof InBandErrorReadFunctionCall } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(EOFWEOFInvocation mi, EqualityOperation eq | // one operand is the sink sink.asExpr() = eq.getAnOperand() and @@ -35,11 +33,13 @@ class DFConf extends DataFlow::Configuration { ) } - override predicate isBarrier(DataFlow::Node barrier) { + predicate isBarrier(DataFlow::Node barrier) { barrier.asExpr() = any(IntegralConversion c).getExpr() } } +module DFFlow = DataFlow::Global; + // The equality operation `eq` checks a char fetched from `read` against a macro predicate isWeakMacroCheck(EqualityOperation eq, InBandErrorReadFunctionCall read) { exists(Expr c, EOFWEOFInvocation mi | @@ -51,10 +51,10 @@ predicate isWeakMacroCheck(EqualityOperation eq, InBandErrorReadFunctionCall rea ) } -from EqualityOperation eq, InBandErrorReadFunctionCall read, DFConf dfConf +from EqualityOperation eq, InBandErrorReadFunctionCall read where not isExcluded(eq, IO3Package::eofShallBeComparedWithUnmodifiedReturnValuesQuery()) and isWeakMacroCheck(eq, read) and - not dfConf.hasFlow(DataFlow::exprNode(read), DataFlow::exprNode(eq.getAnOperand())) + not DFFlow::flow(DataFlow::exprNode(read), DataFlow::exprNode(eq.getAnOperand())) select eq, "The check is not reliable as the type of the return value of $@ is converted.", read, read.toString() diff --git a/cpp/autosar/src/rules/A18-1-4/PointerToAnElementOfAnArrayPassedToASmartPointer.ql b/cpp/autosar/src/rules/A18-1-4/PointerToAnElementOfAnArrayPassedToASmartPointer.ql index 72496d703d..842dc14390 100644 --- a/cpp/autosar/src/rules/A18-1-4/PointerToAnElementOfAnArrayPassedToASmartPointer.ql +++ b/cpp/autosar/src/rules/A18-1-4/PointerToAnElementOfAnArrayPassedToASmartPointer.ql @@ -17,18 +17,14 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.SmartPointers import codingstandards.cpp.dataflow.TaintTracking -import DataFlow::PathGraph +import SingleObjectSmartPointerArrayConstructionFlow::PathGraph class AutosarSmartPointerArraySpecialisation extends AutosarSmartPointer { AutosarSmartPointerArraySpecialisation() { this.getOwnedObjectType() instanceof ArrayType } } -class SingleObjectSmartPointerArrayConstructionConfig extends TaintTracking::Configuration { - SingleObjectSmartPointerArrayConstructionConfig() { - this = "SingleObjectSmartPointerArrayConstructionConfig" - } - - override predicate isSource(DataFlow::Node source) { +module SingleObjectSmartPointerArrayConstructionConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NewArrayExpr or source.asExpr() = any(FunctionCall fc, MemberFunction mf | @@ -40,7 +36,7 @@ class SingleObjectSmartPointerArrayConstructionConfig extends TaintTracking::Con ) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(AutosarSmartPointer sp | not sp instanceof AutosarSmartPointerArraySpecialisation and ( @@ -51,7 +47,7 @@ class SingleObjectSmartPointerArrayConstructionConfig extends TaintTracking::Con ) } - override predicate isAdditionalTaintStep(DataFlow::Node source, DataFlow::Node sink) { + predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node sink) { exists(AutosarUniquePointer sp, FunctionCall fc | fc = sp.getAReleaseCall() and source.asExpr() = fc.getQualifier() and @@ -59,7 +55,7 @@ class SingleObjectSmartPointerArrayConstructionConfig extends TaintTracking::Con ) } - override predicate isSanitizerIn(DataFlow::Node node) { + predicate isBarrierIn(DataFlow::Node node) { // Exclude flow into header files outside the source archive which are summarized by the // additional taint steps above. exists(AutosarUniquePointer sp | @@ -70,12 +66,15 @@ class SingleObjectSmartPointerArrayConstructionConfig extends TaintTracking::Con } } +module SingleObjectSmartPointerArrayConstructionFlow = + TaintTracking::Global; + from - SingleObjectSmartPointerArrayConstructionConfig config, DataFlow::PathNode source, - DataFlow::PathNode sink + SingleObjectSmartPointerArrayConstructionFlow::PathNode source, + SingleObjectSmartPointerArrayConstructionFlow::PathNode sink where not isExcluded(sink.getNode().asExpr(), PointersPackage::pointerToAnElementOfAnArrayPassedToASmartPointerQuery()) and - config.hasFlowPath(source, sink) + SingleObjectSmartPointerArrayConstructionFlow::flowPath(source, sink) select sink.getNode(), source, sink, "A pointer to an element of an array of objects flows to a smart pointer of a single object type." diff --git a/cpp/autosar/src/rules/A5-0-4/PointerArithmeticUsedWithPointersToNonFinalClasses.ql b/cpp/autosar/src/rules/A5-0-4/PointerArithmeticUsedWithPointersToNonFinalClasses.ql index 6caf641446..34b6660778 100644 --- a/cpp/autosar/src/rules/A5-0-4/PointerArithmeticUsedWithPointersToNonFinalClasses.ql +++ b/cpp/autosar/src/rules/A5-0-4/PointerArithmeticUsedWithPointersToNonFinalClasses.ql @@ -18,7 +18,7 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.Type import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import NonFinalClassToPointerArithmeticExprFlow::PathGraph class ArrayAccessOrPointerArith extends Expr { ArrayAccessOrPointerArith() { @@ -42,12 +42,8 @@ class AddressOfPointerCreation extends ClassPointerCreation, AddressOfExpr { AddressOfPointerCreation() { this.getAnOperand().getUnderlyingType() instanceof Class } } -class NonFinalClassToPointerArithmeticExprConfig extends DataFlow::Configuration { - NonFinalClassToPointerArithmeticExprConfig() { - this = "NonFinalClassToPointerArithmeticExprConfig" - } - - override predicate isSource(DataFlow::Node source) { +module NonFinalClassToPointerArithmeticExprConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { exists(Class c | source.asExpr() instanceof ClassPointerCreation and source.asExpr().getUnderlyingType().(PointerType).getBaseType() = c @@ -56,17 +52,21 @@ class NonFinalClassToPointerArithmeticExprConfig extends DataFlow::Configuration ) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(ArrayAccessOrPointerArith e | e.getAnOperand() = sink.asExpr()) } } +module NonFinalClassToPointerArithmeticExprFlow = + DataFlow::Global; + from - ArrayAccessOrPointerArith e, Class clz, Variable v, DataFlow::PathNode source, - DataFlow::PathNode sink + ArrayAccessOrPointerArith e, Class clz, Variable v, + NonFinalClassToPointerArithmeticExprFlow::PathNode source, + NonFinalClassToPointerArithmeticExprFlow::PathNode sink where not isExcluded(e, PointersPackage::pointerArithmeticUsedWithPointersToNonFinalClassesQuery()) and - any(NonFinalClassToPointerArithmeticExprConfig c).hasFlowPath(source, sink) and + NonFinalClassToPointerArithmeticExprFlow::flowPath(source, sink) and v.getAnAssignedValue() = source.getNode().asExpr() and ( e.(PointerArithmeticOperation).getAnOperand() = sink.getNode().asExpr() diff --git a/cpp/autosar/src/rules/A5-1-7/LambdaPassedToDecltype.ql b/cpp/autosar/src/rules/A5-1-7/LambdaPassedToDecltype.ql index da33fd5a78..afbd809664 100644 --- a/cpp/autosar/src/rules/A5-1-7/LambdaPassedToDecltype.ql +++ b/cpp/autosar/src/rules/A5-1-7/LambdaPassedToDecltype.ql @@ -17,26 +17,27 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.dataflow.DataFlow -class LambdaExpressionToInitializer extends DataFlow::Configuration { - LambdaExpressionToInitializer() { this = "LambdaExpressionToInitializer" } +module LambdaExpressionToInitializerConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof LambdaExpression } - override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof LambdaExpression } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(Variable v | v.getInitializer().getExpr() = sink.asExpr()) } } +module LambdaExpressionToInitializerFlow = DataFlow::Global; + from Decltype dt, LambdaExpression lambda where not isExcluded(dt, LambdasPackage::lambdaPassedToDecltypeQuery()) and ( dt.getExpr() = lambda or - exists(LambdaExpressionToInitializer config, VariableAccess va, Variable v | + exists(VariableAccess va, Variable v | dt.getExpr() = va and v = va.getTarget() and - config.hasFlow(DataFlow::exprNode(lambda), DataFlow::exprNode(v.getInitializer().getExpr())) + LambdaExpressionToInitializerFlow::flow(DataFlow::exprNode(lambda), + DataFlow::exprNode(v.getInitializer().getExpr())) ) ) select dt, "Lambda $@ passed as operand to decltype.", lambda, "expression" diff --git a/cpp/autosar/src/rules/A5-1-7/LambdaPassedToTypeid.ql b/cpp/autosar/src/rules/A5-1-7/LambdaPassedToTypeid.ql index d43568af21..08dbecc755 100644 --- a/cpp/autosar/src/rules/A5-1-7/LambdaPassedToTypeid.ql +++ b/cpp/autosar/src/rules/A5-1-7/LambdaPassedToTypeid.ql @@ -16,21 +16,19 @@ import cpp import codingstandards.cpp.dataflow.DataFlow import codingstandards.cpp.autosar -import DataFlow::PathGraph +import LambdaExpressionToTypeidFlow::PathGraph -class LambdaExpressionToTypeid extends DataFlow::Configuration { - LambdaExpressionToTypeid() { this = "LambdaExpressionToTypeid" } +module LambdaExpressionToTypeidConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof LambdaExpression } - override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof LambdaExpression } - - override predicate isSink(DataFlow::Node sink) { - exists(TypeidOperator op | op.getExpr() = sink.asExpr()) - } + predicate isSink(DataFlow::Node sink) { exists(TypeidOperator op | op.getExpr() = sink.asExpr()) } } -from DataFlow::PathNode source, DataFlow::PathNode sink +module LambdaExpressionToTypeidFlow = DataFlow::Global; + +from LambdaExpressionToTypeidFlow::PathNode source, LambdaExpressionToTypeidFlow::PathNode sink where not isExcluded(source.getNode().asExpr(), LambdasPackage::lambdaPassedToTypeidQuery()) and - any(LambdaExpressionToTypeid config).hasFlowPath(source, sink) + LambdaExpressionToTypeidFlow::flowPath(source, sink) select sink.getNode(), source, sink, "Lambda $@ passed as operand to typeid operator.", source.getNode(), "expression" diff --git a/cpp/autosar/src/rules/M5-0-17/PointerSubtractionOnDifferentArrays.ql b/cpp/autosar/src/rules/M5-0-17/PointerSubtractionOnDifferentArrays.ql index dd8fbaa553..ec432cea42 100644 --- a/cpp/autosar/src/rules/M5-0-17/PointerSubtractionOnDifferentArrays.ql +++ b/cpp/autosar/src/rules/M5-0-17/PointerSubtractionOnDifferentArrays.ql @@ -16,40 +16,41 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import ArrayToPointerDiffOperandFlow::PathGraph -class ArrayToPointerDiffOperandConfig extends DataFlow::Configuration { - ArrayToPointerDiffOperandConfig() { this = "ArrayToPointerDiffOperandConfig" } - - override predicate isSource(DataFlow::Node source) { +module ArrayToPointerDiffOperandConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr().(VariableAccess).getType() instanceof ArrayType or // Consider array to pointer decay for parameters. source.asExpr().(VariableAccess).getTarget().(Parameter).getType() instanceof ArrayType } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(PointerDiffExpr e | e.getAnOperand() = sink.asExpr()) } - override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { // Add a flow step from the base to the array expression to track pointers to elements of the array. exists(ArrayExpr e | e.getArrayBase() = pred.asExpr() and e = succ.asExpr()) } } +module ArrayToPointerDiffOperandFlow = DataFlow::Global; + from PointerDiffExpr pointerSubstraction, Variable currentOperandPointee, Variable otherOperandPointee, - DataFlow::PathNode source, DataFlow::PathNode sink, string side + ArrayToPointerDiffOperandFlow::PathNode source, ArrayToPointerDiffOperandFlow::PathNode sink, + string side where not isExcluded(pointerSubstraction, PointersPackage::pointerSubtractionOnDifferentArraysQuery()) and - exists(ArrayToPointerDiffOperandConfig c, Variable sourceLeft, Variable sourceRight | - c.hasFlow(DataFlow::exprNode(sourceLeft.getAnAccess()), + exists(Variable sourceLeft, Variable sourceRight | + ArrayToPointerDiffOperandFlow::flow(DataFlow::exprNode(sourceLeft.getAnAccess()), DataFlow::exprNode(pointerSubstraction.getLeftOperand())) and - c.hasFlow(DataFlow::exprNode(sourceRight.getAnAccess()), + ArrayToPointerDiffOperandFlow::flow(DataFlow::exprNode(sourceRight.getAnAccess()), DataFlow::exprNode(pointerSubstraction.getRightOperand())) and not sourceLeft = sourceRight and - c.hasFlowPath(source, sink) and + ArrayToPointerDiffOperandFlow::flowPath(source, sink) and ( source.getNode().asExpr() = sourceLeft.getAnAccess() and sink.getNode().asExpr() = pointerSubstraction.getLeftOperand() and diff --git a/cpp/autosar/test/rules/A18-1-4/PointerToAnElementOfAnArrayPassedToASmartPointer.expected b/cpp/autosar/test/rules/A18-1-4/PointerToAnElementOfAnArrayPassedToASmartPointer.expected index a96c3fb64f..dcf263fc54 100644 --- a/cpp/autosar/test/rules/A18-1-4/PointerToAnElementOfAnArrayPassedToASmartPointer.expected +++ b/cpp/autosar/test/rules/A18-1-4/PointerToAnElementOfAnArrayPassedToASmartPointer.expected @@ -2,11 +2,13 @@ edges | test.cpp:3:36:3:45 | new[] | test.cpp:19:27:19:44 | call to allocate_int_array | | test.cpp:3:36:3:45 | new[] | test.cpp:23:12:23:29 | call to allocate_int_array | | test.cpp:3:36:3:45 | new[] | test.cpp:27:20:27:37 | call to allocate_int_array | -| test.cpp:11:29:11:41 | call to unique_ptr | test.cpp:12:30:12:36 | call to release | +| test.cpp:11:29:11:41 | call to unique_ptr | test.cpp:12:27:12:28 | v2 | +| test.cpp:12:27:12:28 | v2 | test.cpp:12:30:12:36 | call to release | | test.cpp:27:20:27:37 | call to allocate_int_array | test.cpp:32:12:32:20 | int_array | nodes | test.cpp:3:36:3:45 | new[] | semmle.label | new[] | | test.cpp:11:29:11:41 | call to unique_ptr | semmle.label | call to unique_ptr | +| test.cpp:12:27:12:28 | v2 | semmle.label | v2 | | test.cpp:12:30:12:36 | call to release | semmle.label | call to release | | test.cpp:19:27:19:44 | call to allocate_int_array | semmle.label | call to allocate_int_array | | test.cpp:23:12:23:29 | call to allocate_int_array | semmle.label | call to allocate_int_array | diff --git a/cpp/cert/src/rules/CTR56-CPP/DoNotUsePointerArithmeticOnPolymorphicObjects.ql b/cpp/cert/src/rules/CTR56-CPP/DoNotUsePointerArithmeticOnPolymorphicObjects.ql index 3b34500a80..a7756b6a6a 100644 --- a/cpp/cert/src/rules/CTR56-CPP/DoNotUsePointerArithmeticOnPolymorphicObjects.ql +++ b/cpp/cert/src/rules/CTR56-CPP/DoNotUsePointerArithmeticOnPolymorphicObjects.ql @@ -14,7 +14,7 @@ import cpp import codingstandards.cpp.cert import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import NonFinalClassToPointerArithmeticExprFlow::PathGraph class ArrayAccessOrPointerArith extends Expr { ArrayAccessOrPointerArith() { @@ -38,12 +38,8 @@ class AddressOfPointerCreation extends ClassPointerCreation, AddressOfExpr { AddressOfPointerCreation() { this.getAnOperand().getUnderlyingType() instanceof Class } } -class NonFinalClassToPointerArithmeticExprConfig extends DataFlow::Configuration { - NonFinalClassToPointerArithmeticExprConfig() { - this = "NonFinalClassToPointerArithmeticExprConfig" - } - - override predicate isSource(DataFlow::Node source) { +module NonFinalClassToPointerArithmeticExprConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { exists(Class c | source.asExpr() instanceof ClassPointerCreation and source.asExpr().getUnderlyingType().(PointerType).getBaseType() = c @@ -52,17 +48,21 @@ class NonFinalClassToPointerArithmeticExprConfig extends DataFlow::Configuration ) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(ArrayAccessOrPointerArith e | e.getAnOperand() = sink.asExpr()) } } +module NonFinalClassToPointerArithmeticExprFlow = + DataFlow::Global; + from - ArrayAccessOrPointerArith e, Class clz, Variable v, DataFlow::PathNode source, - DataFlow::PathNode sink + ArrayAccessOrPointerArith e, Class clz, Variable v, + NonFinalClassToPointerArithmeticExprFlow::PathNode source, + NonFinalClassToPointerArithmeticExprFlow::PathNode sink where not isExcluded(e, PointersPackage::doNotUsePointerArithmeticOnPolymorphicObjectsQuery()) and - any(NonFinalClassToPointerArithmeticExprConfig c).hasFlowPath(source, sink) and + NonFinalClassToPointerArithmeticExprFlow::flowPath(source, sink) and v.getAnAssignedValue() = source.getNode().asExpr() and ( e.(PointerArithmeticOperation).getAnOperand() = sink.getNode().asExpr() diff --git a/cpp/cert/src/rules/EXP51-CPP/DoNotDeleteAnArrayThroughAPointerOfTheIncorrectType.ql b/cpp/cert/src/rules/EXP51-CPP/DoNotDeleteAnArrayThroughAPointerOfTheIncorrectType.ql index 6cb62e9046..bdf6a7973e 100644 --- a/cpp/cert/src/rules/EXP51-CPP/DoNotDeleteAnArrayThroughAPointerOfTheIncorrectType.ql +++ b/cpp/cert/src/rules/EXP51-CPP/DoNotDeleteAnArrayThroughAPointerOfTheIncorrectType.ql @@ -14,25 +14,25 @@ import cpp import codingstandards.cpp.cert import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import AllocationToDeleteFlow::PathGraph -class AllocationToDeleteConfig extends DataFlow::Configuration { - AllocationToDeleteConfig() { this = "AllocationToDelete" } +module AllocationToDeleteConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NewArrayExpr } - override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NewArrayExpr } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(DeleteArrayExpr dae | dae.getExpr() = sink.asExpr()) } } +module AllocationToDeleteFlow = DataFlow::Global; + from - AllocationToDeleteConfig config, DataFlow::PathNode source, DataFlow::PathNode sink, + AllocationToDeleteFlow::PathNode source, AllocationToDeleteFlow::PathNode sink, NewArrayExpr newArray, DeleteArrayExpr deleteArray where not isExcluded(deleteArray.getExpr(), FreedPackage::doNotDeleteAnArrayThroughAPointerOfTheIncorrectTypeQuery()) and - config.hasFlowPath(source, sink) and + AllocationToDeleteFlow::flowPath(source, sink) and newArray = source.getNode().asExpr() and deleteArray.getExpr() = sink.getNode().asExpr() and not newArray.getType().getUnspecifiedType() = deleteArray.getExpr().getType().getUnspecifiedType() diff --git a/cpp/cert/src/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.ql b/cpp/cert/src/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.ql index 64b05ce9d9..c25e1aa0ad 100644 --- a/cpp/cert/src/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.ql +++ b/cpp/cert/src/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.ql @@ -74,22 +74,20 @@ class NotWrappedNoThrowAllocExpr extends NoThrowAllocExpr { /** * A data flow configuration for finding nothrow allocation calls which are checked in some kind of guard. */ -class NoThrowNewErrorCheckConfig extends DataFlow::Configuration { - NoThrowNewErrorCheckConfig() { this = "NoThrowNewErrorCheckConfig" } - - override predicate isSource(DataFlow::Node source) { +module NoThrowNewErrorCheckConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NotWrappedNoThrowAllocExpr } - override predicate isSink(DataFlow::Node sink) { - sink.asExpr() = any(GuardCondition gc).getAChild*() - } + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(GuardCondition gc).getAChild*() } } +module NoThrowNewErrorCheckFlow = DataFlow::Global; + from NotWrappedNoThrowAllocExpr ae where not isExcluded(ae, AllocationsPackage::detectAndHandleMemoryAllocationErrorsQuery()) and - not any(NoThrowNewErrorCheckConfig nt).hasFlow(DataFlow::exprNode(ae), _) + not NoThrowNewErrorCheckFlow::flow(DataFlow::exprNode(ae), _) select ae, "nothrow new allocation of $@ returns here without a subsequent check to see whether the pointer is valid.", ae.getUnderlyingAlloc() as underlying, underlying.getType().getName() diff --git a/cpp/cert/src/rules/MEM53-CPP/ManuallyManagedLifetime.qll b/cpp/cert/src/rules/MEM53-CPP/ManuallyManagedLifetime.qll index d51151ff95..358a3583fc 100644 --- a/cpp/cert/src/rules/MEM53-CPP/ManuallyManagedLifetime.qll +++ b/cpp/cert/src/rules/MEM53-CPP/ManuallyManagedLifetime.qll @@ -3,7 +3,6 @@ import codingstandards.cpp.Conversion import codingstandards.cpp.TrivialType import ManuallyManagedLifetime import semmle.code.cpp.controlflow.Dominance -import codingstandards.cpp.dataflow.DataFlow2 import codingstandards.cpp.dataflow.TaintTracking /** @@ -11,10 +10,8 @@ import codingstandards.cpp.dataflow.TaintTracking * * We use a taint-tracking configuration because we also want to track sub-sections */ -class AllocToStaticCastConfig extends TaintTracking::Configuration { - AllocToStaticCastConfig() { this = "AllocToStaticCastConfig" } - - override predicate isSource(DataFlow::Node source) { +module AllocToStaticCastConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { exists(AllocationExpr ae | ae.getType().getUnspecifiedType() instanceof VoidPointerType and source.asExpr() = ae and @@ -23,7 +20,7 @@ class AllocToStaticCastConfig extends TaintTracking::Configuration { ) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(StaticOrCStyleCast sc, Class nonTrivialClass | sc.getExpr() = sink.asExpr() and nonTrivialClass = sc.getType().getUnspecifiedType().(PointerType).getBaseType() and @@ -32,12 +29,14 @@ class AllocToStaticCastConfig extends TaintTracking::Configuration { } } +module AllocToStaticCastFlow = TaintTracking::Global; + /** * A cast of some existing memory, where we believe the resulting pointer has not been properly * constructed. */ class CastWithoutConstruction extends StaticOrCStyleCast { - CastWithoutConstruction() { any(AllocToStaticCastConfig c).hasFlowToExpr(getExpr()) } + CastWithoutConstruction() { AllocToStaticCastFlow::flowToExpr(getExpr()) } } /* @@ -96,18 +95,16 @@ class NonDestructingDeallocationCall extends Expr { * A data flow configuration from a `CastWithoutConstruction` to a free call on the memory without * an intervening destructor invocation. */ -class FreeWithoutDestructorConfig extends DataFlow2::Configuration { - FreeWithoutDestructorConfig() { this = "FreeWithoutDestructorConfig" } - - override predicate isSource(DataFlow::Node source) { +module FreeWithoutDestructorConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() = any(CastWithoutConstruction c).getExpr() } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(NonDestructingDeallocationCall de).getFreedExpr() } - override predicate isBarrier(DataFlow::Node barrier) { + predicate isBarrier(DataFlow::Node barrier) { // Consider any expression which later has a destructor called upon it to be safe. exists(DirectOrIndirectDestructorCall dc | DataFlow::localFlow(barrier, DataFlow::exprNode(dc.getDestructedArgument())) @@ -122,7 +119,9 @@ class FreeWithoutDestructorConfig extends DataFlow2::Configuration { ) } - override predicate isAdditionalFlowStep(DataFlow::Node stepFrom, DataFlow::Node stepTo) { + predicate isAdditionalFlowStep(DataFlow::Node stepFrom, DataFlow::Node stepTo) { stepTo.asExpr().(StaticOrCStyleCast).getExpr() = stepFrom.asExpr() } } + +module FreeWithoutDestructorFlow = DataFlow::Global; diff --git a/cpp/cert/src/rules/MEM53-CPP/MissingConstructorCallForManuallyManagedObject.ql b/cpp/cert/src/rules/MEM53-CPP/MissingConstructorCallForManuallyManagedObject.ql index bc48af3a63..30c5280482 100644 --- a/cpp/cert/src/rules/MEM53-CPP/MissingConstructorCallForManuallyManagedObject.ql +++ b/cpp/cert/src/rules/MEM53-CPP/MissingConstructorCallForManuallyManagedObject.ql @@ -15,16 +15,16 @@ import codingstandards.cpp.cert import codingstandards.cpp.TrivialType import ManuallyManagedLifetime import codingstandards.cpp.dataflow.TaintTracking -import DataFlow::PathGraph +import AllocToStaticCastFlow::PathGraph /* * Find flow from a manual allocation returning void* to a static_cast (or c-style cast) * to a specific type. */ -from AllocToStaticCastConfig config, DataFlow::PathNode source, DataFlow::PathNode sink +from AllocToStaticCastFlow::PathNode source, AllocToStaticCastFlow::PathNode sink where not isExcluded(sink.getNode().asExpr(), AllocationsPackage::missingConstructorCallForManuallyManagedObjectQuery()) and - config.hasFlowPath(source, sink) + AllocToStaticCastFlow::flowPath(source, sink) select sink.getNode(), source, sink, "Allocation to cast without constructor call" diff --git a/cpp/cert/src/rules/MEM53-CPP/MissingDestructorCallForManuallyManagedObject.ql b/cpp/cert/src/rules/MEM53-CPP/MissingDestructorCallForManuallyManagedObject.ql index 26d128a98e..b498729d69 100644 --- a/cpp/cert/src/rules/MEM53-CPP/MissingDestructorCallForManuallyManagedObject.ql +++ b/cpp/cert/src/rules/MEM53-CPP/MissingDestructorCallForManuallyManagedObject.ql @@ -13,12 +13,12 @@ import cpp import codingstandards.cpp.cert import ManuallyManagedLifetime -import codingstandards.cpp.dataflow.DataFlow2 -import DataFlow2::PathGraph +import codingstandards.cpp.dataflow.DataFlow +import FreeWithoutDestructorFlow::PathGraph -from FreeWithoutDestructorConfig dc, DataFlow2::PathNode source, DataFlow2::PathNode sink +from FreeWithoutDestructorFlow::PathNode source, FreeWithoutDestructorFlow::PathNode sink where not isExcluded(sink.getNode().asExpr(), AllocationsPackage::missingDestructorCallForManuallyManagedObjectQuery()) and - dc.hasFlowPath(source, sink) + FreeWithoutDestructorFlow::flowPath(source, sink) select sink.getNode(), source, sink, "Memory freed without an appropriate destructor called." diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll index ab4b11dffe..d856fa4515 100644 --- a/cpp/common/src/codingstandards/cpp/Concurrency.qll +++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll @@ -610,7 +610,7 @@ abstract class ThreadDependentMutex extends DataFlow::Node { class FlowBasedThreadDependentMutex extends ThreadDependentMutex { FlowBasedThreadDependentMutex() { // some sort of dataflow, likely through parameter passing. - exists(ThreadDependentMutexTaintTrackingConfiguration config | config.hasFlow(this, sink)) + ThreadDependentMutexFlow::flow(this, sink) } } @@ -738,18 +738,16 @@ class DeclarationInitAccessBasedThreadDependentMutex extends ThreadDependentMute override DataFlow::Node getAUsage() { result = DataFlow::exprNode(variableSource.getAnAccess()) } } -class ThreadDependentMutexTaintTrackingConfiguration extends TaintTracking::Configuration { - ThreadDependentMutexTaintTrackingConfiguration() { - this = "ThreadDependentMutexTaintTrackingConfiguration" - } - - override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof MutexSource } +module ThreadDependentMutexConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { node.asExpr() instanceof MutexSource } - override predicate isSink(DataFlow::Node node) { + predicate isSink(DataFlow::Node node) { exists(ThreadCreationFunction f | f.getAnArgument() = node.asExpr()) } } +module ThreadDependentMutexFlow = TaintTracking::Global; + /** * Models expressions that destroy mutexes. */ diff --git a/cpp/common/src/codingstandards/cpp/Nullness.qll b/cpp/common/src/codingstandards/cpp/Nullness.qll index b04c013a2d..d76db4afad 100644 --- a/cpp/common/src/codingstandards/cpp/Nullness.qll +++ b/cpp/common/src/codingstandards/cpp/Nullness.qll @@ -1,19 +1,14 @@ import cpp import codingstandards.cpp.dataflow.DataFlow -import codingstandards.cpp.dataflow.DataFlow2 private class PointerToMember extends Variable { PointerToMember() { this.getType() instanceof PointerToMemberType } } -class NullPointerToPointerMemberExpressionConfig extends DataFlow::Configuration { - NullPointerToPointerMemberExpressionConfig() { - this = "NullPointerToPointerMemberExpressionConfig" - } - - override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NullValue } +module NullPointerToPointerMemberExpressionConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NullValue } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { // The null value can flow to a pointer-to-member expressions that points to a function exists(VariableCall call, VariableAccess va | call.getQualifier() = va and va = sink.asExpr() | va.getTarget() instanceof PointerToMember @@ -24,12 +19,13 @@ class NullPointerToPointerMemberExpressionConfig extends DataFlow::Configuration } } -class NullValueToAssignmentConfig extends DataFlow2::Configuration { - NullValueToAssignmentConfig() { this = "NullValueToAssignmentConfig" } +module NullPointerToPointerMemberExpressionFlow = + DataFlow::Global; - override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NullValue } +module NullValueToAssignmentConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NullValue } - override predicate isSink(DataFlow::Node sink) { - exists(Assignment a | a.getRValue() = sink.asExpr()) - } + predicate isSink(DataFlow::Node sink) { exists(Assignment a | a.getRValue() = sink.asExpr()) } } + +module NullValueToAssignmentFlow = DataFlow::Global; diff --git a/cpp/common/src/codingstandards/cpp/allocations/PlacementNew.qll b/cpp/common/src/codingstandards/cpp/allocations/PlacementNew.qll index 39451a743b..5547f2e151 100644 --- a/cpp/common/src/codingstandards/cpp/allocations/PlacementNew.qll +++ b/cpp/common/src/codingstandards/cpp/allocations/PlacementNew.qll @@ -158,20 +158,20 @@ class AllocationExprPlacementNewOrigin extends PlacementNewMemoryOrigin { * A data flow configuration that identifies the origin of the placement argument to a placement * new expression. */ -class PlacementNewOriginConfig extends DataFlow::Configuration { - PlacementNewOriginConfig() { this = "PlacementNewOrigin" } +module PlacementNewOriginConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof PlacementNewMemoryOrigin } - override predicate isSource(DataFlow::Node source) { source instanceof PlacementNewMemoryOrigin } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(PlacementNewExpr pne).getPlacementExpr() // TODO direct calls to placement operator new? } - override predicate isAdditionalFlowStep(DataFlow::Node stepFrom, DataFlow::Node stepTo) { + predicate isAdditionalFlowStep(DataFlow::Node stepFrom, DataFlow::Node stepTo) { // Slightly surprisingly, we can't see the `StaticOrCStyleCast`s as a source out-of-the-box with data // flow - it's only reported under taint tracking. We therefore add a step through static // casts so that we can see them as sources. stepTo.asExpr().(StaticOrCStyleCast).getExpr() = stepFrom.asExpr() } } + +module PlacementNewOriginFlow = DataFlow::Global; diff --git a/cpp/common/src/codingstandards/cpp/rules/accessofundefinedmemberthroughnullpointer/AccessOfUndefinedMemberThroughNullPointer.qll b/cpp/common/src/codingstandards/cpp/rules/accessofundefinedmemberthroughnullpointer/AccessOfUndefinedMemberThroughNullPointer.qll index 2ee7036a1c..ab8659efd8 100644 --- a/cpp/common/src/codingstandards/cpp/rules/accessofundefinedmemberthroughnullpointer/AccessOfUndefinedMemberThroughNullPointer.qll +++ b/cpp/common/src/codingstandards/cpp/rules/accessofundefinedmemberthroughnullpointer/AccessOfUndefinedMemberThroughNullPointer.qll @@ -8,21 +8,23 @@ import codingstandards.cpp.Exclusions import codingstandards.cpp.Nullness import codingstandards.cpp.Expr import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import NullPointerToPointerMemberExpressionFlow::PathGraph abstract class AccessOfUndefinedMemberThroughNullPointerSharedQuery extends Query { } Query getQuery() { result instanceof AccessOfUndefinedMemberThroughNullPointerSharedQuery } query predicate problems( - PointerToMemberExpr pointerToMemberExpr, DataFlow::PathNode source, DataFlow::PathNode sink, - string message, Location sourceLocation, string sourceDescription + PointerToMemberExpr pointerToMemberExpr, + NullPointerToPointerMemberExpressionFlow::PathNode source, + NullPointerToPointerMemberExpressionFlow::PathNode sink, string message, Location sourceLocation, + string sourceDescription ) { not isExcluded(pointerToMemberExpr, getQuery()) and message = "A null pointer-to-member value from $@ is passed as the second operand to a pointer-to-member expression." and sink.getNode().asExpr() = pointerToMemberExpr.getPointerExpr() and - any(NullPointerToPointerMemberExpressionConfig config).hasFlowPath(source, sink) and + NullPointerToPointerMemberExpressionFlow::flowPath(source, sink) and sourceLocation = source.getNode().getLocation() and sourceDescription = "initialization" } diff --git a/cpp/common/src/codingstandards/cpp/rules/accessofundefinedmemberthroughuninitializedstaticpointer/AccessOfUndefinedMemberThroughUninitializedStaticPointer.qll b/cpp/common/src/codingstandards/cpp/rules/accessofundefinedmemberthroughuninitializedstaticpointer/AccessOfUndefinedMemberThroughUninitializedStaticPointer.qll index 7055cce296..ca1e2a4282 100644 --- a/cpp/common/src/codingstandards/cpp/rules/accessofundefinedmemberthroughuninitializedstaticpointer/AccessOfUndefinedMemberThroughUninitializedStaticPointer.qll +++ b/cpp/common/src/codingstandards/cpp/rules/accessofundefinedmemberthroughuninitializedstaticpointer/AccessOfUndefinedMemberThroughUninitializedStaticPointer.qll @@ -44,7 +44,7 @@ newtype TStaticMemberPointerAbstractValue = AssignedNullValue(StaticMemberPointer ptr, Expr val) { // A null value tracked via the data flow graph exists(ControlFlowNode n | - any(NullValueToAssignmentConfig config).hasFlow(_, DataFlow::exprNode(val)) and + NullValueToAssignmentFlow::flow(_, DataFlow::exprNode(val)) and n.(Assignment).getLValue() = ptr.getAnAccess() and n.(Assignment).getRValue() = val ) @@ -63,7 +63,7 @@ newtype TStaticMemberPointerAbstractValue = AssignedNonNullValue(StaticMemberPointer ptr, Expr val) { // A non-null value tracked via the data flow graph exists(ControlFlowNode n | - not any(NullValueToAssignmentConfig config).hasFlow(_, DataFlow::exprNode(val)) and + NullValueToAssignmentFlow::flow(_, DataFlow::exprNode(val)) and n.(Assignment).getLValue() = ptr.getAnAccess() and n.(Assignment).getRValue() = val ) diff --git a/cpp/common/src/codingstandards/cpp/rules/constlikereturnvalue/ConstLikeReturnValue.qll b/cpp/common/src/codingstandards/cpp/rules/constlikereturnvalue/ConstLikeReturnValue.qll index 56d1bd3d47..f4636b6b13 100644 --- a/cpp/common/src/codingstandards/cpp/rules/constlikereturnvalue/ConstLikeReturnValue.qll +++ b/cpp/common/src/codingstandards/cpp/rules/constlikereturnvalue/ConstLikeReturnValue.qll @@ -6,7 +6,7 @@ import cpp import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import DFFlow::PathGraph abstract class ConstLikeReturnValueSharedQuery extends Query { } @@ -41,22 +41,18 @@ class ObjectWrite extends Expr { /** * DF configuration for flows from a `NotModifiableCall` to a object modifications. */ -class DFConf extends DataFlow::Configuration { - DFConf() { this = "DFConf" } +module DFConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NotModifiableCall } - override predicate isSource(DataFlow::Node source) { - source.asExpr() instanceof NotModifiableCall - } - - override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ObjectWrite } + predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ObjectWrite } } -query predicate problems( - Element e, DataFlow::PathNode source, DataFlow::PathNode sink, string message -) { +module DFFlow = DataFlow::Global; + +query predicate problems(Element e, DFFlow::PathNode source, DFFlow::PathNode sink, string message) { not isExcluded(e, getQuery()) and // the modified object comes from a call to one of the ENV functions - any(DFConf d).hasFlowPath(source, sink) and + DFFlow::flowPath(source, sink) and e = sink.getNode().asExpr() and message = "The object returned by the function " + diff --git a/cpp/common/src/codingstandards/cpp/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.qll b/cpp/common/src/codingstandards/cpp/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.qll index 20e73e938b..0aa8d64feb 100644 --- a/cpp/common/src/codingstandards/cpp/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.qll +++ b/cpp/common/src/codingstandards/cpp/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.qll @@ -7,48 +7,47 @@ import cpp import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import ArrayToPointerDiffOperandFlow::PathGraph -class ArrayToPointerDiffOperandConfig extends DataFlow::Configuration { - ArrayToPointerDiffOperandConfig() { this = "ArrayToPointerDiffOperandConfig" } - - override predicate isSource(DataFlow::Node source) { +module ArrayToPointerDiffOperandConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr().(VariableAccess).getType() instanceof ArrayType or // Consider array to pointer decay for parameters. source.asExpr().(VariableAccess).getTarget().(Parameter).getType() instanceof ArrayType } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(PointerDiffExpr e | e.getAnOperand() = sink.asExpr()) } - override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { // Add a flow step from the base to the array expression to track pointers to elements of the array. exists(ArrayExpr e | e.getArrayBase() = pred.asExpr() and e = succ.asExpr()) } } +module ArrayToPointerDiffOperandFlow = DataFlow::Global; + abstract class DoNotSubtractPointersAddressingDifferentArraysSharedQuery extends Query { } Query getQuery() { result instanceof DoNotSubtractPointersAddressingDifferentArraysSharedQuery } query predicate problems( - DataFlow::Node sinkNode, DataFlow::PathNode source, DataFlow::PathNode sink, string message, - Variable currentOperandPointee, string currentOperandPointeeName, Variable otherOperandPointee, - string otherOperandPointeeName + DataFlow::Node sinkNode, ArrayToPointerDiffOperandFlow::PathNode source, + ArrayToPointerDiffOperandFlow::PathNode sink, string message, Variable currentOperandPointee, + string currentOperandPointeeName, Variable otherOperandPointee, string otherOperandPointeeName ) { exists( - PointerDiffExpr pointerSubtraction, string side, ArrayToPointerDiffOperandConfig c, - Variable sourceLeft, Variable sourceRight + PointerDiffExpr pointerSubtraction, string side, Variable sourceLeft, Variable sourceRight | not isExcluded(pointerSubtraction, getQuery()) and - c.hasFlow(DataFlow::exprNode(sourceLeft.getAnAccess()), + ArrayToPointerDiffOperandFlow::flow(DataFlow::exprNode(sourceLeft.getAnAccess()), DataFlow::exprNode(pointerSubtraction.getLeftOperand())) and - c.hasFlow(DataFlow::exprNode(sourceRight.getAnAccess()), + ArrayToPointerDiffOperandFlow::flow(DataFlow::exprNode(sourceRight.getAnAccess()), DataFlow::exprNode(pointerSubtraction.getRightOperand())) and not sourceLeft = sourceRight and - c.hasFlowPath(source, sink) and + ArrayToPointerDiffOperandFlow::flowPath(source, sink) and ( source.getNode().asExpr() = sourceLeft.getAnAccess() and sink.getNode().asExpr() = pointerSubtraction.getLeftOperand() and diff --git a/cpp/common/src/codingstandards/cpp/rules/donotusepointerarithmetictoaddressdifferentarrays/DoNotUsePointerArithmeticToAddressDifferentArrays.qll b/cpp/common/src/codingstandards/cpp/rules/donotusepointerarithmetictoaddressdifferentarrays/DoNotUsePointerArithmeticToAddressDifferentArrays.qll index 5fce6d99fc..dd10b840c5 100644 --- a/cpp/common/src/codingstandards/cpp/rules/donotusepointerarithmetictoaddressdifferentarrays/DoNotUsePointerArithmeticToAddressDifferentArrays.qll +++ b/cpp/common/src/codingstandards/cpp/rules/donotusepointerarithmetictoaddressdifferentarrays/DoNotUsePointerArithmeticToAddressDifferentArrays.qll @@ -18,24 +18,22 @@ Query getQuery() { result instanceof DoNotUsePointerArithmeticToAddressDifferent * A data-flow configuration that tracks access to an array to type to an array index expression. * This is used to determine possible pointer to array creations. */ -class ArrayToArrayExprConfig extends DataFlow::Configuration { - ArrayToArrayExprConfig() { this = "ArrayToArrayIndexConfig" } - - override predicate isSource(DataFlow::Node source) { +module ArrayToArrayExprConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr().(VariableAccess).getType() instanceof ArrayType } - override predicate isSink(DataFlow::Node sink) { - exists(ArrayExpr c | c.getArrayBase() = sink.asExpr()) - } + predicate isSink(DataFlow::Node sink) { exists(ArrayExpr c | c.getArrayBase() = sink.asExpr()) } } +module ArrayToArrayExprFlow = DataFlow::Global; + /** Holds if the address taken expression `addressOf` takes the address of an array element at `index` of `array` with size `arraySize`. */ predicate pointerOperandCreation(AddressOfExpr addressOf, Variable array, int arraySize, int index) { arraySize = array.getType().(ArrayType).getArraySize() and exists(ArrayExpr ae | - any(ArrayToArrayExprConfig cfg) - .hasFlow(DataFlow::exprNode(array.getAnAccess()), DataFlow::exprNode(ae.getArrayBase())) and + ArrayToArrayExprFlow::flow(DataFlow::exprNode(array.getAnAccess()), + DataFlow::exprNode(ae.getArrayBase())) and index = lowerBound(ae.getArrayOffset().getFullyConverted()) and addressOf.getOperand() = ae ) diff --git a/cpp/common/src/codingstandards/cpp/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.qll b/cpp/common/src/codingstandards/cpp/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.qll index c8ac2fd873..155ed1a7f4 100644 --- a/cpp/common/src/codingstandards/cpp/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.qll +++ b/cpp/common/src/codingstandards/cpp/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.qll @@ -8,7 +8,7 @@ import cpp import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import ArrayToRelationalOperationOperandFlow::PathGraph abstract class DoNotUseRelationalOperatorsWithDifferingArraysSharedQuery extends Query { } @@ -32,21 +32,22 @@ class DecayedArrayAccess extends ArraySource { } } -class ArrayToRelationalOperationOperandConfig extends DataFlow::Configuration { - ArrayToRelationalOperationOperandConfig() { this = "ArrayToRelationalOperationOperandConfig" } +module ArrayToRelationalOperationOperandConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof ArraySource } - override predicate isSource(DataFlow::Node source) { source instanceof ArraySource } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(RelationalOperation op | op.getAnOperand() = sink.asExpr()) } - override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { // Add a flow step from the base to the array expression to track pointers to elements of the array. exists(ArrayExpr e | e.getArrayBase() = pred.asExpr() and e = succ.asExpr()) } } +module ArrayToRelationalOperationOperandFlow = + DataFlow::Global; + predicate isComparingPointers(RelationalOperation op) { forall(Expr operand | operand = op.getAnOperand() | operand.getType() instanceof PointerType or operand.getType() instanceof ArrayType @@ -54,22 +55,20 @@ predicate isComparingPointers(RelationalOperation op) { } query predicate problems( - RelationalOperation compare, DataFlow::PathNode source, DataFlow::PathNode sink, string message, + RelationalOperation compare, ArrayToRelationalOperationOperandFlow::PathNode source, + ArrayToRelationalOperationOperandFlow::PathNode sink, string message, Variable selectedOperandPointee, string selectedOperandPointeeName, Variable otherOperandPointee, string otherOperandPointeeName ) { not isExcluded(compare, getQuery()) and - exists( - ArrayToRelationalOperationOperandConfig c, Variable sourceLeft, Variable sourceRight, - string side - | - c.hasFlow(DataFlow::exprNode(sourceLeft.getAnAccess()), + exists(Variable sourceLeft, Variable sourceRight, string side | + ArrayToRelationalOperationOperandFlow::flow(DataFlow::exprNode(sourceLeft.getAnAccess()), DataFlow::exprNode(compare.getLeftOperand())) and - c.hasFlow(DataFlow::exprNode(sourceRight.getAnAccess()), + ArrayToRelationalOperationOperandFlow::flow(DataFlow::exprNode(sourceRight.getAnAccess()), DataFlow::exprNode(compare.getRightOperand())) and not sourceLeft = sourceRight and isComparingPointers(compare) and - c.hasFlowPath(source, sink) and + ArrayToRelationalOperationOperandFlow::flowPath(source, sink) and ( source.getNode().asExpr() = sourceLeft.getAnAccess() and sink.getNode().asExpr() = compare.getLeftOperand() and diff --git a/cpp/common/src/codingstandards/cpp/rules/nonconstantformat/NonConstantFormat.qll b/cpp/common/src/codingstandards/cpp/rules/nonconstantformat/NonConstantFormat.qll index b73a648eeb..91b2b05a3f 100644 --- a/cpp/common/src/codingstandards/cpp/rules/nonconstantformat/NonConstantFormat.qll +++ b/cpp/common/src/codingstandards/cpp/rules/nonconstantformat/NonConstantFormat.qll @@ -106,28 +106,28 @@ predicate isSanitizerNode(DataFlow::Node node) { cannotContainString(node.getType()) } -class NonConstFlow extends TaintTracking::Configuration { - NonConstFlow() { this = "NonConstFlow" } - - override predicate isSource(DataFlow::Node source) { +module NonConstConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { isNonConst(source) and not cannotContainString(source.getType()) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(FormattingFunctionCall fc | sink.asExpr() = fc.getArgument(fc.getFormatParameterIndex())) } - override predicate isSanitizer(DataFlow::Node node) { isSanitizerNode(node) } + predicate isBarrier(DataFlow::Node node) { isSanitizerNode(node) } } +module NonConstFlow = TaintTracking::Global; + query predicate problems( Expr formatString, string message, FormattingFunctionCall call, string formatStringDescription ) { not isExcluded(formatString, getQuery()) and call.getArgument(call.getFormatParameterIndex()) = formatString and - exists(NonConstFlow cf, DataFlow::Node source, DataFlow::Node sink | - cf.hasFlow(source, sink) and + exists(DataFlow::Node source, DataFlow::Node sink | + NonConstFlow::flow(source, sink) and sink.asExpr() = formatString ) and message = diff --git a/cpp/common/src/codingstandards/cpp/rules/onlyfreememoryallocateddynamicallyshared/OnlyFreeMemoryAllocatedDynamicallyShared.qll b/cpp/common/src/codingstandards/cpp/rules/onlyfreememoryallocateddynamicallyshared/OnlyFreeMemoryAllocatedDynamicallyShared.qll index e8025db05b..bede451e24 100644 --- a/cpp/common/src/codingstandards/cpp/rules/onlyfreememoryallocateddynamicallyshared/OnlyFreeMemoryAllocatedDynamicallyShared.qll +++ b/cpp/common/src/codingstandards/cpp/rules/onlyfreememoryallocateddynamicallyshared/OnlyFreeMemoryAllocatedDynamicallyShared.qll @@ -8,8 +8,7 @@ import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.Allocations import codingstandards.cpp.dataflow.DataFlow -import codingstandards.cpp.dataflow.DataFlow2 -import DataFlow::PathGraph +import NonDynamicPointerToFreeFlow::PathGraph /** * A pointer to potentially dynamically allocated memory @@ -69,40 +68,32 @@ class AddressOfExprSourceNode extends Expr { ) ) and // exclude alloc(&allocated_ptr) cases - not any(DynamicMemoryAllocationToAddressOfDefiningArgConfig cfg) - .hasFlowTo(DataFlow::definitionByReferenceNodeFromArgument(this)) + not DynamicMemoryAllocationToAddressOfDefiningArgFlow::flowTo(DataFlow::definitionByReferenceNodeFromArgument(this)) } } /** * A data-flow configuration that tracks flow from an `AllocExprSource` to a `FreeExprSink`. */ -class DynamicMemoryAllocationToAddressOfDefiningArgConfig extends DataFlow2::Configuration { - DynamicMemoryAllocationToAddressOfDefiningArgConfig() { - this = "DynamicMemoryAllocationToAddressOfDefiningArgConfig" - } - - override predicate isSource(DataFlow::Node source) { source instanceof AllocExprSource } +module DynamicMemoryAllocationToAddressOfDefiningArgConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof AllocExprSource } - override predicate isSink(DataFlow::Node sink) { - sink.asDefiningArgument() instanceof AddressOfExpr - } + predicate isSink(DataFlow::Node sink) { sink.asDefiningArgument() instanceof AddressOfExpr } } +module DynamicMemoryAllocationToAddressOfDefiningArgFlow = + DataFlow::Global; + /** * A data-flow configuration that tracks flow from a * `NonDynamicallyAllocatedVariableAssignment` to a `FreeExprSink`. */ -class NonDynamicPointerToFreeConfig extends DataFlow::Configuration { - NonDynamicPointerToFreeConfig() { this = "NonDynamicPointerToFreeConfig" } +module NonDynamicPointerToFreeConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof AddressOfExprSourceNode } - override predicate isSource(DataFlow::Node source) { - source.asExpr() instanceof AddressOfExprSourceNode - } - - override predicate isSink(DataFlow::Node sink) { sink instanceof FreeExprSink } + predicate isSink(DataFlow::Node sink) { sink instanceof FreeExprSink } - 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 @@ -118,21 +109,24 @@ class NonDynamicPointerToFreeConfig extends DataFlow::Configuration { ) } - override predicate isBarrierIn(DataFlow::Node node) { + predicate isBarrierIn(DataFlow::Node node) { // only the last source expression is relevant isSource(node) } } +module NonDynamicPointerToFreeFlow = DataFlow::Global; + abstract class OnlyFreeMemoryAllocatedDynamicallySharedSharedQuery extends Query { } Query getQuery() { result instanceof OnlyFreeMemoryAllocatedDynamicallySharedSharedQuery } query predicate problems( - DataFlow::PathNode element, DataFlow::PathNode source, DataFlow::PathNode sink, string message + NonDynamicPointerToFreeFlow::PathNode element, NonDynamicPointerToFreeFlow::PathNode source, + NonDynamicPointerToFreeFlow::PathNode sink, string message ) { not isExcluded(element.getNode().asExpr(), getQuery()) and element = sink and - any(NonDynamicPointerToFreeConfig cfg).hasFlowPath(source, sink) and + NonDynamicPointerToFreeFlow::flowPath(source, sink) and message = "Free expression frees memory which was not dynamically allocated." } diff --git a/cpp/common/src/codingstandards/cpp/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.qll b/cpp/common/src/codingstandards/cpp/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.qll index eff7873d16..e24fb91539 100644 --- a/cpp/common/src/codingstandards/cpp/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.qll +++ b/cpp/common/src/codingstandards/cpp/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.qll @@ -9,20 +9,18 @@ import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.SmartPointers import codingstandards.cpp.dataflow.TaintTracking -import DataFlow::PathGraph +import PointerToSmartPointerConstructorFlowFlow::PathGraph abstract class OwnedPointerValueStoredInUnrelatedSmartPointerSharedQuery extends Query { } Query getQuery() { result instanceof OwnedPointerValueStoredInUnrelatedSmartPointerSharedQuery } -private class PointerToSmartPointerConstructorFlowConfig extends TaintTracking::Configuration { - PointerToSmartPointerConstructorFlowConfig() { this = "PointerToSmartPointerConstructorFlow" } - - override predicate isSource(DataFlow::Node source) { +private module PointerToSmartPointerConstructorFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { exists(Variable v | v.getAnAssignedValue() = source.asExpr()) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(AutosarSmartPointer sp, ConstructorCall cc | sp.getAConstructorCall() = cc and cc.getArgument(0).getFullyConverted().getType() instanceof PointerType and @@ -30,7 +28,7 @@ private class PointerToSmartPointerConstructorFlowConfig extends TaintTracking:: ) } - override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // Summarize flow through constructor calls exists(AutosarSmartPointer sp, ConstructorCall cc | sp.getAConstructorCall() = cc and @@ -46,7 +44,7 @@ private class PointerToSmartPointerConstructorFlowConfig extends TaintTracking:: ) } - override predicate isSanitizerIn(DataFlow::Node node) { + predicate isBarrierIn(DataFlow::Node node) { // Exclude flow into header files outside the source archive which are summarized by the // additional taint steps above. exists(AutosarSmartPointer sp | @@ -59,15 +57,19 @@ private class PointerToSmartPointerConstructorFlowConfig extends TaintTracking:: } } +private module PointerToSmartPointerConstructorFlowFlow = + TaintTracking::Global; + query predicate problems( - DataFlow::Node sinkNode, DataFlow::PathNode source, DataFlow::PathNode sink, string message + DataFlow::Node sinkNode, PointerToSmartPointerConstructorFlowFlow::PathNode source, + PointerToSmartPointerConstructorFlowFlow::PathNode sink, string message ) { not isExcluded(sinkNode.asExpr(), getQuery()) and - exists(PointerToSmartPointerConstructorFlowConfig config, DataFlow::PathNode sink2 | + exists(PointerToSmartPointerConstructorFlowFlow::PathNode sink2 | sink != sink2 and sinkNode = sink.getNode() and - config.hasFlowPath(source, sink) and - config.hasFlowPath(source, sink2) and + PointerToSmartPointerConstructorFlowFlow::flowPath(source, sink) and + PointerToSmartPointerConstructorFlowFlow::flowPath(source, sink2) and message = "Raw pointer flows to initialize multiple unrelated smart pointers." ) } diff --git a/cpp/common/src/codingstandards/cpp/rules/placementnewinsufficientstorage/PlacementNewInsufficientStorage.qll b/cpp/common/src/codingstandards/cpp/rules/placementnewinsufficientstorage/PlacementNewInsufficientStorage.qll index 515779595f..dc26d13b87 100644 --- a/cpp/common/src/codingstandards/cpp/rules/placementnewinsufficientstorage/PlacementNewInsufficientStorage.qll +++ b/cpp/common/src/codingstandards/cpp/rules/placementnewinsufficientstorage/PlacementNewInsufficientStorage.qll @@ -8,24 +8,23 @@ import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.allocations.PlacementNew import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import PlacementNewOriginFlow::PathGraph abstract class PlacementNewInsufficientStorageSharedQuery extends Query { } Query getQuery() { result instanceof PlacementNewInsufficientStorageSharedQuery } query predicate problems( - PlacementNewExpr placementNew, DataFlow::PathNode source, DataFlow::PathNode sink, string message, - PlacementNewMemoryOrigin memoryOrigin, string memoryOriginDescription + PlacementNewExpr placementNew, PlacementNewOriginFlow::PathNode source, + PlacementNewOriginFlow::PathNode sink, string message, PlacementNewMemoryOrigin memoryOrigin, + string memoryOriginDescription ) { not isExcluded(placementNew, getQuery()) and message = "Placement new expression is used with an insufficiently large memory allocation from $@." and - exists(PlacementNewOriginConfig config | - memoryOrigin = source.getNode() and - placementNew.getPlacementExpr() = sink.getNode().asExpr() and - memoryOriginDescription = memoryOrigin.toString() and - config.hasFlowPath(source, sink) and - memoryOrigin.getMaximumMemorySize() < placementNew.getMinimumAllocationSize() - ) + memoryOrigin = source.getNode() and + placementNew.getPlacementExpr() = sink.getNode().asExpr() and + memoryOriginDescription = memoryOrigin.toString() and + PlacementNewOriginFlow::flowPath(source, sink) and + memoryOrigin.getMaximumMemorySize() < placementNew.getMinimumAllocationSize() } diff --git a/cpp/common/src/codingstandards/cpp/rules/placementnewnotproperlyaligned/PlacementNewNotProperlyAligned.qll b/cpp/common/src/codingstandards/cpp/rules/placementnewnotproperlyaligned/PlacementNewNotProperlyAligned.qll index 19cbe2fff5..72286f2d79 100644 --- a/cpp/common/src/codingstandards/cpp/rules/placementnewnotproperlyaligned/PlacementNewNotProperlyAligned.qll +++ b/cpp/common/src/codingstandards/cpp/rules/placementnewnotproperlyaligned/PlacementNewNotProperlyAligned.qll @@ -8,7 +8,7 @@ import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.allocations.PlacementNew import codingstandards.cpp.dataflow.DataFlow -import DataFlow::PathGraph +import PlacementNewOriginFlow::PathGraph abstract class PlacementNewNotProperlyAlignedSharedQuery extends Query { } @@ -19,20 +19,19 @@ Query getQuery() { result instanceof PlacementNewNotProperlyAlignedSharedQuery } */ query predicate problems( - PlacementNewExpr placementNew, DataFlow::PathNode source, DataFlow::PathNode sink, string message, - PlacementNewMemoryOrigin memoryOrigin, string memoryOriginDescription + PlacementNewExpr placementNew, PlacementNewOriginFlow::PathNode source, + PlacementNewOriginFlow::PathNode sink, string message, PlacementNewMemoryOrigin memoryOrigin, + string memoryOriginDescription ) { not isExcluded(placementNew, getQuery()) and - exists(PlacementNewOriginConfig config | - memoryOrigin = source.getNode() and - placementNew.getPlacementExpr() = sink.getNode().asExpr() and - memoryOriginDescription = memoryOrigin.toString() and - config.hasFlowPath(source, sink) and - exists(int originAlignment | - originAlignment = memoryOrigin.getAlignment() and - // The origin alignment should be exactly divisible by the placement alignment - (originAlignment / placementNew.getAllocatedType().getAlignment()).ceil() = 0 and - message = "Placement new expression is used with inappropriately aligned memory from $@." - ) + memoryOrigin = source.getNode() and + placementNew.getPlacementExpr() = sink.getNode().asExpr() and + memoryOriginDescription = memoryOrigin.toString() and + PlacementNewOriginFlow::flowPath(source, sink) and + exists(int originAlignment | + originAlignment = memoryOrigin.getAlignment() and + // The origin alignment should be exactly divisible by the placement alignment + (originAlignment / placementNew.getAllocatedType().getAlignment()).ceil() = 0 and + message = "Placement new expression is used with inappropriately aligned memory from $@." ) } diff --git a/cpp/common/src/codingstandards/cpp/rules/stringnumberconversionmissingerrorcheck/StringNumberConversionMissingErrorCheck.qll b/cpp/common/src/codingstandards/cpp/rules/stringnumberconversionmissingerrorcheck/StringNumberConversionMissingErrorCheck.qll index e5856ad7c8..98fd51a58f 100644 --- a/cpp/common/src/codingstandards/cpp/rules/stringnumberconversionmissingerrorcheck/StringNumberConversionMissingErrorCheck.qll +++ b/cpp/common/src/codingstandards/cpp/rules/stringnumberconversionmissingerrorcheck/StringNumberConversionMissingErrorCheck.qll @@ -31,8 +31,7 @@ class CharIStreamConstructorCall extends CharIStreamSource, Expr { } override Expr getAUse() { - any(CharIStreamConstructorCallUseConfig c) - .hasFlow(DataFlow::exprNode(this), DataFlow::exprNode(result)) + CharIStreamConstructorCallUseFlow::flow(DataFlow::exprNode(this), DataFlow::exprNode(result)) } } @@ -40,18 +39,16 @@ class CharIStreamConstructorCall extends CharIStreamSource, Expr { * A global taint tracking configuration used to track from `CharIStream` constructor calls to uses * of that stream later in the program. */ -private class CharIStreamConstructorCallUseConfig extends TaintTracking::Configuration { - CharIStreamConstructorCallUseConfig() { this = "CharIStreamUse" } - - override predicate isSource(DataFlow::Node source) { +private module CharIStreamConstructorCallUseConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof CharIStreamConstructorCall } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr().getType().stripType() instanceof CharIStream } - override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // By default we do not get flow from ConstructorFieldInit expressions to accesses // of the field in other member functions, so we add it explicitly here. exists(ConstructorFieldInit cfi, Field f | @@ -63,6 +60,9 @@ private class CharIStreamConstructorCallUseConfig extends TaintTracking::Configu } } +private module CharIStreamConstructorCallUseFlow = + TaintTracking::Global; + /** * A `CharIStream` defined externally, and which therefore cannot be tracked as a source by taint tracking. * diff --git a/cpp/common/src/codingstandards/cpp/rules/throwingoperatornewreturnsnull/ThrowingOperatorNewReturnsNull.qll b/cpp/common/src/codingstandards/cpp/rules/throwingoperatornewreturnsnull/ThrowingOperatorNewReturnsNull.qll index a34beef5cd..9dbefeaa75 100644 --- a/cpp/common/src/codingstandards/cpp/rules/throwingoperatornewreturnsnull/ThrowingOperatorNewReturnsNull.qll +++ b/cpp/common/src/codingstandards/cpp/rules/throwingoperatornewreturnsnull/ThrowingOperatorNewReturnsNull.qll @@ -9,16 +9,14 @@ import codingstandards.cpp.allocations.CustomOperatorNewDelete import codingstandards.cpp.exceptions.ExceptionSpecifications import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions -import DataFlow::PathGraph +import NullFlow::PathGraph abstract class ThrowingOperatorNewReturnsNullSharedQuery extends Query { } Query getQuery() { result instanceof ThrowingOperatorNewReturnsNullSharedQuery } -class NullConfig extends DataFlow::Configuration { - NullConfig() { this = "NullConfig" } - - override predicate isSource(DataFlow::Node source) { +module NullConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NullValue or // Call to an allocation function that may return null @@ -32,7 +30,7 @@ class NullConfig extends DataFlow::Configuration { ) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(CustomOperatorNew co, ReturnStmt rs | co.getNumberOfParameters() = 1 and rs.getEnclosingFunction() = co and @@ -41,11 +39,13 @@ class NullConfig extends DataFlow::Configuration { } } +module NullFlow = DataFlow::Global; + query predicate problems( - ReturnStmt e, DataFlow::PathNode source, DataFlow::PathNode sink, string message + ReturnStmt e, NullFlow::PathNode source, NullFlow::PathNode sink, string message ) { not isExcluded(e, getQuery()) and - any(NullConfig nc).hasFlowPath(source, sink) and + NullFlow::flowPath(source, sink) and sink.getNode().asExpr() = e.getExpr() and exists(CustomOperatorNew op | message = diff --git a/cpp/common/src/codingstandards/cpp/rules/useonlyarrayindexingforpointerarithmetic/UseOnlyArrayIndexingForPointerArithmetic.qll b/cpp/common/src/codingstandards/cpp/rules/useonlyarrayindexingforpointerarithmetic/UseOnlyArrayIndexingForPointerArithmetic.qll index 979918a72b..c421ae3cc9 100644 --- a/cpp/common/src/codingstandards/cpp/rules/useonlyarrayindexingforpointerarithmetic/UseOnlyArrayIndexingForPointerArithmetic.qll +++ b/cpp/common/src/codingstandards/cpp/rules/useonlyarrayindexingforpointerarithmetic/UseOnlyArrayIndexingForPointerArithmetic.qll @@ -10,21 +10,19 @@ import codingstandards.cpp.dataflow.DataFlow abstract class UseOnlyArrayIndexingForPointerArithmeticSharedQuery extends Query { } -class ArrayToArrayBaseConfig extends DataFlow::Configuration { - ArrayToArrayBaseConfig() { this = "ArrayToArrayBaseConfig" } - - override predicate isSource(DataFlow::Node source) { +module ArrayToArrayBaseConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr().(VariableAccess).getType() instanceof ArrayType or // Consider array to pointer decay for parameters. source.asExpr().(VariableAccess).getTarget().(Parameter).getType() instanceof ArrayType } - override predicate isSink(DataFlow::Node sink) { - exists(ArrayExpr e | e.getArrayBase() = sink.asExpr()) - } + predicate isSink(DataFlow::Node sink) { exists(ArrayExpr e | e.getArrayBase() = sink.asExpr()) } } +module ArrayToArrayBaseFlow = DataFlow::Global; + predicate hasPointerResult(PointerArithmeticOperation op) { op instanceof PointerAddExpr or @@ -34,8 +32,7 @@ predicate hasPointerResult(PointerArithmeticOperation op) { predicate shouldBeArray(ArrayExpr arrayExpr) { arrayExpr.getArrayBase().getUnspecifiedType() instanceof PointerType and not exists(VariableAccess va | - any(ArrayToArrayBaseConfig config) - .hasFlow(DataFlow::exprNode(va), DataFlow::exprNode(arrayExpr.getArrayBase())) + ArrayToArrayBaseFlow::flow(DataFlow::exprNode(va), DataFlow::exprNode(arrayExpr.getArrayBase())) ) and not exists(Variable v | v.getAnAssignedValue().getType() instanceof ArrayType and diff --git a/cpp/common/src/codingstandards/cpp/standardlibrary/FileStreams.qll b/cpp/common/src/codingstandards/cpp/standardlibrary/FileStreams.qll index 775159326f..4d495fce3e 100644 --- a/cpp/common/src/codingstandards/cpp/standardlibrary/FileStreams.qll +++ b/cpp/common/src/codingstandards/cpp/standardlibrary/FileStreams.qll @@ -144,8 +144,7 @@ class FileStreamConstructorCall extends FileStreamSource, Expr { } override Expr getAUse() { - any(FileStreamConstructorCallUseConfig c) - .hasFlow(DataFlow::exprNode(this), DataFlow::exprNode(result)) + FileStreamConstructorCallUseFlow::flow(DataFlow::exprNode(this), DataFlow::exprNode(result)) } } @@ -164,18 +163,14 @@ class FileStreamExternGlobal extends FileStreamSource, GlobalOrNamespaceVariable /** * A global taint tracking configuration to track `FileStream` uses in the program. */ -private class FileStreamConstructorCallUseConfig extends TaintTracking::Configuration { - FileStreamConstructorCallUseConfig() { this = "FileStreamUse" } +private module FileStreamConstructorCallUseConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof FileStreamConstructorCall } - override predicate isSource(DataFlow::Node source) { - source.asExpr() instanceof FileStreamConstructorCall - } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr().getType().stripType() instanceof FileStream } - override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // By default we do not get flow from ConstructorFieldInit expressions to accesses // of the field in other member functions, so we add it explicitly here. exists(ConstructorFieldInit cfi, Field f | @@ -186,3 +181,6 @@ private class FileStreamConstructorCallUseConfig extends TaintTracking::Configur ) } } + +private module FileStreamConstructorCallUseFlow = + TaintTracking::Global; diff --git a/cpp/common/test/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.expected b/cpp/common/test/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.expected index 21fe1e3ccd..537228a000 100644 --- a/cpp/common/test/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.expected +++ b/cpp/common/test/rules/donotsubtractpointersaddressingdifferentarrays/DoNotSubtractPointersAddressingDifferentArrays.expected @@ -4,15 +4,19 @@ problems | test.cpp:13:10:13:11 | p4 | test.cpp:5:14:5:15 | l2 | test.cpp:13:10:13:11 | p4 | Subtraction between left operand pointing to array $@ and other operand pointing to array $@. | test.cpp:3:7:3:8 | l2 | l2 | test.cpp:2:7:2:8 | l1 | l1 | | test.cpp:13:15:13:16 | l1 | test.cpp:13:15:13:16 | l1 | test.cpp:13:15:13:16 | l1 | Subtraction between right operand pointing to array $@ and other operand pointing to array $@. | test.cpp:2:7:2:8 | l1 | l1 | test.cpp:3:7:3:8 | l2 | l2 | edges -| test.cpp:4:14:4:15 | l1 | test.cpp:10:10:10:11 | p1 | -| test.cpp:4:14:4:15 | l1 | test.cpp:12:10:12:11 | p1 | -| test.cpp:5:14:5:15 | l2 | test.cpp:11:10:11:11 | p2 | -| test.cpp:5:14:5:15 | l2 | test.cpp:12:15:12:16 | p2 | -| test.cpp:5:14:5:15 | l2 | test.cpp:13:10:13:11 | p4 | -| test.cpp:5:14:5:15 | l2 | test.cpp:14:10:14:11 | p4 | +| test.cpp:4:14:4:15 | l1 | test.cpp:4:14:4:18 | access to array | +| test.cpp:4:14:4:18 | access to array | test.cpp:10:10:10:11 | p1 | +| test.cpp:4:14:4:18 | access to array | test.cpp:12:10:12:11 | p1 | +| test.cpp:5:14:5:15 | l2 | test.cpp:5:14:5:19 | access to array | +| test.cpp:5:14:5:19 | access to array | test.cpp:11:10:11:11 | p2 | +| test.cpp:5:14:5:19 | access to array | test.cpp:12:15:12:16 | p2 | +| test.cpp:5:14:5:19 | access to array | test.cpp:13:10:13:11 | p4 | +| test.cpp:5:14:5:19 | access to array | test.cpp:14:10:14:11 | p4 | nodes | test.cpp:4:14:4:15 | l1 | semmle.label | l1 | +| test.cpp:4:14:4:18 | access to array | semmle.label | access to array | | test.cpp:5:14:5:15 | l2 | semmle.label | l2 | +| test.cpp:5:14:5:19 | access to array | semmle.label | access to array | | test.cpp:10:10:10:11 | p1 | semmle.label | p1 | | test.cpp:10:15:10:16 | l1 | semmle.label | l1 | | test.cpp:11:10:11:11 | p2 | semmle.label | p2 | diff --git a/cpp/common/test/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.expected b/cpp/common/test/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.expected index 1b31174b2f..22ddfd123a 100644 --- a/cpp/common/test/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.expected +++ b/cpp/common/test/rules/donotuserelationaloperatorswithdifferingarrays/DoNotUseRelationalOperatorsWithDifferingArrays.expected @@ -11,20 +11,26 @@ problems | test.cpp:25:7:25:14 | ... >= ... | test.cpp:25:13:25:14 | l3 | test.cpp:25:13:25:14 | l3 | Compare operation >= comparing right operand pointing to array $@ and other operand pointing to array $@. | test.cpp:4:7:4:8 | l3 | l3 | test.cpp:2:7:2:8 | l1 | l1 | edges | test.cpp:6:13:6:14 | l1 | test.cpp:13:12:13:13 | p0 | -| test.cpp:7:14:7:15 | l1 | test.cpp:11:7:11:8 | p1 | -| test.cpp:7:14:7:15 | l1 | test.cpp:13:7:13:8 | p1 | -| test.cpp:7:14:7:15 | l1 | test.cpp:15:13:15:14 | p1 | -| test.cpp:7:14:7:15 | l1 | test.cpp:17:7:17:8 | p1 | -| test.cpp:7:14:7:15 | l1 | test.cpp:23:13:23:14 | p1 | -| test.cpp:7:14:7:15 | l1 | test.cpp:25:7:25:8 | p1 | -| test.cpp:8:14:8:15 | l1 | test.cpp:11:12:11:13 | p2 | -| test.cpp:8:14:8:15 | l1 | test.cpp:21:7:21:8 | p2 | -| test.cpp:9:14:9:15 | l2 | test.cpp:21:12:21:13 | p3 | +| test.cpp:7:14:7:15 | l1 | test.cpp:7:14:7:18 | access to array | +| test.cpp:7:14:7:18 | access to array | test.cpp:11:7:11:8 | p1 | +| test.cpp:7:14:7:18 | access to array | test.cpp:13:7:13:8 | p1 | +| test.cpp:7:14:7:18 | access to array | test.cpp:15:13:15:14 | p1 | +| test.cpp:7:14:7:18 | access to array | test.cpp:17:7:17:8 | p1 | +| test.cpp:7:14:7:18 | access to array | test.cpp:23:13:23:14 | p1 | +| test.cpp:7:14:7:18 | access to array | test.cpp:25:7:25:8 | p1 | +| test.cpp:8:14:8:15 | l1 | test.cpp:8:14:8:18 | access to array | +| test.cpp:8:14:8:18 | access to array | test.cpp:11:12:11:13 | p2 | +| test.cpp:8:14:8:18 | access to array | test.cpp:21:7:21:8 | p2 | +| test.cpp:9:14:9:15 | l2 | test.cpp:9:14:9:18 | access to array | +| test.cpp:9:14:9:18 | access to array | test.cpp:21:12:21:13 | p3 | nodes | test.cpp:6:13:6:14 | l1 | semmle.label | l1 | | test.cpp:7:14:7:15 | l1 | semmle.label | l1 | +| test.cpp:7:14:7:18 | access to array | semmle.label | access to array | | test.cpp:8:14:8:15 | l1 | semmle.label | l1 | +| test.cpp:8:14:8:18 | access to array | semmle.label | access to array | | test.cpp:9:14:9:15 | l2 | semmle.label | l2 | +| test.cpp:9:14:9:18 | access to array | semmle.label | access to array | | test.cpp:11:7:11:8 | p1 | semmle.label | p1 | | test.cpp:11:12:11:13 | p2 | semmle.label | p2 | | test.cpp:13:7:13:8 | p1 | semmle.label | p1 | diff --git a/cpp/common/test/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.expected b/cpp/common/test/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.expected index d5d138ec19..3d00ff0d6a 100644 --- a/cpp/common/test/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.expected +++ b/cpp/common/test/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.expected @@ -7,10 +7,14 @@ problems | test.cpp:17:27:17:28 | v1 | test.cpp:16:13:16:22 | new | test.cpp:17:27:17:28 | v1 | Raw pointer flows to initialize multiple unrelated smart pointers. | edges | test.cpp:3:14:3:15 | v1 | test.cpp:5:27:5:28 | v1 | -| test.cpp:3:14:3:15 | v1 | test.cpp:6:31:6:33 | call to get | +| test.cpp:3:14:3:15 | v1 | test.cpp:5:27:5:28 | v1 | | test.cpp:3:14:3:15 | v1 | test.cpp:7:28:7:29 | v2 | | test.cpp:4:13:4:14 | v1 | test.cpp:7:28:7:29 | v2 | -| test.cpp:5:27:5:29 | call to shared_ptr | test.cpp:6:31:6:33 | call to get | +| test.cpp:5:27:5:28 | v1 | test.cpp:5:27:5:29 | call to shared_ptr | +| test.cpp:5:27:5:29 | call to shared_ptr | test.cpp:6:28:6:29 | p1 | +| test.cpp:5:27:5:29 | call to shared_ptr | test.cpp:6:28:6:29 | p1 | +| test.cpp:6:28:6:29 | p1 | test.cpp:6:31:6:33 | call to get | +| test.cpp:6:28:6:29 | p1 | test.cpp:6:31:6:33 | call to get | | test.cpp:8:8:8:14 | 0 | test.cpp:9:28:9:29 | v2 | | test.cpp:10:8:10:17 | new | test.cpp:11:28:11:29 | v2 | | test.cpp:10:8:10:17 | new | test.cpp:12:28:12:29 | v2 | @@ -21,7 +25,11 @@ nodes | test.cpp:3:14:3:15 | v1 | semmle.label | v1 | | test.cpp:4:13:4:14 | v1 | semmle.label | v1 | | test.cpp:5:27:5:28 | v1 | semmle.label | v1 | +| test.cpp:5:27:5:28 | v1 | semmle.label | v1 | +| test.cpp:5:27:5:29 | call to shared_ptr | semmle.label | call to shared_ptr | | test.cpp:5:27:5:29 | call to shared_ptr | semmle.label | call to shared_ptr | +| test.cpp:6:28:6:29 | p1 | semmle.label | p1 | +| test.cpp:6:28:6:29 | p1 | semmle.label | p1 | | test.cpp:6:31:6:33 | call to get | semmle.label | call to get | | test.cpp:7:28:7:29 | v2 | semmle.label | v2 | | test.cpp:8:8:8:14 | 0 | semmle.label | 0 |