C++: Fix Iterator.qll taint/data flows for operator+=.

Author: geoffw0

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

@@ -167,11 +167,15 @@ private class IteratorAssignArithmeticOperator extends Operator, DataFlowFunctio
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     input.isParameter(0) and
     output.isReturnValue()
-    or
-    input.isParameterDeref(0) and output.isReturnValueDeref()
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    input.isParameterDeref(0) and output.isReturnValueDeref()
+    or
+    // reverse flow from returned reference to the object referenced by the first parameter
+    input.isReturnValueDeref() and
+    output.isParameterDeref(0)
+    or
     input.isParameterDeref(1) and
     output.isParameterDeref(0)
   }
@@ -224,9 +228,7 @@ private class IteratorCrementMemberOperator extends MemberFunction, DataFlowFunc
  * A member `operator->` function for an iterator type.
  */
 private class IteratorFieldMemberOperator extends Operator, TaintFunction {
-  IteratorFieldMemberOperator() {
-    this.getClassAndName("operator->") instanceof Iterator
-  }
+  IteratorFieldMemberOperator() { this.getClassAndName("operator->") instanceof Iterator }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isQualifierObject() and
@@ -260,14 +262,18 @@ private class IteratorAssignArithmeticMemberOperator extends MemberFunction, Dat
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     input.isQualifierAddress() and
     output.isReturnValue()
-    or
-    input.isReturnValueDeref() and
-    output.isQualifierObject()
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isQualifierObject() and
     output.isReturnValueDeref()
+    or
+    // reverse flow from returned reference to the qualifier
+    input.isReturnValueDeref() and
+    output.isQualifierObject()
+    or
+    input.isParameterDeref(0) and
+    output.isQualifierObject()
   }
 }
 
@@ -276,9 +282,7 @@ private class IteratorAssignArithmeticMemberOperator extends MemberFunction, Dat
  */
 private class IteratorArrayMemberOperator extends MemberFunction, TaintFunction,
   IteratorReferenceFunction {
-  IteratorArrayMemberOperator() {
-    this.getClassAndName("operator[]") instanceof Iterator
-  }
+  IteratorArrayMemberOperator() { this.getClassAndName("operator[]") instanceof Iterator }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isQualifierObject() and

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

@@ -3926,10 +3926,12 @@
 | string.cpp:408:8:408:9 | i2 | string.cpp:409:10:409:11 | i7 |  |
 | string.cpp:409:10:409:11 | i7 | string.cpp:409:12:409:12 | call to operator+= |  |
 | string.cpp:409:12:409:12 | call to operator+= | string.cpp:409:8:409:8 | call to operator* | TAINT |
+| string.cpp:409:14:409:14 | 1 | string.cpp:409:10:409:11 | ref arg i7 | TAINT |
 | string.cpp:410:8:410:9 | i2 | string.cpp:410:3:410:9 | ... = ... |  |
 | string.cpp:410:8:410:9 | i2 | string.cpp:411:10:411:11 | i8 |  |
 | string.cpp:411:10:411:11 | i8 | string.cpp:411:12:411:12 | call to operator-= |  |
 | string.cpp:411:12:411:12 | call to operator-= | string.cpp:411:8:411:8 | call to operator* | TAINT |
+| string.cpp:411:14:411:14 | 1 | string.cpp:411:10:411:11 | ref arg i8 | TAINT |
 | string.cpp:413:8:413:9 | s2 | string.cpp:413:11:413:13 | call to end | TAINT |
 | string.cpp:413:11:413:13 | call to end | string.cpp:413:3:413:15 | ... = ... |  |
 | string.cpp:413:11:413:13 | call to end | string.cpp:414:5:414:6 | i9 |  |
@@ -7582,9 +7584,11 @@
 | vector.cpp:528:3:528:4 | ref arg it | vector.cpp:529:9:529:10 | it |  |
 | vector.cpp:528:3:528:4 | ref arg it | vector.cpp:530:3:530:4 | it |  |
 | vector.cpp:528:3:528:4 | ref arg it | vector.cpp:531:9:531:10 | it |  |
+| vector.cpp:528:9:528:9 | 1 | vector.cpp:528:3:528:4 | ref arg it | TAINT |
 | vector.cpp:529:9:529:10 | it | vector.cpp:529:8:529:8 | call to operator* | TAINT |
 | vector.cpp:530:3:530:4 | it | vector.cpp:530:6:530:6 | call to operator+= |  |
 | vector.cpp:530:3:530:4 | ref arg it | vector.cpp:531:9:531:10 | it |  |
+| vector.cpp:530:9:530:14 | call to source | vector.cpp:530:3:530:4 | ref arg it | TAINT |
 | vector.cpp:531:9:531:10 | it | vector.cpp:531:8:531:8 | call to operator* | TAINT |
 | vector.cpp:532:8:532:9 | ref arg vs | vector.cpp:533:2:533:2 | vs |  |
 | vector.cpp:532:8:532:9 | vs | vector.cpp:532:10:532:10 | call to operator[] | TAINT |

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

@@ -528,7 +528,7 @@ void test_vector_iterator() {
 		it += 1;
 		sink(*it);
 		it += source();
-		sink(*it); // $ MISSING: ast,ir
+		sink(*it); // $ ast,ir
 		sink(vs[1]);
 	}
 }