Merge branch 'main' into mathiasvp/replace-ast-with-ir-use-usedataflow

cpp/ql/lib/definitions.qll

@@ -12,8 +12,8 @@ import IDEContextual
  *
  * In some cases it is preferable to modify locations (the
  * `hasLocationInfo()` predicate) so that they are short, and
- * non-overlapping with other locations that might be highlighted in
- * the LGTM interface.
+ * non-overlapping with other locations that might be reported as
+ * code scanning alerts on GitHub.
  *
  * We need to give locations that may not be in the database, so
  * we use `hasLocationInfo()` rather than `getLocation()`.

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/semmle/code/cpp/dataflow/old/internal/DataFlowImpl.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/semmle/code/cpp/dataflow/old/internal/DataFlowImpl2.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/semmle/code/cpp/dataflow/old/internal/DataFlowImpl3.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/semmle/code/cpp/dataflow/old/internal/DataFlowImpl4.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/semmle/code/cpp/dataflow/old/internal/DataFlowImplLocal.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll

@@ -70,7 +70,7 @@ abstract class Configuration extends string {
   /**
    * Holds if `sink` is a relevant data flow sink accepting `state`.
    */
-  predicate isSink(Node source, FlowState state) { none() }
+  predicate isSink(Node sink, FlowState state) { none() }
 
   /**
    * Holds if data flow through `node` is prohibited. This completely removes

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,20 +52,27 @@ 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() }
 }
 
 /**
@@ -63,6 +80,8 @@ private class IteratorByTypedefs extends 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() }
+}

cpp/ql/src/Likely Bugs/RedundantNullCheckSimple.ql

@@ -12,8 +12,8 @@
  */
 
 /*
- * Note: this query is not assigned a precision yet because we don't want it on
- * LGTM until its performance is well understood.
+ * Note: this query is not assigned a precision yet because we don't want it
+ * to be included in query suites until its performance is well understood.
  */
 
 import cpp

cpp/ql/src/Metrics/Dependencies/ExternalDependencies.qll

@@ -52,7 +52,7 @@ class Library extends LibraryT {
     // The versions reported for C/C++ dependencies are just the versions that
     // happen to be installed on the system where the build takes place.
     // Reporting those versions is likely to cause misunderstandings, both for
-    // people reading them and for the vulnerability checker of lgtm.
+    // people reading them and for vulnerability checkers.
     result = "unknown"
   }