Merge pull request github#11537 from MathiasVP/iterator-model-changes

C++: Change iterator models

Author: MathiasVP

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

@@ -31,7 +31,17 @@ private class IteratorTraits extends Class {
  * `std::iterator_traits` instantiation for it.
  */
 private class IteratorByTraits extends Iterator {
-  IteratorByTraits() { exists(IteratorTraits it | it.getIteratorType() = this) }
+  IteratorTraits trait;
+
+  IteratorByTraits() { trait.getIteratorType() = this }
+
+  override Type getValueType() {
+    exists(TypedefType t |
+      trait.getAMember() = t and
+      t.getName() = "value_type" and
+      result = t.getUnderlyingType()
+    )
+  }
 }
 
 /**
@@ -42,27 +52,36 @@ private class IteratorByTraits extends Iterator {
  */
 private class IteratorByPointer extends Iterator instanceof PointerType {
   IteratorByPointer() { not this instanceof IteratorByTraits }
+
+  override Type getValueType() { result = super.getBaseType() }
 }
 
 /**
  * A type which has the typedefs expected for an iterator.
  */
 private class IteratorByTypedefs extends Iterator, Class {
+  TypedefType valueType;
+
   IteratorByTypedefs() {
     this.getAMember().(TypedefType).hasName("difference_type") and
-    this.getAMember().(TypedefType).hasName("value_type") and
+    valueType = this.getAMember() and
+    valueType.hasName("value_type") and
     this.getAMember().(TypedefType).hasName("pointer") and
     this.getAMember().(TypedefType).hasName("reference") and
     this.getAMember().(TypedefType).hasName("iterator_category") and
     not this.hasQualifiedName(["std", "bsl"], "iterator_traits")
   }
+
+  override Type getValueType() { result = valueType.getUnderlyingType() }
 }
 
 /**
  * The `std::iterator` class.
  */
 private class StdIterator extends Iterator, Class {
   StdIterator() { this.hasQualifiedName(["std", "bsl"], "iterator") }
+
+  override Type getValueType() { result = this.getTemplateArgument(1).(Type).getUnderlyingType() }
 }
 
 /**
@@ -166,12 +185,15 @@ private class IteratorSubOperator extends Operator, TaintFunction {
 /**
  * A non-member `operator+=` or `operator-=` function for an iterator type.
  */
-private class IteratorAssignArithmeticOperator extends Operator, DataFlowFunction, TaintFunction {
+class IteratorAssignArithmeticOperator extends Operator {
   IteratorAssignArithmeticOperator() {
     this.hasName(["operator+=", "operator-="]) and
     exists(getIteratorArgumentInput(this, 0))
   }
+}
 
+private class IteratorAssignArithmeticOperatorModel extends IteratorAssignArithmeticOperator,
+  DataFlowFunction, TaintFunction {
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     input.isParameter(0) and
     output.isReturnValue()
@@ -210,11 +232,14 @@ class IteratorPointerDereferenceMemberOperator extends MemberFunction, TaintFunc
 /**
  * An `operator++` or `operator--` member function for an iterator type.
  */
-private class IteratorCrementMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
+class IteratorCrementMemberOperator extends MemberFunction {
   IteratorCrementMemberOperator() {
     this.getClassAndName(["operator++", "operator--"]) instanceof Iterator
   }
+}
 
+private class IteratorCrementMemberOperatorModel extends IteratorCrementMemberOperator,
+  DataFlowFunction, TaintFunction {
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     input.isQualifierAddress() and
     output.isReturnValue()

cpp/ql/lib/semmle/code/cpp/models/implementations/StdContainer.qll

@@ -5,38 +5,53 @@
 import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.Iterator
 
+/**
+ * A sequence container template class (for example, `std::vector`) from the
+ * standard library.
+ */
+abstract class StdSequenceContainer extends Class {
+  Type getElementType() { result = this.getTemplateArgument(0) }
+}
+
 /**
  * The `std::array` template class.
  */
-private class Array extends Class {
+private class Array extends StdSequenceContainer {
   Array() { this.hasQualifiedName(["std", "bsl"], "array") }
 }
 
+/**
+ * The `std::string` template class.
+ */
+private class String extends StdSequenceContainer {
+  String() { this.hasQualifiedName(["std", "bsl"], "basic_string") }
+}
+
 /**
  * The `std::deque` template class.
  */
-private class Deque extends Class {
+private class Deque extends StdSequenceContainer {
   Deque() { this.hasQualifiedName(["std", "bsl"], "deque") }
 }
 
 /**
  * The `std::forward_list` template class.
  */
-private class ForwardList extends Class {
+private class ForwardList extends StdSequenceContainer {
   ForwardList() { this.hasQualifiedName(["std", "bsl"], "forward_list") }
 }
 
 /**
  * The `std::list` template class.
  */
-private class List extends Class {
+private class List extends StdSequenceContainer {
   List() { this.hasQualifiedName(["std", "bsl"], "list") }
 }
 
 /**
  * The `std::vector` template class.
  */
-private class Vector extends Class {
+private class Vector extends StdSequenceContainer {
   Vector() { this.hasQualifiedName(["std", "bsl"], "vector") }
 }
 

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

@@ -15,15 +15,6 @@ private class StdBasicString extends ClassTemplateInstantiation {
   StdBasicString() { this.hasQualifiedName(["std", "bsl"], "basic_string") }
 }
 
-/**
- * The `std::basic_string::iterator` declaration.
- */
-private class StdBasicStringIterator extends Iterator, Type {
-  StdBasicStringIterator() {
-    this.getEnclosingElement() instanceof StdBasicString and this.hasName("iterator")
-  }
-}
-
 /**
  * A `std::string` function for which taint should be propagated.
  */

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

@@ -29,5 +29,17 @@ abstract class GetIteratorFunction extends Function {
 
 /**
  * A type which can be used as an iterator.
+ *
+ * Note: Do _not_ `extend` when inheriting from this class in queries. Always use `instanceof`:
+ * ```
+ * class MyIterator instanceof Iterator { ... }
+ * ```
  */
-abstract class Iterator extends Type { }
+abstract class Iterator extends Type {
+  /**
+   * Gets the value type of this iterator, if any.
+   *
+   * For example, the value type of a `std::vector<int>::iterator` is `int`.
+   */
+  Type getValueType() { none() }
+}