C++: Fix iterator flow context sensitivity

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

@@ -1,6 +1,8 @@
 private import cpp
 private import DataFlowUtil
 private import DataFlowDispatch
+private import FlowVar
+
 
 /** Gets the instance argument of a non-static call. */
 private Node getInstanceArgument(Call call) {
@@ -106,7 +108,7 @@ private class ExprOutNode extends OutNode, ExprNode {
   override DataFlowCall getCall() { result = this.getExpr() }
 }
 
-private class RefOutNode extends OutNode, DefinitionByReferenceNode {
+private class RefOutNode extends OutNode, DefinitionByReferenceOrIteratorNode {
   /** Gets the underlying call. */
   override DataFlowCall getCall() { result = this.getArgument().getParent() }
 }
@@ -120,7 +122,7 @@ OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
   kind = TNormalReturnKind()
   or
   exists(int i |
-    result.asDefiningArgument() = call.getArgument(i) and
+    result.(DefinitionByReferenceOrIteratorNode).getArgument() = call.getArgument(i) and
     kind = TRefReturnKind(i)
   )
 }

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

@@ -182,29 +182,23 @@ class ImplicitParameterNode extends ParameterNode, TInstanceParameterNode {
   override predicate isParameterOf(Function fun, int i) { f = fun and i = -1 }
 }
 
-/**
- * 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`.
- */
-class DefinitionByReferenceNode extends PartialDefinitionNode {
+class DefinitionByReferenceOrIteratorNode extends PartialDefinitionNode {
   Expr inner;
   Expr argument;
 
-  DefinitionByReferenceNode() {
-    this.getPartialDefinition().(DefinitionByReference).definesExpressions(inner, argument)
+  DefinitionByReferenceOrIteratorNode() {
+    this.getPartialDefinition().definesExpressions(inner, argument) and
+    (
+      this.getPartialDefinition() instanceof DefinitionByReference
+      or
+      this.getPartialDefinition() instanceof DefinitionByIterator
+    )
   }
 
   override Function getFunction() { result = inner.getEnclosingFunction() }
 
   override Type getType() { result = inner.getType() }
 
-  override string toString() { result = "ref arg " + argument.toString() }
-
   override Location getLocation() { result = argument.getLocation() }
 
   override ExprNode getPreUpdateNode() { result.getExpr() = argument }
@@ -221,6 +215,22 @@ class DefinitionByReferenceNode extends PartialDefinitionNode {
   }
 }
 
+
+/**
+ * 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`.
+ */
+class DefinitionByReferenceNode extends DefinitionByReferenceOrIteratorNode {
+  override VariablePartialDefinition pd;
+
+  override string toString() { result = "ref arg " + argument.toString() }
+}
+
 /**
  * The value of an uninitialized local variable, viewed as a node in a data
  * flow graph.
@@ -551,10 +561,10 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   or
   // In `f(&x->a)`, this step provides the flow from post-`&` to post-`x->a`,
   // from which there is field flow to `x` via reverse read.
-  exists(VariablePartialDefinition def, Expr inner, Expr outer |
+  exists(PartialDefinition def, Expr inner, Expr outer |
     def.definesExpressions(inner, outer) and
     inner = nodeTo.(InnerPartialDefinitionNode).getPreUpdateNode().asExpr() and
-    outer = nodeFrom.(VariablePartialDefinitionNode).getPreUpdateNode().asExpr()
+    outer = nodeFrom.(PartialDefinitionNode).getPreUpdateNode().asExpr()
   )
   or
   // Reverse flow: data that flows from the post-update node of a reference

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

@@ -243,6 +243,13 @@ private module PartialDefinitions {
     }
   }
 
+  /**
+   * A partial definition that's a definition by reference.
+   */
+  class DefinitionByIterator extends IteratorPartialDefinition {
+    DefinitionByIterator() { exists(Call c | this = c.getAnArgument() or this = c.getQualifier()) }
+  }
+
   /**
    * A partial definition that's a definition by reference.
    */

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

@@ -31,11 +31,6 @@ predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) {
  */
 predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
   localAdditionalTaintStep(src, sink)
-  or
-  exists(FunctionCall call, int i |
-    sink.(DataFlow::IteratorPartialDefinitionNode).getPartialDefinition().definesExpressions(_, call.getArgument(i)) and
-    src.(DataFlow::RefParameterFinalValueNode).getParameter() = call.getTarget().getParameter(i)
-  )
 }
 
 /**

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

@@ -388,5 +388,4 @@
 | vector.cpp:385:7:385:8 | v8 | vector.cpp:382:8:382:13 | call to source |
 | vector.cpp:392:7:392:8 | v9 | vector.cpp:330:10:330:15 | call to source |
 | vector.cpp:392:7:392:8 | v9 | vector.cpp:389:8:389:13 | call to source |
-| vector.cpp:396:7:396:9 | v10 | vector.cpp:399:38:399:43 | call to source |
 | vector.cpp:400:7:400:9 | v11 | vector.cpp:399:38:399:43 | call to source |

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

@@ -313,5 +313,4 @@
 | vector.cpp:385:7:385:8 | vector.cpp:382:8:382:13 | AST only |
 | vector.cpp:392:7:392:8 | vector.cpp:330:10:330:15 | AST only |
 | vector.cpp:392:7:392:8 | vector.cpp:389:8:389:13 | AST only |
-| vector.cpp:396:7:396:9 | vector.cpp:399:38:399:43 | AST only |
 | vector.cpp:400:7:400:9 | vector.cpp:399:38:399:43 | AST only |