C++: respond to PR comments on Iterator.qll

cpp/ql/src/semmle/code/cpp/models/Models.qll

@@ -4,6 +4,7 @@ private import implementations.Fread
 private import implementations.Gets
 private import implementations.IdentityFunction
 private import implementations.Inet
+private import implementations.Iterator
 private import implementations.MemberFunction
 private import implementations.Memcpy
 private import implementations.Memset

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

@@ -2,6 +2,9 @@ import cpp
 import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.DataFlow
 
+/**
+ * An instantiation of the `std::iterator_traits` template.
+ */
 class IteratorTraits extends Class {
   IteratorTraits() {
     this.hasQualifiedName("std", "iterator_traits") and
@@ -15,6 +18,9 @@ class IteratorTraits extends Class {
   Type getIteratorType() { result = this.getTemplateArgument(0) }
 }
 
+/**
+ * A type which has the typedefs expected for an iterator.
+ */
 class IteratorByTypedefs extends Class {
   IteratorByTypedefs() {
     this.getAMember().(TypedefType).hasName("difference_type") and
@@ -26,25 +32,32 @@ class IteratorByTypedefs extends Class {
   }
 }
 
+/**
+ * The `std::iterator` class.
+ */
 class StdIterator extends Class {
-  StdIterator() {
-    this.hasQualifiedName("std", "iterator")
-  }
+  StdIterator() { this.hasQualifiedName("std", "iterator") }
 }
 
-class LegacyIterator extends Type {
-  LegacyIterator() {
+/**
+ * A type which can be used as an iterator
+ */
+class Iterator extends Type {
+  Iterator() {
     this instanceof IteratorByTypedefs or
     exists(IteratorTraits it | it.getIteratorType() = this) or
     this instanceof StdIterator
   }
 }
 
+/**
+ * A non-member prefix `operator*` function for an iterator type.
+ */
 class IteratorPointerDereferenceOperator extends Operator, TaintFunction {
   IteratorPointerDereferenceOperator() {
     this.hasName("operator*") and
     this.getACallToThisFunction().getArgument(0).getFullyConverted().getUnderlyingType() instanceof
-      LegacyIterator
+      Iterator
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -53,7 +66,10 @@ class IteratorPointerDereferenceOperator extends Operator, TaintFunction {
   }
 }
 
-class IteratorCrementOperator extends Operator, DataFlowFunction, TaintFunction {
+/**
+ * A non-member `operator++` or `operator--` function for an iterator type.
+ */
+class IteratorCrementOperator extends Operator, DataFlowFunction {
   IteratorCrementOperator() {
     (
       this.hasName("operator++") or
@@ -65,20 +81,18 @@ class IteratorCrementOperator extends Operator, DataFlowFunction, TaintFunction
         .getFullyConverted()
         .getUnderlyingType()
         .(ReferenceType)
-        .getBaseType() instanceof LegacyIterator
+        .getBaseType() instanceof Iterator
   }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     input.isParameter(0) and
     output.isReturnValue()
   }
-
-  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isParameterDeref(0) and
-    output.isParameterDeref(0)
-  }
 }
 
+/**
+ * A non-member `operator->` function for an iterator type.
+ */
 class IteratorFieldOperator extends Operator, TaintFunction {
   IteratorFieldOperator() {
     this.hasName("operator->") and
@@ -88,7 +102,7 @@ class IteratorFieldOperator extends Operator, TaintFunction {
         .getFullyConverted()
         .getUnspecifiedType()
         .(PointerType)
-        .getBaseType() instanceof LegacyIterator
+        .getBaseType() instanceof Iterator
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -97,6 +111,9 @@ class IteratorFieldOperator extends Operator, TaintFunction {
   }
 }
 
+/**
+ * A non-member `operator+` function for an iterator type.
+ */
 class IteratorAddOperator extends Operator, TaintFunction {
   IteratorAddOperator() {
     this.hasName("operator+") and
@@ -107,14 +124,14 @@ class IteratorAddOperator extends Operator, TaintFunction {
           .getFullyConverted()
           .getUnspecifiedType()
           .(PointerType)
-          .getBaseType() instanceof LegacyIterator or
+          .getBaseType() instanceof Iterator or
       this
           .getACallToThisFunction()
           .getArgument(0)
           .getFullyConverted()
           .getUnspecifiedType()
           .(PointerType)
-          .getBaseType() instanceof LegacyIterator
+          .getBaseType() instanceof Iterator
     )
   }
 
@@ -128,18 +145,14 @@ class IteratorAddOperator extends Operator, TaintFunction {
 }
 
 /**
- * A non-member `operator-` function that takes an iterator as its first argument.
+ * A non-member `operator-` function that takes an iterator as its first argument. This includes
+ * both iterator subtraction and iterator difference overloaded operators.
  */
 class IteratorSubOperator extends Operator, TaintFunction {
   IteratorSubOperator() {
     this.hasName("operator-") and
-    this
-        .getACallToThisFunction()
-        .getArgument(0)
-        .getFullyConverted()
-        .getUnspecifiedType()
-        .(PointerType)
-        .getBaseType() instanceof LegacyIterator
+    this.getACallToThisFunction().getArgument(0).getType().(PointerType).getBaseType() instanceof
+      Iterator
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -151,13 +164,16 @@ class IteratorSubOperator extends Operator, TaintFunction {
   }
 }
 
-class IteratorAssignArithmeticOperator extends MemberFunction, DataFlowFunction, TaintFunction {
+/**
+ * A non-member `operator+=` or `operator-=` function for an iterator type.
+ */
+class IteratorAssignArithmeticOperator extends Operator, DataFlowFunction, TaintFunction {
   IteratorAssignArithmeticOperator() {
     (
       this.hasName("operator+=") or
       this.hasName("operator-=")
     ) and
-    this.getDeclaringType() instanceof LegacyIterator
+    this.getDeclaringType() instanceof Iterator
   }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
@@ -171,10 +187,26 @@ class IteratorAssignArithmeticOperator extends MemberFunction, DataFlowFunction,
   }
 }
 
+class IteratorArrayOperator extends Operator, TaintFunction {
+  IteratorArrayOperator() {
+    this.hasName("operator[]") and
+    this.getACallToThisFunction().getArgument(0).getType().(PointerType).getBaseType() instanceof
+      Iterator
+  }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    (input.isParameter(0) or input.isParameter(1)) and
+    output.isReturnValue()
+  }
+}
+
+/**
+ * A prefix `operator*` member function for an iterator type.
+ */
 class IteratorPointerDereferenceMemberOperator extends MemberFunction, TaintFunction {
   IteratorPointerDereferenceMemberOperator() {
     this.hasName("operator*") and
-    this.getDeclaringType() instanceof LegacyIterator
+    this.getDeclaringType() instanceof Iterator
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -183,30 +215,31 @@ class IteratorPointerDereferenceMemberOperator extends MemberFunction, TaintFunc
   }
 }
 
-class IteratorCrementMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
+/**
+ * An `operator++` or `operator--` member function for an iterator type.
+ */
+class IteratorCrementMemberOperator extends MemberFunction, DataFlowFunction {
   IteratorCrementMemberOperator() {
     (
       this.hasName("operator++") or
       this.hasName("operator--")
     ) and
-    this.getDeclaringType() instanceof LegacyIterator
+    this.getDeclaringType() instanceof Iterator
   }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     input.isQualifierAddress() and
     output.isReturnValue()
   }
-
-  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isQualifierObject() and
-    output.isQualifierObject()
-  }
 }
 
+/**
+ * A member `operator->` function for an iterator type.
+ */
 class IteratorFieldMemberOperator extends Operator, TaintFunction {
   IteratorFieldMemberOperator() {
     this.hasName("operator->") and
-    this.getDeclaringType() instanceof LegacyIterator
+    this.getDeclaringType() instanceof Iterator
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -221,8 +254,8 @@ class IteratorFieldMemberOperator extends Operator, TaintFunction {
 class IteratorBinaryArithmeticMemberOperator extends MemberFunction, TaintFunction {
   IteratorBinaryArithmeticMemberOperator() {
     this.hasName("operator-") and
-    this.getDeclaringType() instanceof LegacyIterator and
-    this.getParameter(0).getUnspecifiedType() instanceof LegacyIterator
+    this.getDeclaringType() instanceof Iterator and
+    this.getParameter(0).getUnspecifiedType() instanceof Iterator
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -234,13 +267,16 @@ class IteratorBinaryArithmeticMemberOperator extends MemberFunction, TaintFuncti
   }
 }
 
+/**
+ * An `operator+=` or `operator-=` member function of an iterator class.
+ */
 class IteratorAssignArithmeticMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
   IteratorAssignArithmeticMemberOperator() {
     (
       this.hasName("operator+=") or
       this.hasName("operator-=")
     ) and
-    this.getDeclaringType() instanceof LegacyIterator
+    this.getDeclaringType() instanceof Iterator
   }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
@@ -254,10 +290,13 @@ class IteratorAssignArithmeticMemberOperator extends MemberFunction, DataFlowFun
   }
 }
 
+/**
+ * An `operator[]` member function of an iterator class.
+ */
 class IteratorArrayMemberOperator extends MemberFunction, TaintFunction {
   IteratorArrayMemberOperator() {
     this.hasName("operator[]") and
-    this.getDeclaringType() instanceof LegacyIterator
+    this.getDeclaringType() instanceof Iterator
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -267,4 +306,4 @@ class IteratorArrayMemberOperator extends MemberFunction, TaintFunction {
     ) and
     output.isReturnValue()
   }
-}
+}

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

@@ -67,7 +67,7 @@ class StdStringAppend extends TaintFunction {
   }
 
   int getAnIteratorParameter() {
-    getParameter(result).getType() instanceof LegacyIterator
+    getParameter(result).getType() instanceof Iterator
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {