C++: Define DefinitionByReferenceNode

This enables data flow through `memcpy` and similar functions modeled in
`semmle.code.cpp.model`.

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll

@@ -3,10 +3,14 @@
  */
 import cpp
 private import semmle.code.cpp.dataflow.internal.FlowVar
+private import semmle.code.cpp.models.interfaces.DataFlow
 
 private newtype TNode =
   TExprNode(Expr e) or
   TParameterNode(Parameter p) { exists(p.getFunction().getBlock()) } or
+  TDefinitionByReferenceNode(VariableAccess va, Expr argument) {
+    definitionByReference(va, argument)
+  } or
   TUninitializedNode(LocalVariable v) {
     not v.hasInitializer()
   }
@@ -26,6 +30,8 @@ class Node extends TNode {
     result = this.asParameter().getFunction()
     or
     result = this.asUninitialized().getFunction()
+    or
+    result = this.asDefiningArgument().getEnclosingFunction()
   }
 
   /**
@@ -40,6 +46,8 @@ class Node extends TNode {
     result = this.asExpr().getType()
     or
     result = asVariable(this).getType()
+    or
+    result = this.asDefiningVariableAccess().getType()
   }
 
   /** Gets the expression corresponding to this node, if any. */
@@ -48,6 +56,14 @@ class Node extends TNode {
   /** Gets the parameter corresponding to this node, if any. */
   Parameter asParameter() { result = this.(ParameterNode).getParameter() }
 
+  /** Gets the argument that defines this `DefinitionByReferenceNode`, if any. */
+  Expr asDefiningArgument() { result = this.(DefinitionByReferenceNode).getArgument() }
+
+  /** Gets the VariableAccess that defines this `DefinitionByReferenceNode`, if any. */
+  VariableAccess asDefiningVariableAccess() {
+    result = this.(DefinitionByReferenceNode).getVariableAccess()
+  }
+
   /**
    * Gets the uninitialized local variable corresponding to this node, if
    * any.
@@ -100,6 +116,29 @@ class ParameterNode extends Node, TParameterNode {
   }
 }
 
+/**
+ * A node that represents the value of a variable after a function call that
+ * may have changed the variable because it's passed by reference.
+ *
+ * A typical example would be a call `f(&x)`. Firstly, there will be flow into
+ * `x` from previous definitions of `x`. Secondly, there will be a
+ * `DefinitionByReferenceNode` to represent the value of `x` after the call has
+ * returned. This node will have its `getArgument()` equal to `&x` and its
+ * `getVariableAccess()` equal to `x`.
+ */
+class DefinitionByReferenceNode extends Node, TDefinitionByReferenceNode {
+  VariableAccess va;
+  Expr argument;
+
+  DefinitionByReferenceNode() { this = TDefinitionByReferenceNode(va, argument) }
+  override string toString() { result = "ref arg " + argument.toString() }
+  override Location getLocation() { result = argument.getLocation() }
+  /** Gets the argument corresponding to this node. */
+  Expr getArgument() { result = argument }
+  /** Gets the variable access corresponding to this node. */
+  VariableAccess getVariableAccess() { result = va }
+}
+
 /**
  * The value of an uninitialized local variable, viewed as a node in a data
  * flow graph.
@@ -143,6 +182,22 @@ ExprNode exprNode(Expr e) { result.getExpr() = e }
  */
 ParameterNode parameterNode(Parameter p) { result.getParameter() = p }
 
+/**
+ * Gets the `Node` corresponding to a definition by reference of the variable
+ * that is passed as `argument` of a call.
+ */
+DefinitionByReferenceNode definitionByReferenceNodeFromArgument(Expr argument) {
+  result.getArgument() = argument
+}
+
+/**
+ * Gets the `Node` corresponding to a definition by reference of the variable
+ * that is passed by reference as `va` in a call argument.
+ */
+DefinitionByReferenceNode definitionByReferenceNodeFromVariableAccess(VariableAccess va) {
+  result.getVariableAccess() = va
+}
+
 /**
  * Gets the `Node` corresponding to the value of an uninitialized local
  * variable `v`.
@@ -171,9 +226,14 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) {
       exprToVarStep(nodeFrom.asExpr(), var)
       or
       varSourceBaseCase(var, asVariable(nodeFrom))
+      or
+      var.definedByReference(nodeFrom.asDefiningArgument())
     ) and
     varToExprStep(var, nodeTo.asExpr())
   )
+  or
+  // Expr -> DefinitionByReferenceNode
+  exprToDefinitionByReferenceStep(nodeFrom.asExpr(), nodeTo.asDefiningArgument())
 }
 
 /**
@@ -239,6 +299,23 @@ private predicate exprToExprStep_nocfg(Expr fromExpr, Expr toExpr) {
   )
 }
 
+private predicate exprToDefinitionByReferenceStep(Expr exprIn, Expr argOut) {
+  exists(DataFlowFunction f, Call call, FunctionOutput outModel, int argOutIndex |
+    call.getTarget() = f and
+    argOut = call.getArgument(argOutIndex) and
+    outModel.isOutParameterPointer(argOutIndex) and
+    exists(int argInIndex, FunctionInput inModel |
+      f.hasDataFlow(inModel, outModel)
+    |
+      inModel.isInParameterPointer(argInIndex) and
+      call.passesByReference(argInIndex, exprIn)
+      or
+      inModel.isInParameter(argInIndex) and
+      exprIn = call.getArgument(argInIndex)
+    )
+  )
+}
+
 VariableAccess getAnAccessToAssignedVariable(Expr assign) {
   (assign instanceof Assignment
    or

cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected

@@ -5,6 +5,7 @@
 | example.c:24:13:24:30 | ... = ... | example.c:24:2:24:30 | ... = ... |
 | example.c:24:24:24:30 | ... + ... | example.c:24:13:24:30 | ... = ... |
 | example.c:26:13:26:16 | call to getX | example.c:26:2:26:25 | ... = ... |
+| example.c:26:18:26:24 | ref arg & ... | example.c:26:2:26:7 | coords |
 | test.cpp:6:12:6:17 | call to source | test.cpp:7:8:7:9 | t1 |
 | test.cpp:6:12:6:17 | call to source | test.cpp:8:8:8:9 | t1 |
 | test.cpp:6:12:6:17 | call to source | test.cpp:9:8:9:9 | t1 |
@@ -30,7 +31,13 @@
 | test.cpp:24:10:24:11 | t2 | test.cpp:26:8:26:9 | t1 |
 | test.cpp:430:48:430:54 | source1 | test.cpp:432:17:432:23 | source1 |
 | test.cpp:431:12:431:13 | 0 | test.cpp:432:11:432:13 | tmp |
+| test.cpp:432:10:432:13 | ref arg & ... | test.cpp:433:8:433:10 | tmp |
+| test.cpp:432:17:432:23 | source1 | test.cpp:432:10:432:13 | ref arg & ... |
 | test.cpp:436:53:436:59 | source1 | test.cpp:439:17:439:23 | source1 |
 | test.cpp:436:66:436:66 | b | test.cpp:441:7:441:7 | b |
 | test.cpp:437:12:437:13 | 0 | test.cpp:438:19:438:21 | tmp |
 | test.cpp:437:12:437:13 | 0 | test.cpp:439:11:439:13 | tmp |
+| test.cpp:439:10:439:13 | ref arg & ... | test.cpp:439:33:439:35 | tmp |
+| test.cpp:439:10:439:13 | ref arg & ... | test.cpp:440:8:440:10 | tmp |
+| test.cpp:439:10:439:13 | ref arg & ... | test.cpp:442:10:442:12 | tmp |
+| test.cpp:439:17:439:23 | source1 | test.cpp:439:10:439:13 | ref arg & ... |

cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp

@@ -430,16 +430,16 @@ void *memcpy(void *dest, const void *src, size_t count);
 void flowThroughMemcpy_ssa_with_local_flow(int source1) {
   int tmp = 0;
   memcpy(&tmp, &source1, sizeof tmp);
-  sink(tmp); // tainted (FALSE NEGATIVE)
+  sink(tmp); // tainted
 }
 
 void flowThroughMemcpy_blockvar_with_local_flow(int source1, int b) {
   int tmp = 0;
   int *capture = &tmp;
   memcpy(&tmp, &source1, sizeof tmp);
-  sink(tmp); // tainted (FALSE NEGATIVE)
+  sink(tmp); // tainted
   if (b) {
-    sink(tmp); // different sub-basic-block
+    sink(tmp); // tainted
   }
 }
 

cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected

@@ -27,6 +27,9 @@
 | test.cpp:366:7:366:7 | x | test.cpp:362:4:362:9 | call to source |
 | test.cpp:397:10:397:18 | globalVar | test.cpp:395:17:395:22 | call to source |
 | test.cpp:423:10:423:14 | field | test.cpp:421:13:421:18 | call to source |
+| test.cpp:433:8:433:10 | tmp | test.cpp:430:48:430:54 | source1 |
+| test.cpp:440:8:440:10 | tmp | test.cpp:436:53:436:59 | source1 |
+| test.cpp:442:10:442:12 | tmp | test.cpp:436:53:436:59 | source1 |
 | true_upon_entry.cpp:21:8:21:8 | x | true_upon_entry.cpp:17:11:17:16 | call to source |
 | true_upon_entry.cpp:29:8:29:8 | x | true_upon_entry.cpp:27:9:27:14 | call to source |
 | true_upon_entry.cpp:39:8:39:8 | x | true_upon_entry.cpp:33:11:33:16 | call to source |

cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected

@@ -9,6 +9,9 @@
 | test.cpp:136:27:136:32 | test.cpp:140:22:140:23 | AST only |
 | test.cpp:395:17:395:22 | test.cpp:397:10:397:18 | AST only |
 | test.cpp:421:13:421:18 | test.cpp:423:10:423:14 | AST only |
+| test.cpp:430:48:430:54 | test.cpp:433:8:433:10 | AST only |
+| test.cpp:436:53:436:59 | test.cpp:440:8:440:10 | AST only |
+| test.cpp:436:53:436:59 | test.cpp:442:10:442:12 | AST only |
 | true_upon_entry.cpp:9:11:9:16 | true_upon_entry.cpp:13:8:13:8 | IR only |
 | true_upon_entry.cpp:62:11:62:16 | true_upon_entry.cpp:66:8:66:8 | IR only |
 | true_upon_entry.cpp:98:11:98:16 | true_upon_entry.cpp:105:8:105:8 | IR only |

cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected

@@ -129,6 +129,9 @@
 | taint.cpp:164:19:164:24 | call to source | taint.cpp:172:18:172:24 | tainted |  |
 | taint.cpp:165:22:165:25 | {...} | taint.cpp:170:10:170:15 | buffer |  |
 | taint.cpp:165:24:165:24 | 0 | taint.cpp:165:22:165:25 | {...} | TAINT |
+| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:171:8:171:13 | buffer |  |
+| taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
+| taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:180:19:180:19 | p | taint.cpp:181:9:181:9 | p |  |
 | taint.cpp:181:9:181:9 | p | taint.cpp:181:8:181:9 | * ... | TAINT |
 | taint.cpp:185:11:185:16 | call to source | taint.cpp:186:11:186:11 | x |  |