C++: output iterator flow with user-defined operators

Author: Robert Marsh

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

@@ -52,6 +52,17 @@ private predicate lvalueToLvalueStep(Expr lvalueIn, Expr lvalueOut) {
   or
   // C++ only
   lvalueIn = lvalueOut.(Assignment).getLValue().getFullyConverted()
+  or
+  // C++ only
+  exists(Call c |
+    lvalueOut = c and
+    c.getTarget().hasName(["operator*", "operator++"]) and
+    (
+      c.getQualifier() = lvalueIn or
+      not c.getTarget() instanceof MemberFunction and
+      c.getArgument(0) = lvalueIn
+    )
+  )
 }
 
 private predicate pointerToLvalueStep(Expr pointerIn, Expr lvalueOut) {
@@ -92,6 +103,17 @@ private predicate pointerToPointerStep(Expr pointerIn, Expr pointerOut) {
 
 private predicate lvalueToReferenceStep(Expr lvalueIn, Expr referenceOut) {
   lvalueIn.getConversion() = referenceOut.(ReferenceToExpr)
+  or
+  // C++ only
+  exists(Call c |
+    referenceOut = c and
+    c.getTarget().hasName(["operator*", "operator++"]) and
+    (
+      c.getQualifier() = lvalueIn or
+      not c.getTarget() instanceof MemberFunction and
+      c.getArgument(0) = lvalueIn
+    )
+  )
 }
 
 private predicate referenceToLvalueStep(Expr referenceIn, Expr lvalueOut) {

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

@@ -585,6 +585,23 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
     nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr() = call and
     nodeTo.asDefiningArgument() = call.getQualifier()
   )
+  or
+  // Iterators that have a user-defined `operator=`. Take flow from the RHS to
+  // the post-update node for the iterator.
+  // The built-in `=` case is handled by FlowVar, other user-defined `operator=`
+  // will be handled by interprocedural flow.
+  exists(IteratorPartialDefinitionNode postUpdate, Call opEquals |
+    postUpdate = nodeTo and
+    opEquals.getTarget().hasName("operator=") and
+    if opEquals.getTarget() instanceof MemberFunction
+    then
+      opEquals.getQualifier() = postUpdate.getPreUpdateNode().asExpr() and
+      opEquals.getArgument(0) = nodeFrom.asExpr()
+    else (
+      opEquals.getArgument(0) = postUpdate.getPreUpdateNode().asExpr() and
+      opEquals.getArgument(1) = nodeFrom.asExpr()
+    )
+  )
 }
 
 /**

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

@@ -163,9 +163,9 @@ private module PartialDefinitions {
         valueToUpdate(convertedInner, this.getFullyConverted(), node) and
         innerDefinedExpr = convertedInner.getUnconverted() and
         (
-          innerDefinedExpr.(Call).getQualifier() = getAnIteratorAccess(collection)
+          innerDefinedExpr.(Call).getQualifier() = getCrementedExpr*(getAnIteratorAccess(collection))
           or
-          innerDefinedExpr.(Call).getQualifier() = collection.getAnAccess() and
+          innerDefinedExpr.(Call).getQualifier() = getCrementedExpr*(collection.getAnAccess()) and
           collection instanceof IteratorParameter
         ) and
         innerDefinedExpr.(Call).getTarget() instanceof IteratorPointerDereferenceMemberOperator
@@ -823,10 +823,12 @@ module FlowVar_internal {
       def.getAnUltimateDefiningValue(iterator) = c and
       result = def.getAUse(iterator)
     )
-    or
+  }
+
+  Expr getCrementedExpr(Expr e) {
     exists(Call crement |
       crement = result and
-      [crement.getQualifier(), crement.getArgument(0)] = getAnIteratorAccess(collection) and
+      [crement.getQualifier(), crement.getArgument(0)] = e and
       crement.getTarget().getName() = ["operator++", "operator--"]
     )
   }

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

@@ -250,6 +250,10 @@
 | smart_pointer.cpp:13:10:13:10 | p | smart_pointer.cpp:11:52:11:57 | call to source |
 | smart_pointer.cpp:24:10:24:10 | call to operator* | smart_pointer.cpp:23:52:23:57 | call to source |
 | smart_pointer.cpp:25:10:25:10 | p | smart_pointer.cpp:23:52:23:57 | call to source |
+| smart_pointer.cpp:38:10:38:10 | p | smart_pointer.cpp:37:10:37:15 | call to source |
+| smart_pointer.cpp:39:10:39:10 | call to operator* | smart_pointer.cpp:37:10:37:15 | call to source |
+| smart_pointer.cpp:46:10:46:10 | p | smart_pointer.cpp:45:10:45:15 | call to source |
+| smart_pointer.cpp:47:10:47:10 | call to operator* | smart_pointer.cpp:45:10:45:15 | call to source |
 | smart_pointer.cpp:52:12:52:14 | call to get | smart_pointer.cpp:51:52:51:57 | call to source |
 | smart_pointer.cpp:57:12:57:14 | call to get | smart_pointer.cpp:56:52:56:57 | call to source |
 | standalone_iterators.cpp:40:10:40:10 | call to operator* | standalone_iterators.cpp:39:45:39:51 | source1 |
@@ -258,6 +262,9 @@
 | standalone_iterators.cpp:46:10:46:10 | call to operator* | standalone_iterators.cpp:45:39:45:45 | source1 |
 | standalone_iterators.cpp:47:10:47:10 | call to operator* | standalone_iterators.cpp:45:39:45:45 | source1 |
 | standalone_iterators.cpp:48:10:48:10 | call to operator* | standalone_iterators.cpp:45:39:45:45 | source1 |
+| standalone_iterators.cpp:52:10:52:10 | call to operator* | standalone_iterators.cpp:51:37:51:43 | source1 |
+| standalone_iterators.cpp:53:10:53:10 | call to operator* | standalone_iterators.cpp:51:37:51:43 | source1 |
+| standalone_iterators.cpp:54:10:54:10 | call to operator* | standalone_iterators.cpp:51:37:51:43 | source1 |
 | string.cpp:29:7:29:7 | a | string.cpp:25:12:25:17 | call to source |
 | string.cpp:31:7:31:7 | c | string.cpp:27:16:27:21 | call to source |
 | string.cpp:33:9:33:13 | call to c_str | string.cpp:27:16:27:21 | call to source |
@@ -657,3 +664,5 @@
 | vector.cpp:409:7:409:9 | v13 | vector.cpp:408:11:408:16 | call to source |
 | vector.cpp:414:7:414:9 | v14 | vector.cpp:413:11:413:16 | call to source |
 | vector.cpp:422:8:422:10 | out | vector.cpp:417:33:417:45 | source_string |
+| vector.cpp:429:8:429:10 | out | vector.cpp:417:33:417:45 | source_string |
+| vector.cpp:436:8:436:10 | out | vector.cpp:435:11:435:16 | call to source |

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

@@ -0,0 +1,7 @@
+import TaintTestCommon
+
+import DataFlow::PathGraph
+
+from DataFlow::PathNode sink, DataFlow::PathNode source, TestAllocationConfig cfg
+where cfg.hasFlowPath(source, sink)
+select sink, source

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

@@ -200,10 +200,17 @@
 | set.cpp:237:7:237:9 | set.cpp:236:37:236:42 | AST only |
 | smart_pointer.cpp:12:10:12:10 | smart_pointer.cpp:11:52:11:57 | AST only |
 | smart_pointer.cpp:24:10:24:10 | smart_pointer.cpp:23:52:23:57 | AST only |
+| smart_pointer.cpp:38:10:38:10 | smart_pointer.cpp:37:10:37:15 | AST only |
+| smart_pointer.cpp:39:10:39:10 | smart_pointer.cpp:37:10:37:15 | AST only |
+| smart_pointer.cpp:46:10:46:10 | smart_pointer.cpp:45:10:45:15 | AST only |
+| smart_pointer.cpp:47:10:47:10 | smart_pointer.cpp:45:10:45:15 | AST only |
 | standalone_iterators.cpp:41:10:41:10 | standalone_iterators.cpp:39:45:39:51 | AST only |
 | standalone_iterators.cpp:42:10:42:10 | standalone_iterators.cpp:39:45:39:51 | AST only |
 | standalone_iterators.cpp:47:10:47:10 | standalone_iterators.cpp:45:39:45:45 | AST only |
 | standalone_iterators.cpp:48:10:48:10 | standalone_iterators.cpp:45:39:45:45 | AST only |
+| standalone_iterators.cpp:52:10:52:10 | standalone_iterators.cpp:51:37:51:43 | AST only |
+| standalone_iterators.cpp:53:10:53:10 | standalone_iterators.cpp:51:37:51:43 | AST only |
+| standalone_iterators.cpp:54:10:54:10 | standalone_iterators.cpp:51:37:51:43 | AST only |
 | string.cpp:33:9:33:13 | string.cpp:27:16:27:21 | AST only |
 | string.cpp:39:13:39:17 | string.cpp:14:10:14:15 | AST only |
 | string.cpp:43:13:43:17 | string.cpp:14:10:14:15 | AST only |
@@ -383,3 +390,5 @@
 | vector.cpp:409:7:409:9 | vector.cpp:408:11:408:16 | AST only |
 | vector.cpp:414:7:414:9 | vector.cpp:413:11:413:16 | AST only |
 | vector.cpp:422:8:422:10 | vector.cpp:417:33:417:45 | AST only |
+| vector.cpp:429:8:429:10 | vector.cpp:417:33:417:45 | AST only |
+| vector.cpp:436:8:436:10 | vector.cpp:435:11:435:16 | AST only |