Merge pull request #468 from jketema/config-update

Modernize dataflow configurations

Author: lcartey

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,15 +33,15 @@ class NonArrayPointerToArrayIndexingExprConfig extends DataFlow::Configuration {
     )
   }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(PointerArithmeticOrArrayExpr ae |
       sink.asExpr() = ae.getPointerOperand() and
       not sink.asExpr() instanceof Literal and
       not ae.isNonPointerOperandZero()
     )
   }
 
-  override predicate isBarrierOut(DataFlow::Node node) {
+  predicate isBarrierOut(DataFlow::Node node) {
     // the default interprocedural data-flow model flows through any field or array assignment
     // expressions to the qualifier (array base, pointer dereferenced, or qualifier) instead of the
     // individual element or field that the assignment modifies. this default behaviour causes
@@ -63,6 +61,9 @@ class NonArrayPointerToArrayIndexingExprConfig extends DataFlow::Configuration {
   }
 }
 
+module NonArrayPointerToArrayIndexingExprFlow =
+  DataFlow::Global<NonArrayPointerToArrayIndexingExprConfig>;
+
 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."

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<ScaledIntegerPointerArithmeticConfig>;
+
+from
+  ScaledIntegerPointerArithmeticFlow::PathNode src,
+  ScaledIntegerPointerArithmeticFlow::PathNode sink
 where
   not isExcluded(sink.getNode().asExpr(),
     Pointers2Package::doNotAddOrSubtractAScaledIntegerToAPointerQuery()) and
-  config.hasFlowPath(src, sink)
+  ScaledIntegerPointerArithmeticFlow::flowPath(src, sink)
 select sink, src, sink, "Scaled integer used in pointer arithmetic."

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<TssCreateToTssDeleteConfig>;
+
 from TSSCreateFunctionCall tcfc
 where
   not isExcluded(tcfc, Concurrency4Package::cleanUpThreadSpecificStorageQuery()) and
   // all calls to `tss_create` must be bookended by calls to tss_delete
   // even if a thread is not created.
-  not exists(TssCreateToTssDeleteDataFlowConfiguration config |
-    config.hasFlow(DataFlow::definitionByReferenceNodeFromArgument(tcfc.getKey()), _)
+  not (
+    TssCreateToTssDeleteFlow::flow(DataFlow::definitionByReferenceNodeFromArgument(tcfc.getKey()), _)
     or
-    config.hasFlow(DataFlow::exprNode(tcfc.getKey()), _)
+    TssCreateToTssDeleteFlow::flow(DataFlow::exprNode(tcfc.getKey()), _)
   )
   or
   // if a thread is created, we must check additional items

c/cert/src/rules/EXP36-C/DoNotCastPointerToMoreStrictlyAlignedPointerType.ql

@@ -15,9 +15,8 @@ import cpp
 import codingstandards.c.cert
 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 +95,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 +116,37 @@ class DefaultAlignedPointerExpr extends UnconvertedCastFromNonVoidPointerExpr, E
  * to exclude an `DefaultAlignedPointerAccessExpr` as a source if a preceding source
  * defined by this configuration provides more accurate alignment information.
  */
-class AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig extends DataFlow2::Configuration
+module AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig implements
+  DataFlow::ConfigSig
 {
-  AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig() {
-    this = "AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig"
-  }
-
-  override predicate isSource(DataFlow::Node source) {
+  predicate isSource(DataFlow::Node source) {
     source.asExpr() instanceof AddressOfAlignedVariableExpr or
     source.asExpr() instanceof DefinedAlignmentAllocationExpr
   }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     sink.asExpr() instanceof UnconvertedCastFromNonVoidPointerExpr
   }
 }
 
+module AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprFlow =
+  DataFlow::Global<AllocationOrAddressOfExprToUnconvertedCastFromNonVoidPointerExprConfig>;
+
 /**
  * A data-flow configuration for analysing the flow of `ExprWithAlignment` pointer expressions
  * to casts which perform pointer type conversions and potentially create pointer alignment issues.
  */
-class ExprWithAlignmentToCStyleCastConfiguration extends DataFlow::Configuration {
-  ExprWithAlignmentToCStyleCastConfiguration() {
-    this = "ExprWithAlignmentToCStyleCastConfiguration"
-  }
+module ExprWithAlignmentToCStyleCastConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source.asExpr() instanceof ExprWithAlignment }
 
-  override predicate isSource(DataFlow::Node source) {
-    source.asExpr() instanceof ExprWithAlignment
-  }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(CStyleCast cast |
       cast.getUnderlyingType() instanceof PointerType and
       cast.getUnconverted() = sink.asExpr()
     )
   }
 
-  override predicate isBarrierOut(DataFlow::Node node) {
+  predicate isBarrierOut(DataFlow::Node node) {
     // the default interprocedural data-flow model flows through any array assignment expressions
     // to the qualifier (array base or pointer dereferenced) instead of the individual element
     // that the assignment modifies. this default behaviour causes false positives for any future
@@ -169,12 +161,15 @@ class ExprWithAlignmentToCStyleCastConfiguration extends DataFlow::Configuration
   }
 }
 
+module ExprWithAlignmentToCStyleCastFlow = DataFlow::Global<ExprWithAlignmentToCStyleCastConfig>;
+
 from
-  DataFlow::PathNode source, DataFlow::PathNode sink, ExprWithAlignment expr, CStyleCast cast,
+  ExprWithAlignmentToCStyleCastFlow::PathNode source,
+  ExprWithAlignmentToCStyleCastFlow::PathNode sink, ExprWithAlignment expr, CStyleCast cast,
   Type toBaseType, int alignmentFrom, int alignmentTo
 where
   not isExcluded(cast, Pointers3Package::doNotCastPointerToMoreStrictlyAlignedPointerTypeQuery()) and
-  any(ExprWithAlignmentToCStyleCastConfiguration config).hasFlowPath(source, sink) and
+  ExprWithAlignmentToCStyleCastFlow::flowPath(source, sink) and
   source.getNode().asExpr() = expr and
   sink.getNode().asExpr() = cast.getUnconverted() and
   toBaseType = cast.getActualType().(PointerType).getBaseType() and

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<SuspectFunctionPointerToCallConfig>;
+
 from
-  SuspectFunctionPointerToCallConfig config, DataFlow::PathNode src, DataFlow::PathNode sink,
+  SuspectFunctionPointerToCallFlow::PathNode src, SuspectFunctionPointerToCallFlow::PathNode sink,
   Access access
 where
   not isExcluded(src.getNode().asExpr(),
     ExpressionsPackage::doNotCallFunctionPointerWithIncompatibleTypeQuery()) and
   access = src.getNode().asExpr() and
-  config.hasFlowPath(src, sink)
+  SuspectFunctionPointerToCallFlow::flowPath(src, sink)
 select src, src, sink,
   "Incompatible function $@ assigned to function pointer is eventually called through the pointer.",
   access.getTarget(), access.getTarget().getName()

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,17 +103,16 @@ class IndirectCastConfiguration extends DataFlow::Configuration {
     )
   }
 
-  override predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) {
+  predicate isBarrier(DataFlow::Node node, FlowState state) {
     state instanceof IndirectCastReallocatedFlowState and
     exists(FunctionCall fc |
       fc.getTarget() instanceof MemsetFunction and
       fc.getArgument(0) = node.asExpr()
     )
   }
 
-  override predicate isAdditionalFlowStep(
-    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
-    DataFlow::FlowState state2
+  predicate isAdditionalFlowStep(
+    DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
   ) {
     // track pointer flow through realloc calls and update state to `IndirectCastReallocatedFlowState`
     state1 instanceof IndirectCastDefaultFlowState and
@@ -135,6 +134,8 @@ class IndirectCastConfiguration extends DataFlow::Configuration {
   }
 }
 
+module IndirectCastFlow = DataFlow::GlobalWithState<IndirectCastConfig>;
+
 pragma[inline]
 predicate areTypesSameExceptForConstSpecifiers(Type a, Type b) {
   a.stripType() = b.stripType() and
@@ -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

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<CastConfig>;
+
+from CastFlow::PathNode src, CastFlow::PathNode sink
 where
-  conf.hasFlowPath(src, sink)
+  CastFlow::flowPath(src, sink)
   or
   sink.getNode()
       .asExpr()