From 20f3df0d09418b576c15c3dad7907d5bb0be11c4 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Tue, 26 Feb 2019 21:15:51 +0100
Subject: [PATCH 1/8] C++: Add tests to demo lack dataflow by reference

---
 .../dataflow-tests/localFlow.expected         |  9 ++++++
 .../dataflow/dataflow-tests/test.cpp          | 32 +++++++++++++++++++
 .../dataflow/dataflow-tests/test.expected     |  2 ++
 .../dataflow-tests/test_diff.expected         |  2 ++
 .../dataflow-tests/uninitialized.expected     |  5 +++
 5 files changed, 50 insertions(+)

diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
index 5bd5c9e8925a..22b74ac561bc 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
@@ -28,3 +28,12 @@
 | test.cpp:24:10:24:11 | t2 | test.cpp:23:23:23:24 | t1 |
 | test.cpp:24:10:24:11 | t2 | test.cpp:24:5:24:11 | ... = ... |
 | 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: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:437:12:437:13 | 0 | test.cpp:439:33:439:35 | tmp |
+| test.cpp:437:12:437:13 | 0 | test.cpp:440:8:440:10 | tmp |
+| test.cpp:437:12:437:13 | 0 | test.cpp:442:10:442:12 | tmp |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
index 88271adf40e9..486a5950b3c5 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
@@ -423,3 +423,35 @@ class FlowThroughFields {
     sink(field); // tainted
   }
 };
+
+typedef unsigned long size_t;
+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)
+}
+
+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)
+  if (b) {
+    sink(tmp); // different sub-basic-block
+  }
+}
+
+void cleanedByMemcpy_ssa(int clean1) {
+  int tmp;
+  memcpy(&tmp, &clean1, sizeof tmp);
+  sink(tmp); // clean
+}
+
+void cleanedByMemcpy_blockvar(int clean1) {
+  int tmp;
+  int *capture = &tmp;
+  memcpy(&tmp, &clean1, sizeof tmp);
+  sink(tmp); // clean (FALSE POSITIVE)
+}
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
index b2519f4805b9..7358a4abb943 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
@@ -27,6 +27,8 @@
 | 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:449:8:449:10 | tmp | test.cpp:447:7:447:9 | tmp |
+| test.cpp:456:8:456:10 | tmp | test.cpp:453:7:453:9 | tmp |
 | 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 |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
index b7f2ae6d6ef3..dd902540f858 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
@@ -9,6 +9,8 @@
 | 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:447:7:447:9 | test.cpp:449:8:449:10 | AST only |
+| test.cpp:453:7:453:9 | test.cpp:456:8:456:10 | 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 |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
index 4067f9d3971c..149851158e47 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
@@ -1,3 +1,8 @@
 | test.cpp:75:7:75:8 | u1 | test.cpp:76:8:76:9 | u1 |
 | test.cpp:83:7:83:8 | u2 | test.cpp:84:13:84:14 | u2 |
 | test.cpp:83:7:83:8 | u2 | test.cpp:85:8:85:9 | u2 |
+| test.cpp:447:7:447:9 | tmp | test.cpp:448:11:448:13 | tmp |
+| test.cpp:447:7:447:9 | tmp | test.cpp:449:8:449:10 | tmp |
+| test.cpp:453:7:453:9 | tmp | test.cpp:454:19:454:21 | tmp |
+| test.cpp:453:7:453:9 | tmp | test.cpp:455:11:455:13 | tmp |
+| test.cpp:453:7:453:9 | tmp | test.cpp:456:8:456:10 | tmp |

From 5647a1a658fbe2ee38d2bf4ed1c4a88d916779a5 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Tue, 26 Feb 2019 21:28:17 +0100
Subject: [PATCH 2/8] C++: BlockVar value stops at def by ref (partial)

---
 .../code/cpp/dataflow/internal/FlowVar.qll    | 53 +++++++++++++++++--
 .../dataflow-tests/localFlow.expected         |  3 --
 .../dataflow/dataflow-tests/test.cpp          |  2 +-
 .../dataflow/dataflow-tests/test.expected     |  2 -
 .../dataflow-tests/test_diff.expected         |  2 -
 .../dataflow-tests/uninitialized.expected     |  4 --
 6 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index 75d5d300bae5..2c2641a2ef3f 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -51,6 +51,12 @@ cached class FlowVar extends TFlowVar {
    */
   cached abstract predicate definedByExpr(Expr e, ControlFlowNode node);
 
+  /**
+   * Holds if this `FlowVar` corresponds to the data written by a call that
+   * passes a variable as argument `arg`.
+   */
+  cached abstract predicate definedByReference(Expr arg);
+
   /**
    * Holds if this `FlowVar` corresponds to the initial value of `v`. The following
    * is an exhaustive list of cases where this may happen.
@@ -137,6 +143,8 @@ module FlowVar_internal {
         or
         assignmentLikeOperation(sbb, v, _)
         or
+        blockVarDefinedByReference(sbb, v, _)
+        or
         blockVarDefinedByVariable(sbb, v)
       )
     }
@@ -174,6 +182,11 @@ module FlowVar_internal {
        else node = def.getDefinition())
     }
 
+    override predicate definedByReference(Expr arg) {
+      definitionByReference(v.getAnAccess(), arg) and
+      arg = def.getDefinition()
+    }
+
     override predicate definedByInitialValue(LocalScopeVariable param) {
       def.definedByParameter(param) and
       param = v
@@ -191,6 +204,8 @@ module FlowVar_internal {
       this.definedByExpr(_, _)
       or
       this.definedByInitialValue(_)
+      or
+      this.definedByReference(_)
     }
 
     /**
@@ -221,7 +236,17 @@ module FlowVar_internal {
     BlockVar() { this = TBlockVar(sbb, v) }
 
     override VariableAccess getAnAccess() {
-      variableAccessInSBB(v, getAReachedBlockVarSBB(this), result)
+      exists(SubBasicBlock reached |
+        reached = getAReachedBlockVarSBB(this)
+      |
+        variableAccessInSBB(v, reached, result)
+        or
+        // Allow flow into a `VariableAccess` that is used as definition by
+        // reference. This flow is blocked by `getAReachedBlockVarSBB` because
+        // flow should not propagate past that.
+        result = reached.getASuccessor().(VariableAccess) and
+        blockVarDefinedByReference(result, v, _)
+      )
     }
 
     override predicate definedByInitialValue(LocalScopeVariable lsv) {
@@ -237,6 +262,10 @@ module FlowVar_internal {
       node = sbb.getANode()
     }
 
+    override predicate definedByReference(Expr arg) {
+      blockVarDefinedByReference(sbb, v, arg)
+    }
+
     override string toString() {
       exists(Expr e |
         this.definedByExpr(e, _) and
@@ -246,9 +275,15 @@ module FlowVar_internal {
       this.definedByInitialValue(_) and
       result = "initial value of "+ v
       or
+      exists(Expr arg |
+        this.definedByReference(arg) and
+        result = "ref def: "+ arg
+      )
+      or
       // impossible case
       not this.definedByExpr(_, _) and
       not this.definedByInitialValue(_) and
+      not this.definedByReference(_) and
       result = "undefined "+ v
     }
 
@@ -373,7 +408,8 @@ module FlowVar_internal {
       mid = getAReachedBlockVarSBB(start) and
       result = mid.getASuccessor() and
       not skipLoop(mid, result, sbbDef, v) and
-      not assignmentLikeOperation(result, v, _)
+      not assignmentLikeOperation(result, v, _) and
+      not blockVarDefinedByReference(result, v, _)
     )
   }
 
@@ -481,6 +517,9 @@ module FlowVar_internal {
    */
   predicate overwrite(VariableAccess va, ControlFlowNode node) {
     va = node.(AssignExpr).getLValue()
+    or
+    va = node and
+    definitionByReference(node, _)
   }
 
   /**
@@ -515,6 +554,11 @@ module FlowVar_internal {
     )
   }
 
+  predicate blockVarDefinedByReference(ControlFlowNode node, Variable v, Expr argument) {
+    node = v.getAnAccess() and
+    definitionByReference(node, argument)
+  }
+
   /**
    * Holds if `v` is initialized by `init` to have value `assignedExpr`.
    */
@@ -534,8 +578,11 @@ module FlowVar_internal {
   class DataFlowSubBasicBlockCutNode extends SubBasicBlockCutNode {
     DataFlowSubBasicBlockCutNode() {
       exists(Variable v |
-        not fullySupportedSsaVariable(v) and
+        not fullySupportedSsaVariable(v)
+      |
         assignmentLikeOperation(this, v, _)
+        or
+        blockVarDefinedByReference(this, v, _)
         // It is not necessary to cut the basic blocks at `Initializer` nodes
         // because the affected variable can have no _other_ value before its
         // initializer. It is not necessary to cut basic blocks at procedure
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
index 22b74ac561bc..7c9a5d2725dc 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
@@ -34,6 +34,3 @@
 | 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:437:12:437:13 | 0 | test.cpp:439:33:439:35 | tmp |
-| test.cpp:437:12:437:13 | 0 | test.cpp:440:8:440:10 | tmp |
-| test.cpp:437:12:437:13 | 0 | test.cpp:442:10:442:12 | tmp |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
index 486a5950b3c5..1ceee5ab7d1c 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
@@ -453,5 +453,5 @@ void cleanedByMemcpy_blockvar(int clean1) {
   int tmp;
   int *capture = &tmp;
   memcpy(&tmp, &clean1, sizeof tmp);
-  sink(tmp); // clean (FALSE POSITIVE)
+  sink(tmp); // clean
 }
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
index 7358a4abb943..b2519f4805b9 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
@@ -27,8 +27,6 @@
 | 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:449:8:449:10 | tmp | test.cpp:447:7:447:9 | tmp |
-| test.cpp:456:8:456:10 | tmp | test.cpp:453:7:453:9 | tmp |
 | 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 |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
index dd902540f858..b7f2ae6d6ef3 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
@@ -9,8 +9,6 @@
 | 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:447:7:447:9 | test.cpp:449:8:449:10 | AST only |
-| test.cpp:453:7:453:9 | test.cpp:456:8:456:10 | 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 |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
index 149851158e47..82b70e6a8c42 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
@@ -1,8 +1,4 @@
 | test.cpp:75:7:75:8 | u1 | test.cpp:76:8:76:9 | u1 |
 | test.cpp:83:7:83:8 | u2 | test.cpp:84:13:84:14 | u2 |
 | test.cpp:83:7:83:8 | u2 | test.cpp:85:8:85:9 | u2 |
-| test.cpp:447:7:447:9 | tmp | test.cpp:448:11:448:13 | tmp |
-| test.cpp:447:7:447:9 | tmp | test.cpp:449:8:449:10 | tmp |
 | test.cpp:453:7:453:9 | tmp | test.cpp:454:19:454:21 | tmp |
-| test.cpp:453:7:453:9 | tmp | test.cpp:455:11:455:13 | tmp |
-| test.cpp:453:7:453:9 | tmp | test.cpp:456:8:456:10 | tmp |

From 80183464d9840acf2ee05f4f9ba8ab08489229f3 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Wed, 27 Feb 2019 14:32:00 +0100
Subject: [PATCH 3/8] C++: Define DefinitionByReferenceNode

This enables data flow through `memcpy` and similar functions modeled in
`semmle.code.cpp.model`.
---
 .../cpp/dataflow/internal/DataFlowUtil.qll    | 77 +++++++++++++++++++
 .../dataflow-tests/localFlow.expected         |  7 ++
 .../dataflow/dataflow-tests/test.cpp          |  6 +-
 .../dataflow/dataflow-tests/test.expected     |  3 +
 .../dataflow-tests/test_diff.expected         |  3 +
 .../dataflow/taint-tests/localTaint.expected  |  3 +
 6 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 62dca33d323a..ef533cea6f27 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/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
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
index 7c9a5d2725dc..04099670a0c1 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
+++ b/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 & ... |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
index 1ceee5ab7d1c..957a59e39250 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
+++ b/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
   }
 }
 
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
index b2519f4805b9..b7ab4580897c 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
+++ b/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 |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
index b7f2ae6d6ef3..a6807d945523 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
+++ b/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 |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
index 0ed943ffa23e..652e39d754da 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
+++ b/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 |  |

From 972d00822cf3ab58139f82845dd248c8a61da597 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Wed, 27 Feb 2019 15:28:54 +0100
Subject: [PATCH 4/8] C++: Generalize std::move data flow

---
 .../code/cpp/dataflow/internal/DataFlowUtil.qll      | 12 ++++++++----
 .../dataflow/dataflow-tests/localFlow.expected       |  2 ++
 .../dataflow/taint-tests/localTaint.expected         |  2 ++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index ef533cea6f27..fe51d1c3b07b 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -292,10 +292,14 @@ private predicate exprToExprStep_nocfg(Expr fromExpr, Expr toExpr) {
     fromExpr = op.getOperand()
   )
   or
-  toExpr = any(FunctionCall moveCall |
-    moveCall.getTarget().getNamespace().getName() = "std" and
-    moveCall.getTarget().getName() = "move" and
-    fromExpr = moveCall.getArgument(0)
+  toExpr = any(Call call |
+    exists(DataFlowFunction f, FunctionInput inModel , FunctionOutput outModel, int iIn |
+      call.getTarget() = f and
+      f.hasDataFlow(inModel, outModel) and
+      outModel.isOutReturnValue() and
+      inModel.isInParameter(iIn) and
+      fromExpr = call.getArgument(iIn)
+    )
   )
 }
 
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
index 04099670a0c1..30fd13629d7d 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
@@ -31,12 +31,14 @@
 | 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 | & ... | test.cpp:432:3:432:8 | call to memcpy |
 | 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 | & ... | test.cpp:439:3:439:8 | call to memcpy |
 | 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 |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
index 652e39d754da..601ad0a33528 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
@@ -129,8 +129,10 @@
 | 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 | buffer | taint.cpp:170:3:170:8 | call to strcpy |  |
 | 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 | buffer | taint.cpp:172:3:172:8 | call to strcat |  |
 | 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 |

From 7ff732d9627a3ba26e26e40e6a1a2ee1f2008fec Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Thu, 28 Feb 2019 08:23:24 +0100
Subject: [PATCH 5/8] C++: Use OO dispatch for getType and getFunction

---
 .../cpp/dataflow/internal/DataFlowUtil.qll    | 36 +++++++------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index fe51d1c3b07b..a062bbc2a253 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -24,15 +24,7 @@ private newtype TNode =
 */
 class Node extends TNode {
   /** Gets the function to which this node belongs. */
-  Function getFunction() {
-    result = this.asExpr().getEnclosingFunction()
-    or
-    result = this.asParameter().getFunction()
-    or
-    result = this.asUninitialized().getFunction()
-    or
-    result = this.asDefiningArgument().getEnclosingFunction()
-  }
+  Function getFunction() { none() } // overridden in subclasses
 
   /**
    * INTERNAL: Do not use. Alternative name for `getFunction`.
@@ -42,13 +34,7 @@ class Node extends TNode {
   }
 
   /** Gets the type of this node. */
-  Type getType() {
-    result = this.asExpr().getType()
-    or
-    result = asVariable(this).getType()
-    or
-    result = this.asDefiningVariableAccess().getType()
-  }
+  Type getType() { none() } // overridden in subclasses
 
   /** Gets the expression corresponding to this node, if any. */
   Expr asExpr() { result = this.(ExprNode).getExpr() }
@@ -90,6 +76,8 @@ class Node extends TNode {
 class ExprNode extends Node, TExprNode {
   Expr expr;
   ExprNode() { this = TExprNode(expr) }
+  override Function getFunction() { result = expr.getEnclosingFunction() }
+  override Type getType() { result = expr.getType() }
   override string toString() { result = expr.toString() }
   override Location getLocation() { result = expr.getLocation() }
   /** Gets the expression corresponding to this node. */
@@ -103,6 +91,8 @@ class ExprNode extends Node, TExprNode {
 class ParameterNode extends Node, TParameterNode {
   Parameter param;
   ParameterNode() { this = TParameterNode(param) }
+  override Function getFunction() { result = param.getFunction() }
+  override Type getType() { result = param.getType() }
   override string toString() { result = param.toString() }
   override Location getLocation() { result = param.getLocation() }
   /** Gets the parameter corresponding to this node. */
@@ -131,6 +121,8 @@ class DefinitionByReferenceNode extends Node, TDefinitionByReferenceNode {
   Expr argument;
 
   DefinitionByReferenceNode() { this = TDefinitionByReferenceNode(va, argument) }
+  override Function getFunction() { result = va.getEnclosingFunction() }
+  override Type getType() { result = va.getType() }
   override string toString() { result = "ref arg " + argument.toString() }
   override Location getLocation() { result = argument.getLocation() }
   /** Gets the argument corresponding to this node. */
@@ -146,6 +138,8 @@ class DefinitionByReferenceNode extends Node, TDefinitionByReferenceNode {
 class UninitializedNode extends Node, TUninitializedNode {
   LocalVariable v;
   UninitializedNode() { this = TUninitializedNode(v) }
+  override Function getFunction() { result = v.getFunction() }
+  override Type getType() { result = v.getType() }
   override string toString() { result = v.toString() }
   override Location getLocation() { result = v.getLocation() }
   /** Gets the uninitialized local variable corresponding to this node. */
@@ -206,12 +200,6 @@ UninitializedNode uninitializedNode(LocalVariable v) {
   result.getLocalVariable() = v
 }
 
-private Variable asVariable(Node node) {
-  result = node.asParameter()
-  or
-  result = node.asUninitialized()
-}
-
 /**
  * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
  * (intra-procedural) step.
@@ -225,7 +213,9 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) {
     (
       exprToVarStep(nodeFrom.asExpr(), var)
       or
-      varSourceBaseCase(var, asVariable(nodeFrom))
+      varSourceBaseCase(var, nodeFrom.asParameter())
+      or
+      varSourceBaseCase(var, nodeFrom.asUninitialized())
       or
       var.definedByReference(nodeFrom.asDefiningArgument())
     ) and

From 2bc0a8d6fb961fa245aa7356bf768e9abc77cc59 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Thu, 28 Feb 2019 08:25:15 +0100
Subject: [PATCH 6/8] C++: Remove getVariableAccess from def-by-ref node

This accessor may not be forward-compatible with an IR-based version,
and it's unclear whether it has any use. The `VariableAccess` remains in
the `TDefinitionByReferenceNode` constructor since it's used to
implement `getType`.
---
 .../cpp/dataflow/internal/DataFlowUtil.qll     | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index a062bbc2a253..261083c02df7 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -45,11 +45,6 @@ class Node extends TNode {
   /** 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.
@@ -113,8 +108,7 @@ class ParameterNode extends Node, TParameterNode {
  * 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`.
+ * returned. This node will have its `getArgument()` equal to `&x`.
  */
 class DefinitionByReferenceNode extends Node, TDefinitionByReferenceNode {
   VariableAccess va;
@@ -127,8 +121,6 @@ class DefinitionByReferenceNode extends Node, TDefinitionByReferenceNode {
   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 }
 }
 
 /**
@@ -184,14 +176,6 @@ 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`.

From 8e6daafd7ca3fee13d904dcdc04a9441207c1129 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Thu, 28 Feb 2019 09:10:38 +0100
Subject: [PATCH 7/8] C++: Add DefinitionByReferenceNode.getParameter

This commits also adds a test that uses `getParameter`. The new tests
demonstrate that support for array-to-pointer decay works, but we get
data flow to the array rather than its contents.
---
 .../cpp/dataflow/internal/DataFlowUtil.qll    |  7 ++++
 .../dataflow-tests/DataflowTestCommon.qll     |  2 ++
 .../dataflow/dataflow-tests/test.cpp          | 36 +++++++++++++++++++
 .../dataflow/dataflow-tests/test.expected     |  5 +++
 .../dataflow-tests/test_diff.expected         |  5 +++
 5 files changed, 55 insertions(+)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 261083c02df7..3839006cf342 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -121,6 +121,13 @@ class DefinitionByReferenceNode extends Node, TDefinitionByReferenceNode {
   override Location getLocation() { result = argument.getLocation() }
   /** Gets the argument corresponding to this node. */
   Expr getArgument() { result = argument }
+  /** Gets the parameter through which this value is assigned. */
+  Parameter getParameter() {
+    exists(FunctionCall call, int i |
+      argument = call.getArgument(i) and
+      result = call.getTarget().getParameter(i)
+    )
+  }
 }
 
 /**
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/DataflowTestCommon.qll b/cpp/ql/test/library-tests/dataflow/dataflow-tests/DataflowTestCommon.qll
index 2f9c932f2187..21bf44afd29a 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/DataflowTestCommon.qll
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/DataflowTestCommon.qll
@@ -12,6 +12,8 @@ class TestAllocationConfig extends DataFlow::Configuration {
     or
     source.asParameter().getName().matches("source%")
     or
+    source.(DataFlow::DefinitionByReferenceNode).getParameter().getName().matches("ref_source%")
+    or
     // Track uninitialized variables
     exists(source.asUninitialized())
   }
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
index 957a59e39250..9fa3ad7f2f35 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
@@ -455,3 +455,39 @@ void cleanedByMemcpy_blockvar(int clean1) {
   memcpy(&tmp, &clean1, sizeof tmp);
   sink(tmp); // clean
 }
+
+void intRefSource(int &ref_source);
+void intPointerSource(int *ref_source);
+void intArraySource(int ref_source[], size_t len);
+
+void intRefSourceCaller() {
+  int local;
+  intRefSource(local);
+  sink(local); // tainted
+}
+
+void intPointerSourceCaller() {
+  int local;
+  intPointerSource(&local);
+  sink(local); // tainted
+}
+
+void intPointerSourceCaller2() {
+  int local[1];
+  intPointerSource(local);
+  sink(local); // tainted
+  sink(*local); // clean
+}
+
+void intArraySourceCaller() {
+  int local;
+  intArraySource(&local, 1);
+  sink(local); // tainted
+}
+
+void intArraySourceCaller2() {
+  int local[2];
+  intArraySource(local, 2);
+  sink(local); // tainted
+  sink(*local); // clean
+}
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
index b7ab4580897c..38cbbc1bc4bc 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
@@ -30,6 +30,11 @@
 | 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 |
+| test.cpp:466:8:466:12 | local | test.cpp:465:16:465:20 | ref arg local |
+| test.cpp:472:8:472:12 | local | test.cpp:471:20:471:25 | ref arg & ... |
+| test.cpp:478:8:478:12 | local | test.cpp:477:20:477:24 | ref arg local |
+| test.cpp:485:8:485:12 | local | test.cpp:484:18:484:23 | ref arg & ... |
+| test.cpp:491:8:491:12 | local | test.cpp:490:18:490:22 | ref arg local |
 | 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 |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
index a6807d945523..1eccb0fedf19 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
@@ -12,6 +12,11 @@
 | 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 |
+| test.cpp:465:16:465:20 | test.cpp:466:8:466:12 | AST only |
+| test.cpp:471:20:471:25 | test.cpp:472:8:472:12 | AST only |
+| test.cpp:477:20:477:24 | test.cpp:478:8:478:12 | AST only |
+| test.cpp:484:18:484:23 | test.cpp:485:8:485:12 | AST only |
+| test.cpp:490:18:490:22 | test.cpp:491:8:491: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 |

From 7afb4898e64d949cdd594adb1ab4ac8692d19078 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Thu, 28 Feb 2019 09:37:51 +0100
Subject: [PATCH 8/8] C++: Change note for def-by-ref data flow

---
 change-notes/1.20/analysis-cpp.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/change-notes/1.20/analysis-cpp.md b/change-notes/1.20/analysis-cpp.md
index 578fbbc99660..631f881eb558 100644
--- a/change-notes/1.20/analysis-cpp.md
+++ b/change-notes/1.20/analysis-cpp.md
@@ -34,6 +34,10 @@
 
 ## Changes to QL libraries
 
+* The `semmle.code.cpp.dataflow.DataFlow` library now supports _definition by reference_ via output parameters of known functions.
+    * Data flows through `memcpy` and `memmove` by default.
+    * Custom flow into or out of arguments assigned by reference can be modelled with the new class `DataFlow::DefinitionByReferenceNode`.
+    * The data flow library adds flow through library functions that are modeled in `semmle.code.cpp.models.interfaces.DataFlow`. Queries can add subclasses of `DataFlowFunction` to specify additional flow.
 * There is a new `Namespace.isInline()` predicate, which holds if the namespace was declared as `inline namespace`.
 * The `Expr.isConstant()` predicate now also holds for _address constant expressions_, which are addresses that will be constant after the program has been linked. These address constants do not have a result for `Expr.getValue()`.
 * There are new `Function.isDeclaredConstexpr()` and `Function.isConstexpr()` predicates. They can be used to tell whether a function was declared as `constexpr`, and whether it actually is `constexpr`.