Merge pull request #4468 from github/rdmarsh2/cpp/output-iterators-2

C++: flow through output iterators with user-defined operator= and operator*

Author: jbj

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

@@ -801,12 +801,34 @@ module FlowVar_internal {
   }
 
   Expr getAnIteratorAccess(Variable collection) {
-    exists(Call c, SsaDefinition def, Variable iterator |
-      c.getQualifier() = collection.getAnAccess() and
-      c.getTarget() instanceof BeginOrEndFunction and
+    exists(
+      Call c, SsaDefinition def, Variable iterator, FunctionInput input, FunctionOutput output
+    |
+      c.getTarget().(GetIteratorFunction).getsIterator(input, output) and
+      (
+        (
+          input.isQualifierObject() or
+          input.isQualifierAddress()
+        ) and
+        c.getQualifier() = collection.getAnAccess()
+        or
+        exists(int index |
+          input.isParameter(index) or
+          input.isParameterDeref(index)
+        |
+          c.getArgument(index) = collection.getAnAccess()
+        )
+      ) and
+      output.isReturnValue() and
       def.getAnUltimateDefiningValue(iterator) = c and
       result = def.getAUse(iterator)
     )
+    or
+    exists(Call crement |
+      crement = result and
+      [crement.getQualifier(), crement.getArgument(0)] = getAnIteratorAccess(collection) and
+      crement.getTarget().getName() = ["operator++", "operator--"]
+    )
   }
 
   class IteratorParameter extends Parameter {

cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll

@@ -92,6 +92,9 @@ class IteratorPointerDereferenceOperator extends Operator, TaintFunction, Iterat
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input = iteratorInput and
     output.isReturnValue()
+    or
+    input.isReturnValueDeref() and
+    output.isParameterDeref(0)
   }
 }
 
@@ -180,6 +183,9 @@ class IteratorPointerDereferenceMemberOperator extends MemberFunction, TaintFunc
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isQualifierObject() and
     output.isReturnValue()
+    or
+    input.isReturnValueDeref() and
+    output.isQualifierObject()
   }
 }
 
@@ -274,11 +280,32 @@ class IteratorArrayMemberOperator extends MemberFunction, TaintFunction, Iterato
   }
 }
 
+/**
+ * An `operator=` member function of an iterator class that is not a copy or move assignment
+ * operator.
+ *
+ * The `hasTaintFlow` override provides flow through output iterators that return themselves with
+ * `operator*` and use their own `operator=` to assign to the container.
+ */
+class IteratorAssignmentMemberOperator extends MemberFunction, TaintFunction {
+  IteratorAssignmentMemberOperator() {
+    this.hasName("operator=") and
+    this.getDeclaringType() instanceof Iterator and
+    not this instanceof CopyAssignmentOperator and
+    not this instanceof MoveAssignmentOperator
+  }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    input.isParameterDeref(0) and
+    output.isQualifierObject()
+  }
+}
+
 /**
  * A `begin` or `end` member function, or a related member function, that
  * returns an iterator.
  */
-class BeginOrEndFunction extends MemberFunction, TaintFunction {
+class BeginOrEndFunction extends MemberFunction, TaintFunction, GetIteratorFunction {
   BeginOrEndFunction() {
     this
         .hasName(["begin", "cbegin", "rbegin", "crbegin", "end", "cend", "rend", "crend",
@@ -290,4 +317,24 @@ class BeginOrEndFunction extends MemberFunction, TaintFunction {
     input.isQualifierObject() and
     output.isReturnValue()
   }
+
+  override predicate getsIterator(FunctionInput input, FunctionOutput output) {
+    input.isQualifierObject() and
+    output.isReturnValue()
+  }
+}
+
+/**
+ * The `std::front_inserter`, `std::inserter`, and `std::back_inserter`
+ * functions.
+ */
+class InserterIteratorFunction extends GetIteratorFunction {
+  InserterIteratorFunction() {
+    this.hasQualifiedName("std", ["front_inserter", "inserter", "back_inserter"])
+  }
+
+  override predicate getsIterator(FunctionInput input, FunctionOutput output) {
+    input.isParameterDeref(0) and
+    output.isReturnValue()
+  }
 }

cpp/ql/src/semmle/code/cpp/models/interfaces/Iterator.qll

@@ -15,3 +15,14 @@ import semmle.code.cpp.models.Models
  * can be used to write to the iterator's underlying collection.
  */
 abstract class IteratorReferenceFunction extends Function { }
+
+/**
+ * A function which takes a container and returns an iterator over that container.
+ */
+abstract class GetIteratorFunction extends Function {
+  /**
+   * Holds if the return value or buffer represented by `output` is an iterator over the container
+   * passd in the argument, qualifier, or buffer represented by `input`.
+   */
+  abstract predicate getsIterator(FunctionInput input, FunctionOutput output);
+}