Merge pull request github#12608 from jketema/configsig

C++: Use `DataFlow::ConfigSig` in more places

Author: jketema

cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql

@@ -15,8 +15,7 @@ import experimental.semmle.code.cpp.semantic.SemanticBound
 import experimental.semmle.code.cpp.semantic.SemanticExprSpecific
 import semmle.code.cpp.ir.IR
 import semmle.code.cpp.ir.dataflow.DataFlow
-import semmle.code.cpp.ir.dataflow.DataFlow2
-import DataFlow2::PathGraph
+import PointerArithmeticToDerefFlow::PathGraph
 
 pragma[nomagic]
 Instruction getABoundIn(SemBound b, IRFunction func) {
@@ -36,16 +35,16 @@ predicate bounded(Instruction i, Instruction b, int delta) {
   )
 }
 
-class FieldAddressToPointerArithmeticConf extends DataFlow::Configuration {
-  FieldAddressToPointerArithmeticConf() { this = "FieldAddressToPointerArithmeticConf" }
+module FieldAddressToPointerArithmeticConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { isFieldAddressSource(_, source) }
 
-  override predicate isSource(DataFlow::Node source) { isFieldAddressSource(_, source) }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(PointerAddInstruction pai | pai.getLeft() = sink.asInstruction())
   }
 }
 
+module FieldAddressToPointerArithmeticFlow = DataFlow::Make<FieldAddressToPointerArithmeticConfig>;
+
 predicate isFieldAddressSource(Field f, DataFlow::Node source) {
   source.asInstruction().(FieldAddressInstruction).getField() = f
 }
@@ -70,11 +69,8 @@ predicate isInvalidPointerDerefSink(DataFlow::Node sink, Instruction i, string o
 }
 
 predicate isConstantSizeOverflowSource(Field f, PointerAddInstruction pai, int delta) {
-  exists(
-    int size, int bound, FieldAddressToPointerArithmeticConf conf, DataFlow::Node source,
-    DataFlow::InstructionNode sink
-  |
-    conf.hasFlow(source, sink) and
+  exists(int size, int bound, DataFlow::Node source, DataFlow::InstructionNode sink |
+    FieldAddressToPointerArithmeticFlow::hasFlow(source, sink) and
     isFieldAddressSource(f, source) and
     pai.getLeft() = sink.asInstruction() and
     f.getUnspecifiedType().(ArrayType).getArraySize() = size and
@@ -86,21 +82,21 @@ predicate isConstantSizeOverflowSource(Field f, PointerAddInstruction pai, int d
   )
 }
 
-class PointerArithmeticToDerefConf extends DataFlow2::Configuration {
-  PointerArithmeticToDerefConf() { this = "PointerArithmeticToDerefConf" }
-
-  override predicate isSource(DataFlow::Node source) {
+module PointerArithmeticToDerefConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     isConstantSizeOverflowSource(_, source.asInstruction(), _)
   }
 
-  override predicate isSink(DataFlow::Node sink) { isInvalidPointerDerefSink(sink, _, _) }
+  predicate isSink(DataFlow::Node sink) { isInvalidPointerDerefSink(sink, _, _) }
 }
 
+module PointerArithmeticToDerefFlow = DataFlow::Make<PointerArithmeticToDerefConfig>;
+
 from
-  Field f, DataFlow2::PathNode source, DataFlow2::PathNode sink, Instruction deref,
-  PointerArithmeticToDerefConf conf, string operation, int delta
+  Field f, PointerArithmeticToDerefFlow::PathNode source,
+  PointerArithmeticToDerefFlow::PathNode sink, Instruction deref, string operation, int delta
 where
-  conf.hasFlowPath(source, sink) and
+  PointerArithmeticToDerefFlow::hasFlowPath(source, sink) and
   isInvalidPointerDerefSink(sink.getNode(), deref, operation) and
   isConstantSizeOverflowSource(f, source.getNode().asInstruction(), delta)
 select source, source, sink,

cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql

@@ -17,7 +17,6 @@
 
 import cpp
 import experimental.semmle.code.cpp.dataflow.ProductFlow
-import semmle.code.cpp.ir.dataflow.DataFlow3
 import experimental.semmle.code.cpp.semantic.analysis.RangeAnalysis
 import experimental.semmle.code.cpp.semantic.SemanticBound
 import experimental.semmle.code.cpp.semantic.SemanticExprSpecific
@@ -204,14 +203,14 @@ predicate isInvalidPointerDerefSink(DataFlow::Node sink, Instruction i, string o
  * A configuration to track flow from a pointer-arithmetic operation found
  * by `AllocToInvalidPointerConf` to a dereference of the pointer.
  */
-class InvalidPointerToDerefConf extends DataFlow3::Configuration {
-  InvalidPointerToDerefConf() { this = "InvalidPointerToDerefConf" }
+module InvalidPointerToDerefConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { invalidPointerToDerefSource(_, source, _) }
 
-  override predicate isSource(DataFlow::Node source) { invalidPointerToDerefSource(_, source, _) }
-
-  override predicate isSink(DataFlow::Node sink) { isInvalidPointerDerefSink(sink, _, _) }
+  predicate isSink(DataFlow::Node sink) { isInvalidPointerDerefSink(sink, _, _) }
 }
 
+module InvalidPointerToDerefFlow = DataFlow::Make<InvalidPointerToDerefConfig>;
+
 /**
  * Holds if `pai` is a pointer-arithmetic operation and `source` is a dataflow node with a
  * pointer-value that is non-strictly upper bounded by `pai + delta`.
@@ -236,13 +235,13 @@ newtype TMergedPathNode =
   // The path nodes computed by the first projection of `AllocToInvalidPointerConf`
   TPathNode1(DataFlow::PathNode p) or
   // The path nodes computed by `InvalidPointerToDerefConf`
-  TPathNode3(DataFlow3::PathNode p) or
+  TPathNode3(InvalidPointerToDerefFlow::PathNode p) or
   // The read/write that uses the invalid pointer identified by `InvalidPointerToDerefConf`.
   // This one is needed because the sink identified by `InvalidPointerToDerefConf` is the
   // pointer, but we want to raise an alert at the dereference.
   TPathNodeSink(Instruction i) {
     exists(DataFlow::Node n |
-      any(InvalidPointerToDerefConf conf).hasFlow(_, n) and
+      InvalidPointerToDerefFlow::hasFlow(_, n) and
       isInvalidPointerDerefSink(n, i, _)
     )
   }
@@ -252,7 +251,7 @@ class MergedPathNode extends TMergedPathNode {
 
   final DataFlow::PathNode asPathNode1() { this = TPathNode1(result) }
 
-  final DataFlow3::PathNode asPathNode3() { this = TPathNode3(result) }
+  final InvalidPointerToDerefFlow::PathNode asPathNode3() { this = TPathNode3(result) }
 
   final Instruction asSinkNode() { this = TPathNodeSink(result) }
 
@@ -280,7 +279,7 @@ class PathNode1 extends MergedPathNode, TPathNode1 {
 
 class PathNode3 extends MergedPathNode, TPathNode3 {
   override string toString() {
-    exists(DataFlow3::PathNode p |
+    exists(InvalidPointerToDerefFlow::PathNode p |
       this = TPathNode3(p) and
       result = p.toString()
     )
@@ -324,7 +323,9 @@ query predicate edges(MergedPathNode node1, MergedPathNode node2) {
  * Holds if `p1` is a sink of `AllocToInvalidPointerConf` and `p2` is a source
  * of `InvalidPointerToDerefConf`, and they are connected through `pai`.
  */
-predicate joinOn1(PointerArithmeticInstruction pai, DataFlow::PathNode p1, DataFlow3::PathNode p2) {
+predicate joinOn1(
+  PointerArithmeticInstruction pai, DataFlow::PathNode p1, InvalidPointerToDerefFlow::PathNode p2
+) {
   isSinkImpl(pai, p1.getNode(), _, _) and
   invalidPointerToDerefSource(pai, p2.getNode(), _)
 }
@@ -334,28 +335,29 @@ predicate joinOn1(PointerArithmeticInstruction pai, DataFlow::PathNode p1, DataF
  * that dereferences `p1`. The string `operation` describes whether the `i` is
  * a `StoreInstruction` or `LoadInstruction`.
  */
-predicate joinOn2(DataFlow3::PathNode p1, Instruction i, string operation) {
+predicate joinOn2(InvalidPointerToDerefFlow::PathNode p1, Instruction i, string operation) {
   isInvalidPointerDerefSink(p1.getNode(), i, operation)
 }
 
 predicate hasFlowPath(
-  MergedPathNode source1, MergedPathNode sink, DataFlow3::PathNode source3,
+  MergedPathNode source1, MergedPathNode sink, InvalidPointerToDerefFlow::PathNode source3,
   PointerArithmeticInstruction pai, string operation
 ) {
   exists(
-    AllocToInvalidPointerConf conf1, InvalidPointerToDerefConf conf2, DataFlow3::PathNode sink3,
+    AllocToInvalidPointerConf conf1, InvalidPointerToDerefFlow::PathNode sink3,
     DataFlow::PathNode sink1
   |
     conf1.hasFlowPath(source1.asPathNode1(), _, sink1, _) and
     joinOn1(pai, sink1, source3) and
-    conf2.hasFlowPath(source3, sink3) and
+    InvalidPointerToDerefFlow::hasFlowPath(source3, sink3) and
     joinOn2(sink3, sink.asSinkNode(), operation)
   )
 }
 
 from
-  MergedPathNode source, MergedPathNode sink, int k, string kstr, DataFlow3::PathNode source3,
-  PointerArithmeticInstruction pai, string operation, Expr offset, DataFlow::Node n
+  MergedPathNode source, MergedPathNode sink, int k, string kstr,
+  InvalidPointerToDerefFlow::PathNode source3, PointerArithmeticInstruction pai, string operation,
+  Expr offset, DataFlow::Node n
 where
   hasFlowPath(source, sink, source3, pai, operation) and
   invalidPointerToDerefSource(pai, source3.getNode(), k) and

cpp/ql/test/examples/docs-examples/analyzing-data-flow-in-cpp/exercise2.ql

@@ -1,26 +1,22 @@
 import cpp
 import semmle.code.cpp.dataflow.new.DataFlow
 
-class LiteralToGethostbynameConfiguration extends DataFlow::Configuration {
-  LiteralToGethostbynameConfiguration() { this = "LiteralToGethostbynameConfiguration" }
+module LiteralToGethostbynameConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source.asIndirectExpr(1) instanceof StringLiteral }
 
-  override predicate isSource(DataFlow::Node source) {
-    source.asIndirectExpr(1) instanceof StringLiteral
-  }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(FunctionCall fc |
       sink.asIndirectExpr(1) = fc.getArgument(0) and
       fc.getTarget().hasName("gethostbyname")
     )
   }
 }
 
-from
-  StringLiteral sl, FunctionCall fc, LiteralToGethostbynameConfiguration cfg, DataFlow::Node source,
-  DataFlow::Node sink
+module LiteralToGethostbynameFlow = DataFlow::Make<LiteralToGethostbynameConfig>;
+
+from StringLiteral sl, FunctionCall fc, DataFlow::Node source, DataFlow::Node sink
 where
   source.asIndirectExpr(1) = sl and
   sink.asIndirectExpr(1) = fc.getArgument(0) and
-  cfg.hasFlow(source, sink)
+  LiteralToGethostbynameFlow::hasFlow(source, sink)
 select sl, fc

cpp/ql/test/examples/docs-examples/analyzing-data-flow-in-cpp/exercise4.ql

@@ -5,24 +5,22 @@ class GetenvSource extends DataFlow::Node {
   GetenvSource() { this.asIndirectExpr(1).(FunctionCall).getTarget().hasGlobalName("getenv") }
 }
 
-class GetenvToGethostbynameConfiguration extends DataFlow::Configuration {
-  GetenvToGethostbynameConfiguration() { this = "GetenvToGethostbynameConfiguration" }
+module GetenvToGethostbynameConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof GetenvSource }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof GetenvSource }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(FunctionCall fc |
       sink.asIndirectExpr(1) = fc.getArgument(0) and
       fc.getTarget().hasName("gethostbyname")
     )
   }
 }
 
-from
-  Expr getenv, FunctionCall fc, GetenvToGethostbynameConfiguration cfg, DataFlow::Node source,
-  DataFlow::Node sink
+module GetenvToGethostbynameFlow = DataFlow::Make<GetenvToGethostbynameConfig>;
+
+from Expr getenv, FunctionCall fc, DataFlow::Node source, DataFlow::Node sink
 where
   source.asIndirectExpr(1) = getenv and
   sink.asIndirectExpr(1) = fc.getArgument(0) and
-  cfg.hasFlow(source, sink)
+  GetenvToGethostbynameFlow::hasFlow(source, sink)
 select getenv, fc

cpp/ql/test/examples/docs-examples/analyzing-data-flow-in-cpp/fopen-flow-from-getenv.ql

@@ -1,29 +1,27 @@
 import cpp
 import semmle.code.cpp.dataflow.new.DataFlow
 
-class EnvironmentToFileConfiguration extends DataFlow::Configuration {
-  EnvironmentToFileConfiguration() { this = "EnvironmentToFileConfiguration" }
-
-  override predicate isSource(DataFlow::Node source) {
+module EnvironmentToFileConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     exists(Function getenv |
       source.asIndirectExpr(1).(FunctionCall).getTarget() = getenv and
       getenv.hasGlobalName("getenv")
     )
   }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(FunctionCall fc |
       sink.asIndirectExpr(1) = fc.getArgument(0) and
       fc.getTarget().hasGlobalName("fopen")
     )
   }
 }
 
-from
-  Expr getenv, Expr fopen, EnvironmentToFileConfiguration config, DataFlow::Node source,
-  DataFlow::Node sink
+module EnvironmentToFileFlow = DataFlow::Make<EnvironmentToFileConfig>;
+
+from Expr getenv, Expr fopen, DataFlow::Node source, DataFlow::Node sink
 where
   source.asIndirectExpr(1) = getenv and
   sink.asIndirectExpr(1) = fopen and
-  config.hasFlow(source, sink)
+  EnvironmentToFileFlow::hasFlow(source, sink)
 select fopen, "This 'fopen' uses data from $@.", getenv, "call to 'getenv'"

cpp/ql/test/examples/docs-examples/analyzing-data-flow-in-cpp/index-flow-from-ntohl.ql

@@ -2,18 +2,16 @@ import cpp
 import semmle.code.cpp.controlflow.Guards
 import semmle.code.cpp.dataflow.new.TaintTracking
 
-class NetworkToBufferSizeConfiguration extends TaintTracking::Configuration {
-  NetworkToBufferSizeConfiguration() { this = "NetworkToBufferSizeConfiguration" }
-
-  override predicate isSource(DataFlow::Node node) {
+module NetworkToBufferSizeConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) {
     node.asExpr().(FunctionCall).getTarget().hasGlobalName("ntohl")
   }
 
-  override predicate isSink(DataFlow::Node node) {
+  predicate isSink(DataFlow::Node node) {
     exists(ArrayExpr ae | node.asExpr() = ae.getArrayOffset())
   }
 
-  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+  predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
     exists(Loop loop, LoopCounter lc |
       loop = lc.getALoop() and
       loop.getControllingExpr().(RelationalOperation).getGreaterOperand() = pred.asExpr()
@@ -22,7 +20,7 @@ class NetworkToBufferSizeConfiguration extends TaintTracking::Configuration {
     )
   }
 
-  override predicate isSanitizer(DataFlow::Node node) {
+  predicate isBarrier(DataFlow::Node node) {
     exists(GuardCondition gc, Variable v |
       gc.getAChild*() = v.getAnAccess() and
       node.asExpr() = v.getAnAccess() and
@@ -32,7 +30,9 @@ class NetworkToBufferSizeConfiguration extends TaintTracking::Configuration {
   }
 }
 
-from DataFlow::Node ntohl, DataFlow::Node offset, NetworkToBufferSizeConfiguration conf
-where conf.hasFlow(ntohl, offset)
+module NetworkToBufferSizeFlow = TaintTracking::Make<NetworkToBufferSizeConfig>;
+
+from DataFlow::Node ntohl, DataFlow::Node offset
+where NetworkToBufferSizeFlow::hasFlow(ntohl, offset)
 select offset, "This array offset may be influenced by $@.", ntohl,
   "converted data from the network"