Merge remote-tracking branch 'upstream/main' into docsforautofix

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,15 @@
+## 1.2.0
+
+### New Features
+
+* The syntax for models-as-data rows has been extended to make it easier to select sources, sinks, and summaries that involve templated functions and classes. Additionally, the syntax has also been extended to make it easier to specify models with arbitrary levels of indirection. See `dataflow/ExternalFlow.qll` for the updated documentation and specification for the model format.
+* It is now possible to extend the classes `AllocationFunction` and `DeallocationFunction` via data extensions. Extensions of these classes should be added to the `lib/ext/allocation` and `lib/ext/deallocation` directories respectively.
+
+### Minor Analysis Improvements
+
+* The queries "Potential double free" (`cpp/double-free`) and "Potential use after free" (`cpp/use-after-free`) now produce fewer false positives.
+* The "Guards" library (`semmle.code.cpp.controlflow.Guards`) now also infers guards from calls to the builtin operation `__builtin_expect`. As a result, some queries may produce fewer false positives.
+
 ## 1.1.1
 
 No user-facing changes.

cpp/ql/lib/change-notes/2024-06-10-builtin-expect.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The "Guards" library (`semmle.code.cpp.controlflow.Guards`) now also infers guards from calls to the builtin operation `__builtin_expect`. As a result, some queries may produce fewer false positives.

cpp/ql/lib/change-notes/2024-06-13-double-free.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The queries "Potential double free" (`cpp/double-free`) and "Potential use after free" (`cpp/use-after-free`) now produce fewer false positives.

cpp/ql/lib/change-notes/2024-06-20-extensible-allocation-deallocation.md

@@ -1,4 +0,0 @@
----
-category: feature
----
-* It is now possible to extend the classes `AllocationFunction` and `DeallocationFunction` via data extensions. Extensions of these classes should be added to the `lib/ext/allocation` and `lib/ext/deallocation` directories respectively.

cpp/ql/lib/change-notes/2024-07-03-extended-mad-syntax.md

@@ -1,4 +0,0 @@
----
-category: feature
----
-* The syntax for models-as-data rows has been extended to make it easier to select sources, sinks, and summaries that involve templated functions and classes. Additionally, the syntax has also been extended to make it easier to specify models with arbitrary levels of indirection. See `dataflow/ExternalFlow.qll` for the updated documentation and specification for the model format.

cpp/ql/lib/change-notes/released/1.2.0.md

@@ -0,0 +1,11 @@
+## 1.2.0
+
+### New Features
+
+* The syntax for models-as-data rows has been extended to make it easier to select sources, sinks, and summaries that involve templated functions and classes. Additionally, the syntax has also been extended to make it easier to specify models with arbitrary levels of indirection. See `dataflow/ExternalFlow.qll` for the updated documentation and specification for the model format.
+* It is now possible to extend the classes `AllocationFunction` and `DeallocationFunction` via data extensions. Extensions of these classes should be added to the `lib/ext/allocation` and `lib/ext/deallocation` directories respectively.
+
+### Minor Analysis Improvements
+
+* The queries "Potential double free" (`cpp/double-free`) and "Potential use after free" (`cpp/use-after-free`) now produce fewer false positives.
+* The "Guards" library (`semmle.code.cpp.controlflow.Guards`) now also infers guards from calls to the builtin operation `__builtin_expect`. As a result, some queries may produce fewer false positives.

cpp/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.1.1
+lastReleaseVersion: 1.2.0

cpp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-all
-version: 1.1.2-dev
+version: 1.2.1-dev
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp

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

@@ -104,7 +104,7 @@ predicate hasRawIndirectInstruction(Instruction instr, int indirectionIndex) {
 
 cached
 private newtype TDefImpl =
-  TDefAddressImpl(BaseIRVariable v) or
+  TDefAddressImpl(BaseSourceVariable v) or
   TDirectDefImpl(Operand address, int indirectionIndex) {
     isDef(_, _, address, _, _, indirectionIndex)
   } or
@@ -325,9 +325,9 @@ private Instruction getInitializationTargetAddress(IRVariable v) {
   )
 }
 
-/** An initial definition of an `IRVariable`'s address. */
-private class DefAddressImpl extends DefImpl, TDefAddressImpl {
-  BaseIRVariable v;
+/** An initial definition of an SSA variable address. */
+abstract private class DefAddressImpl extends DefImpl, TDefAddressImpl {
+  BaseSourceVariable v;
 
   DefAddressImpl() {
     this = TDefAddressImpl(v) and
@@ -342,6 +342,19 @@ private class DefAddressImpl extends DefImpl, TDefAddressImpl {
 
   final override Node0Impl getValue() { none() }
 
+  override Cpp::Location getLocation() { result = v.getLocation() }
+
+  final override SourceVariable getSourceVariable() {
+    result.getBaseVariable() = v and
+    result.getIndirection() = 0
+  }
+
+  final override BaseSourceVariable getBaseSourceVariable() { result = v }
+}
+
+private class DefVariableAddressImpl extends DefAddressImpl {
+  override BaseIRVariable v;
+
   final override predicate hasIndexInBlock(IRBlock block, int index) {
     exists(IRVariable var | var = v.getIRVariable() |
       block.getInstruction(index) = getInitializationTargetAddress(var)
@@ -353,15 +366,14 @@ private class DefAddressImpl extends DefImpl, TDefAddressImpl {
       index = 0
     )
   }
+}
 
-  override Cpp::Location getLocation() { result = v.getIRVariable().getLocation() }
+private class DefCallAddressImpl extends DefAddressImpl {
+  override BaseCallVariable v;
 
-  final override SourceVariable getSourceVariable() {
-    result.getBaseVariable() = v and
-    result.getIndirection() = 0
+  final override predicate hasIndexInBlock(IRBlock block, int index) {
+    block.getInstruction(index) = v.getCallInstruction()
   }
-
-  final override BaseSourceVariable getBaseSourceVariable() { result = v }
 }
 
 private class DirectDef extends DefImpl, TDirectDefImpl {

cpp/ql/src/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.0.3
+
+No user-facing changes.
+
 ## 1.0.2
 
 No user-facing changes.

cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToStrncat.ql

@@ -4,7 +4,7 @@
  * @kind problem
  * @problem.severity warning
  * @security-severity 9.3
- * @precision medium
+ * @precision high
  * @id cpp/unsafe-strncat
  * @tags reliability
  *       correctness

cpp/ql/src/change-notes/2024-07-08-unsafe-strncat-query.md

@@ -0,0 +1,4 @@
+---
+category: queryMetadata
+---
+* The precision of `cpp/unsafe-strncat` ("Potentially unsafe call to strncat") has been increased to `high`. As a result, it will be run by default as part of the Code Scanning suite.

cpp/ql/src/change-notes/released/1.0.3.md

@@ -0,0 +1,3 @@
+## 1.0.3
+
+No user-facing changes.

cpp/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.0.2
+lastReleaseVersion: 1.0.3

cpp/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-queries
-version: 1.0.3-dev
+version: 1.0.4-dev
 groups:
   - cpp
   - queries

cpp/ql/test/library-tests/dataflow/calls-as-ssa-variables/test.cpp

@@ -0,0 +1,29 @@
+namespace std
+{
+  struct ptrdiff_t;
+  struct input_iterator_tag
+  {
+  };
+  struct forward_iterator_tag : public input_iterator_tag
+  {
+  };
+}
+
+struct A
+{
+  using value_type = int;
+  using difference_type = std::ptrdiff_t;
+  using pointer = int*;
+  using reference = int&;
+  using iterator_category = std::forward_iterator_tag;
+};
+
+A get();
+
+void test()
+{
+  while (true)
+  {
+    auto &&x = get();
+  }
+}

cpp/ql/test/library-tests/dataflow/calls-as-ssa-variables/test.expected

@@ -0,0 +1,4 @@
+edges
+nodes
+subpaths
+#select

cpp/ql/test/library-tests/dataflow/calls-as-ssa-variables/test.ql

@@ -0,0 +1,23 @@
+/**
+ * @kind path-problem
+ */
+
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.dataflow.new.DataFlow
+import Flow::PathGraph
+
+module Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source.asInstruction().(VariableAddressInstruction).getIRVariable() instanceof IRTempVariable
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    sink.asInstruction().(CallInstruction).getStaticCallTarget().hasName("get")
+  }
+}
+
+module Flow = DataFlow::Global<Config>;
+
+from Flow::PathNode source, Flow::PathNode sink
+where Flow::flowPath(source, sink)
+select sink.getNode(), source, sink, ""

cpp/ql/test/query-tests/Likely Bugs/Memory Management/SuspiciousCallToStrncat/SuspiciousCallToStrncat.expected

@@ -3,3 +3,5 @@
 | test.c:67:3:67:9 | call to strncat | Potentially unsafe call to strncat. |
 | test.c:75:3:75:9 | call to strncat | Potentially unsafe call to strncat. |
 | test.c:76:3:76:9 | call to strncat | Potentially unsafe call to strncat. |
+| test.c:91:3:91:9 | call to strncat | Potentially unsafe call to strncat. |
+| test.c:99:3:99:9 | call to strncat | Potentially unsafe call to strncat. |

cpp/ql/test/query-tests/Likely Bugs/Memory Management/SuspiciousCallToStrncat/test.c

@@ -82,3 +82,20 @@ void strncat_test5(char *s) {
   strncat(buf, s, len - strlen(buf) - 1); // GOOD
   strncat(buf, s, len - strlen(buf)); // GOOD
 }
+
+void strncat_test6() {
+  {
+  char dest[60];
+  dest[0] = '\0';
+  // Will write `dest[0 .. 5]`
+  strncat(dest, "small", sizeof(dest)); // GOOD [FALSE POSITIVE]
+  }
+
+  {
+  char dest[60];
+  memset(dest, 'a', sizeof(dest));
+  dest[54] = '\0';
+  // Will write `dest[54 .. 59]`
+  strncat(dest, "small", sizeof(dest)); // GOOD [FALSE POSITIVE]
+  }
+}

cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/IteratorToExpiredContainer/IteratorToExpiredContainer.expected

@@ -3,3 +3,4 @@
 | test.cpp:702:27:702:27 | call to operator[] | This object is destroyed at the end of the full-expression. |
 | test.cpp:727:23:727:23 | call to operator[] | This object is destroyed at the end of the full-expression. |
 | test.cpp:735:23:735:23 | call to operator[] | This object is destroyed at the end of the full-expression. |
+| test.cpp:857:3:857:17 | pointer to ~PlusPlusReturnByValueIterator output argument | This object is destroyed at the end of the full-expression. |

cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/IteratorToExpiredContainer/test.cpp

@@ -801,4 +801,60 @@ void test5(int i)
     for(const auto& vs : vvs) { }
     ++i;
   } // GOOD
+}
+
+struct HasBeginAndEnd
+{
+  ~HasBeginAndEnd();
+  using value_type = int;
+  using difference_type = std::ptrdiff_t;
+  using pointer = int*;
+  using reference = int&;
+  using iterator_category = std::random_access_iterator_tag;
+  std::vector<int>::iterator begin() const;
+  std::vector<int>::iterator end() const;
+};
+
+HasBeginAndEnd getHasBeginAndEnd();
+
+bool getBool();
+
+void test6()
+{
+  while(getBool())
+  {
+    for (const int& x : getHasBeginAndEnd()) // GOOD
+    {
+    }
+  }
+}
+
+struct PlusPlusReturnByValueIterator
+{
+  using value_type = int;
+  using difference_type = std::ptrdiff_t;
+  using pointer = int *;
+  using reference = int &;
+  using iterator_category = std::forward_iterator_tag;
+
+  PlusPlusReturnByValueIterator();
+  PlusPlusReturnByValueIterator(PlusPlusReturnByValueIterator const &);
+
+  PlusPlusReturnByValueIterator operator++();
+  bool operator==(PlusPlusReturnByValueIterator other) const;
+  bool operator!=(PlusPlusReturnByValueIterator other) const;
+  reference operator*() const;
+  pointer operator->() const;
+
+  ~PlusPlusReturnByValueIterator();
+
+  PlusPlusReturnByValueIterator begin();
+};
+
+void test7()
+{
+  PlusPlusReturnByValueIterator it;
+  it.operator++(); // GOOD [FALSE POSITIVE]
+
+  it.begin();
 }