C++: input iterator models

2026-05-02 20:25:13 +02:00 · 2020-08-18 17:05:37 -07:00
parent d32d6c9d8d
commit 20188b7bc2
7 changed files with 142 additions and 67 deletions
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Iterator.qll
@@ -1,5 +1,6 @@
 import cpp
 import semmle.code.cpp.models.interfaces.Taint
+import semmle.code.cpp.models.interfaces.DataFlow

 class IteratorTraits extends Class {
  IteratorTraits() {
@@ -25,12 +26,17 @@ class IteratorByTypedefs extends Class {
  }
 }

-class IteratorByStdIteratorTraits extends Type { }
+class StdIterator extends Class {
+  StdIterator() {
+    this.hasQualifiedName("std", "iterator")
+  }
+}

 class LegacyIterator extends Type {
  LegacyIterator() {
    this instanceof IteratorByTypedefs or
-    exists(IteratorTraits it | it.getIteratorType() = this)
+    exists(IteratorTraits it | it.getIteratorType() = this) or
+    this instanceof StdIterator
  }
 }

@@ -47,7 +53,7 @@ class IteratorPointerDereferenceOperator extends Operator, TaintFunction {
  }
 }

-class IteratorCrementOperator extends Operator, TaintFunction {
+class IteratorCrementOperator extends Operator, DataFlowFunction, TaintFunction {
  IteratorCrementOperator() {
    (
      this.hasName("operator++") or
@@ -62,6 +68,11 @@ class IteratorCrementOperator extends Operator, TaintFunction {
        .getBaseType() instanceof LegacyIterator
  }

+  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)
@@ -75,7 +86,7 @@ class IteratorFieldOperator extends Operator, TaintFunction {
        .getACallToThisFunction()
        .getArgument(0)
        .getFullyConverted()
-        .getUnderlyingType()
+        .getUnspecifiedType()
        .(PointerType)
        .getBaseType() instanceof LegacyIterator
  }
@@ -88,24 +99,22 @@ class IteratorFieldOperator extends Operator, TaintFunction {

 class IteratorAddOperator extends Operator, TaintFunction {
  IteratorAddOperator() {
+    this.hasName("operator+") and
    (
-      this.hasName("operator+")
-    ) and
-    (
-    this
-        .getACallToThisFunction()
-        .getArgument(0)
-        .getFullyConverted()
-        .getUnderlyingType()
-        .(PointerType)
-        .getBaseType() instanceof LegacyIterator or
-    this
-        .getACallToThisFunction()
-        .getArgument(0)
-        .getFullyConverted()
-        .getUnderlyingType()
-        .(PointerType)
-        .getBaseType() instanceof LegacyIterator
+      this
+          .getACallToThisFunction()
+          .getArgument(0)
+          .getFullyConverted()
+          .getUnspecifiedType()
+          .(PointerType)
+          .getBaseType() instanceof LegacyIterator or
+      this
+          .getACallToThisFunction()
+          .getArgument(0)
+          .getFullyConverted()
+          .getUnspecifiedType()
+          .(PointerType)
+          .getBaseType() instanceof LegacyIterator
    )
  }

@@ -118,23 +127,17 @@ class IteratorAddOperator extends Operator, TaintFunction {
  }
 }

+/**
+ * A non-member `operator-` function that takes an iterator as its first argument.
+ */
 class IteratorSubOperator extends Operator, TaintFunction {
  IteratorSubOperator() {
-    (
-      this.hasName("operator-")
-    ) and
+    this.hasName("operator-") and
    this
        .getACallToThisFunction()
        .getArgument(0)
        .getFullyConverted()
-        .getUnderlyingType()
-        .(PointerType)
-        .getBaseType() instanceof LegacyIterator and
-    not this
-        .getACallToThisFunction()
-        .getArgument(1)
-        .getFullyConverted()
-        .getUnderlyingType()
+        .getUnspecifiedType()
        .(PointerType)
        .getBaseType() instanceof LegacyIterator
  }
@@ -148,35 +151,26 @@ class IteratorSubOperator extends Operator, TaintFunction {
  }
 }

-class IteratorDiffOperator extends Operator, TaintFunction {
-  IteratorDiffOperator() {
+class IteratorAssignArithmeticOperator extends MemberFunction, DataFlowFunction, TaintFunction {
+  IteratorAssignArithmeticOperator() {
    (
-      this.hasName("operator-")
+      this.hasName("operator+=") or
+      this.hasName("operator-=")
    ) and
-    this
-        .getACallToThisFunction()
-        .getArgument(0)
-        .getFullyConverted()
-        .getUnderlyingType()
-        .(PointerType)
-        .getBaseType() instanceof LegacyIterator and
-    not this
-        .getACallToThisFunction()
-        .getArgument(1)
-        .getFullyConverted()
-        .getUnderlyingType()
-        .(PointerType)
-        .getBaseType() instanceof LegacyIterator
+    this.getDeclaringType() instanceof LegacyIterator
+  }
+
+  override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
+    input.isParameter(0) and
+    output.isReturnValue()
  }

  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    (
-      input.isParameter(0) or
-      input.isParameter(1)
-    ) and
-    output.isReturnValue()
+    input.isParameterDeref(1) and
+    output.isParameterDeref(0)
  }
 }
+
 class IteratorPointerDereferenceMemberOperator extends MemberFunction, TaintFunction {
  IteratorPointerDereferenceMemberOperator() {
    this.hasName("operator*") and
@@ -189,7 +183,7 @@ class IteratorPointerDereferenceMemberOperator extends MemberFunction, TaintFunc
  }
 }

-class IteratorCrementMemberOperator extends MemberFunction, TaintFunction {
+class IteratorCrementMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
  IteratorCrementMemberOperator() {
    (
      this.hasName("operator++") or
@@ -198,6 +192,11 @@ class IteratorCrementMemberOperator extends MemberFunction, TaintFunction {
    this.getDeclaringType() instanceof LegacyIterator
  }

+  override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
+    input.isQualifierAddress() and
+    output.isReturnValue()
+  }
+
  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
    input.isQualifierObject() and
    output.isQualifierObject()
@@ -216,13 +215,14 @@ class IteratorFieldMemberOperator extends Operator, TaintFunction {
  }
 }

-class IteratorMemberBinaryOperator extends MemberFunction, TaintFunction {
-  IteratorMemberBinaryOperator() {
-    (
-      this.hasName("operator+") or
-      this.hasName("operator-")
-    ) and
-    this.getDeclaringType() instanceof LegacyIterator
+/**
+ * An `operator+` or `operator-` member function of an iterator class.
+ */
+class IteratorBinaryArithmeticMemberOperator extends MemberFunction, TaintFunction {
+  IteratorBinaryArithmeticMemberOperator() {
+    this.hasName("operator-") and
+    this.getDeclaringType() instanceof LegacyIterator and
+    this.getParameter(0).getUnspecifiedType() instanceof LegacyIterator
  }

  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -234,8 +234,8 @@ class IteratorMemberBinaryOperator extends MemberFunction, TaintFunction {
  }
 }

-class IteratorMemberAssignOperator extends MemberFunction, TaintFunction {
-  IteratorMemberAssignOperator() {
+class IteratorAssignArithmeticMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
+  IteratorAssignArithmeticMemberOperator() {
    (
      this.hasName("operator+=") or
      this.hasName("operator-=")
@@ -243,13 +243,28 @@ class IteratorMemberAssignOperator extends MemberFunction, TaintFunction {
    this.getDeclaringType() instanceof LegacyIterator
  }

+  override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
+    input.isQualifierAddress() and
+    output.isReturnValue()
+  }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    input.isParameter(0) and
+    output.isQualifierObject()
+  }
+}
+
+class IteratorArrayMemberOperator extends MemberFunction, TaintFunction {
+  IteratorArrayMemberOperator() {
+    this.hasName("operator[]") and
+    this.getDeclaringType() instanceof LegacyIterator
+  }
+
  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
    (
      input.isQualifierObject() or
      input.isParameter(0)
    ) and
-    output.isQualifierObject()
-    or
    output.isReturnValue()
  }
-}
+}
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll
@@ -1,4 +1,5 @@
 import semmle.code.cpp.models.interfaces.Taint
+import semmle.code.cpp.models.implementations.Iterator

 /**
 * The `std::basic_string` template class.
@@ -65,11 +66,17 @@ class StdStringAppend extends TaintFunction {
    getParameter(result).getType() = getDeclaringType().getTemplateArgument(0) // i.e. `std::basic_string::CharT`
  }

+  int getAnIteratorParameter() {
+    getParameter(result).getType() instanceof LegacyIterator
+  }
+
  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
    // flow from string and parameter to string (qualifier) and return value
    (
      input.isQualifierObject() or
      input.isParameterDeref(getAStringParameter())
+      or
+      input.isParameter(getAnIteratorParameter())
    ) and
    (
      output.isQualifierObject() or
@@ -104,6 +111,20 @@ class StdStringAssign extends TaintFunction {
  }
 }

+class StdStringBegin extends TaintFunction {
+  StdStringBegin() {
+    this.hasQualifiedName("std", "basic_string", "begin") or
+    this.hasQualifiedName("std", "basic_string", "cbegin") or
+    this.hasQualifiedName("std", "basic_string", "rbegin") or
+    this.hasQualifiedName("std", "basic_string", "crbegin")
+  }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    input.isQualifierObject() and
+    output.isReturnValue()
+  }
+}
+
 /**
 * The standard function `std::string.copy`.
 */
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
@@ -368,6 +368,8 @@
 | string.cpp:119:16:119:24 | call to basic_string | string.cpp:124:33:124:33 | s |  |
 | string.cpp:119:16:119:24 | call to basic_string | string.cpp:124:50:124:50 | s |  |
 | string.cpp:119:16:119:24 | call to basic_string | string.cpp:128:16:128:16 | s |  |
+| string.cpp:120:15:120:15 | (__begin) | string.cpp:120:15:120:15 | call to operator* | TAINT |
+| string.cpp:120:15:120:15 | (__range) | string.cpp:120:15:120:15 | call to begin | TAINT |
 | string.cpp:120:15:120:15 | call to begin | string.cpp:120:15:120:15 | (__begin) |  |
 | string.cpp:120:15:120:15 | call to begin | string.cpp:120:15:120:15 | (__begin) |  |
 | string.cpp:120:15:120:15 | call to begin | string.cpp:120:15:120:15 | (__begin) |  |
@@ -382,6 +384,7 @@
 | string.cpp:120:15:120:15 | s | string.cpp:120:15:120:15 | call to operator* | TAINT |
 | string.cpp:124:33:124:33 | ref arg s | string.cpp:124:50:124:50 | s |  |
 | string.cpp:124:33:124:33 | ref arg s | string.cpp:128:16:128:16 | s |  |
+| string.cpp:124:33:124:33 | s | string.cpp:124:35:124:39 | call to begin | TAINT |
 | string.cpp:124:35:124:39 | call to begin | string.cpp:124:44:124:45 | it |  |
 | string.cpp:124:35:124:39 | call to begin | string.cpp:124:61:124:62 | it |  |
 | string.cpp:124:35:124:39 | call to begin | string.cpp:125:9:125:10 | it |  |
@@ -390,6 +393,9 @@
 | string.cpp:124:61:124:62 | ref arg it | string.cpp:124:44:124:45 | it |  |
 | string.cpp:124:61:124:62 | ref arg it | string.cpp:124:61:124:62 | it |  |
 | string.cpp:124:61:124:62 | ref arg it | string.cpp:125:9:125:10 | it |  |
+| string.cpp:125:9:125:10 | it | string.cpp:125:8:125:8 | call to operator* | TAINT |
+| string.cpp:128:16:128:16 | (__begin) | string.cpp:128:16:128:16 | call to operator* | TAINT |
+| string.cpp:128:16:128:16 | (__range) | string.cpp:128:16:128:16 | call to begin | TAINT |
 | string.cpp:128:16:128:16 | call to begin | string.cpp:128:16:128:16 | (__begin) |  |
 | string.cpp:128:16:128:16 | call to begin | string.cpp:128:16:128:16 | (__begin) |  |
 | string.cpp:128:16:128:16 | call to begin | string.cpp:128:16:128:16 | (__begin) |  |
@@ -404,6 +410,8 @@
 | string.cpp:128:16:128:16 | s | string.cpp:128:16:128:16 | call to operator* | TAINT |
 | string.cpp:132:28:132:33 | call to source | string.cpp:132:28:132:36 | call to basic_string | TAINT |
 | string.cpp:132:28:132:36 | call to basic_string | string.cpp:133:22:133:28 | const_s |  |
+| string.cpp:133:22:133:22 | (__begin) | string.cpp:133:22:133:22 | call to operator* | TAINT |
+| string.cpp:133:22:133:22 | (__range) | string.cpp:133:22:133:22 | call to begin | TAINT |
 | string.cpp:133:22:133:22 | call to begin | string.cpp:133:22:133:22 | (__begin) |  |
 | string.cpp:133:22:133:22 | call to begin | string.cpp:133:22:133:22 | (__begin) |  |
 | string.cpp:133:22:133:22 | call to begin | string.cpp:133:22:133:22 | (__begin) |  |
@@ -677,6 +685,18 @@
 | string.cpp:319:16:319:24 | call to basic_string | string.cpp:322:19:322:19 | b |  |
 | string.cpp:321:7:321:7 | a | string.cpp:321:9:321:14 | call to substr | TAINT |
 | string.cpp:322:7:322:7 | b | string.cpp:322:9:322:14 | call to substr | TAINT |
+| string.cpp:327:18:327:24 | hello | string.cpp:327:18:327:25 | call to basic_string | TAINT |
+| string.cpp:327:18:327:25 | call to basic_string | string.cpp:329:8:329:9 | s1 |  |
+| string.cpp:327:18:327:25 | call to basic_string | string.cpp:330:8:330:9 | s1 |  |
+| string.cpp:327:18:327:25 | call to basic_string | string.cpp:331:8:331:9 | s1 |  |
+| string.cpp:328:18:328:23 | call to source | string.cpp:328:18:328:26 | call to basic_string | TAINT |
+| string.cpp:328:18:328:26 | call to basic_string | string.cpp:330:18:330:19 | s2 |  |
+| string.cpp:328:18:328:26 | call to basic_string | string.cpp:330:30:330:31 | s2 |  |
+| string.cpp:330:8:330:9 | s1 | string.cpp:330:11:330:16 | call to append | TAINT |
+| string.cpp:330:18:330:19 | ref arg s2 | string.cpp:330:30:330:31 | s2 |  |
+| string.cpp:330:18:330:19 | s2 | string.cpp:330:21:330:25 | call to begin | TAINT |
+| string.cpp:330:21:330:25 | call to begin | string.cpp:330:11:330:16 | call to append | TAINT |
+| string.cpp:330:33:330:35 | call to end | string.cpp:330:11:330:16 | call to append | TAINT |
 | stringstream.cpp:13:20:13:22 | call to basic_stringstream | stringstream.cpp:16:2:16:4 | ss1 |  |
 | stringstream.cpp:13:20:13:22 | call to basic_stringstream | stringstream.cpp:22:7:22:9 | ss1 |  |
 | stringstream.cpp:13:20:13:22 | call to basic_stringstream | stringstream.cpp:27:7:27:9 | ss1 |  |
@@ -1520,6 +1540,7 @@
 | vector.cpp:12:21:12:28 | call to vector | vector.cpp:18:38:18:38 | v |  |
 | vector.cpp:12:21:12:28 | call to vector | vector.cpp:18:55:18:55 | v |  |
 | vector.cpp:12:21:12:28 | call to vector | vector.cpp:22:15:22:15 | v |  |
+| vector.cpp:14:14:14:14 | (__begin) | vector.cpp:14:14:14:14 | call to operator* | TAINT |
 | vector.cpp:14:14:14:14 | call to begin | vector.cpp:14:14:14:14 | (__begin) |  |
 | vector.cpp:14:14:14:14 | call to begin | vector.cpp:14:14:14:14 | (__begin) |  |
 | vector.cpp:14:14:14:14 | call to begin | vector.cpp:14:14:14:14 | (__begin) |  |
@@ -1542,6 +1563,8 @@
 | vector.cpp:18:66:18:67 | ref arg it | vector.cpp:18:49:18:50 | it |  |
 | vector.cpp:18:66:18:67 | ref arg it | vector.cpp:18:66:18:67 | it |  |
 | vector.cpp:18:66:18:67 | ref arg it | vector.cpp:19:9:19:10 | it |  |
+| vector.cpp:19:9:19:10 | it | vector.cpp:19:8:19:8 | call to operator* | TAINT |
+| vector.cpp:22:15:22:15 | (__begin) | vector.cpp:22:15:22:15 | call to operator* | TAINT |
 | vector.cpp:22:15:22:15 | call to begin | vector.cpp:22:15:22:15 | (__begin) |  |
 | vector.cpp:22:15:22:15 | call to begin | vector.cpp:22:15:22:15 | (__begin) |  |
 | vector.cpp:22:15:22:15 | call to begin | vector.cpp:22:15:22:15 | (__begin) |  |
@@ -1556,6 +1579,7 @@
 | vector.cpp:22:15:22:15 | v | vector.cpp:22:15:22:15 | call to operator* | TAINT |
 | vector.cpp:26:33:26:39 | source1 | vector.cpp:26:33:26:40 | call to vector | TAINT |
 | vector.cpp:26:33:26:40 | call to vector | vector.cpp:27:21:27:27 | const_v |  |
+| vector.cpp:27:21:27:21 | (__begin) | vector.cpp:27:21:27:21 | call to operator* | TAINT |
 | vector.cpp:27:21:27:21 | call to begin | vector.cpp:27:21:27:21 | (__begin) |  |
 | vector.cpp:27:21:27:21 | call to begin | vector.cpp:27:21:27:21 | (__begin) |  |
 | vector.cpp:27:21:27:21 | call to begin | vector.cpp:27:21:27:21 | (__begin) |  |
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/stl.h
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/stl.h
@@ -63,6 +63,7 @@ namespace std
 		basic_string& append(const basic_string& str);
 		basic_string& append(const charT* s);
 		basic_string& append(size_type n, charT c);
+		template<class InputIt> constexpr basic_string& append(InputIt first, InputIt last);
 		basic_string& assign(const basic_string& str);
 		basic_string& assign(size_type n, charT c);
 		basic_string& insert(size_type pos, const basic_string& str);
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp
@@ -321,3 +321,13 @@ void test_string_substr()
 	sink(a.substr(0, a.length()));
 	sink(b.substr(0, b.length())); // tainted
 }
+
+void test_string_iterators() {
+	{
+		std::string s1("hello");
+		std::string s2(source());
+		sink(s1);
+		sink(s1.append(s2.begin(), s2.end()));
+		sink(s1);
+	}
+}
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
@@ -48,6 +48,7 @@
 | string.cpp:113:8:113:9 | s1 | string.cpp:109:32:109:37 | call to source |
 | string.cpp:114:8:114:9 | s2 | string.cpp:111:20:111:25 | call to source |
 | string.cpp:121:8:121:8 | c | string.cpp:119:16:119:21 | call to source |
+| string.cpp:125:8:125:8 | call to operator* | string.cpp:119:16:119:21 | call to source |
 | string.cpp:129:8:129:8 | c | string.cpp:119:16:119:21 | call to source |
 | string.cpp:134:8:134:8 | c | string.cpp:132:28:132:33 | call to source |
 | string.cpp:144:11:144:11 | call to operator+ | string.cpp:141:18:141:23 | call to source |
@@ -91,6 +92,7 @@
 | string.cpp:302:7:302:8 | s3 | string.cpp:290:17:290:22 | call to source |
 | string.cpp:311:9:311:12 | call to data | string.cpp:308:16:308:21 | call to source |
 | string.cpp:322:9:322:14 | call to substr | string.cpp:319:16:319:21 | call to source |
+| string.cpp:330:11:330:16 | call to append | string.cpp:328:18:328:23 | call to source |
 | structlikeclass.cpp:35:8:35:9 | s1 | structlikeclass.cpp:29:22:29:27 | call to source |
 | structlikeclass.cpp:36:8:36:9 | s2 | structlikeclass.cpp:30:24:30:29 | call to source |
 | structlikeclass.cpp:37:8:37:9 | s3 | structlikeclass.cpp:29:22:29:27 | call to source |
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
@@ -45,6 +45,7 @@
 | string.cpp:113:8:113:9 | string.cpp:109:32:109:37 | AST only |
 | string.cpp:114:8:114:9 | string.cpp:111:20:111:25 | AST only |
 | string.cpp:121:8:121:8 | string.cpp:119:16:119:21 | AST only |
+| string.cpp:125:8:125:8 | string.cpp:119:16:119:21 | AST only |
 | string.cpp:129:8:129:8 | string.cpp:119:16:119:21 | AST only |
 | string.cpp:134:8:134:8 | string.cpp:132:28:132:33 | AST only |
 | string.cpp:144:11:144:11 | string.cpp:141:18:141:23 | AST only |
@@ -88,6 +89,7 @@
 | string.cpp:302:7:302:8 | string.cpp:290:17:290:22 | AST only |
 | string.cpp:311:9:311:12 | string.cpp:308:16:308:21 | AST only |
 | string.cpp:322:9:322:14 | string.cpp:319:16:319:21 | AST only |
+| string.cpp:330:11:330:16 | string.cpp:328:18:328:23 | AST only |
 | structlikeclass.cpp:35:8:35:9 | structlikeclass.cpp:29:22:29:27 | AST only |
 | structlikeclass.cpp:36:8:36:9 | structlikeclass.cpp:30:24:30:29 | AST only |
 | structlikeclass.cpp:37:8:37:9 | structlikeclass.cpp:29:22:29:27 | AST only |