Merge itergator

CLAUDE.md

@@ -0,0 +1,75 @@
+# CodeQL Queries Repository
+
+## CI Checks
+
+CI runs two steps (see `.github/workflows/test.yml`):
+
+1. `make format-check` — verifies all `.ql` and `.qll` files are properly formatted
+2. `make test` — runs all CodeQL tests across all languages
+
+After editing any `.ql` or `.qll` file, run format check:
+```sh
+codeql query format --check-only <file>    # check only
+codeql query format --in-place <file>      # auto-fix
+```
+
+Or check/fix all files at once:
+```sh
+make format-check   # check all
+make format         # fix all
+```
+
+After significant changes (new queries, modified library logic, changed tests), run the full test suite:
+```sh
+make test
+```
+
+To run tests for a single query:
+```sh
+codeql test run cpp/test/query-tests/security/IteratorInvalidation/
+```
+
+## C/C++ Test Stubs
+
+Tests cannot use real system headers. Minimal stub headers live in `cpp/test/include/` organized by library:
+- `libc/` — C standard library stubs (signal.h, stdlib.h, string_stubs.h, etc.)
+- `stl/` — C++ STL stubs (vector.h, deque.h, unordered_set.h)
+- `openssl/` — OpenSSL stubs (evp.h, bn.h, rand.h, etc.)
+- `mbedtls/` — mbed TLS stubs (bignum.h)
+
+Stubs use a `USE_HEADERS` guard pattern to optionally fall back to real headers:
+```c
+#ifndef USE_HEADERS
+// ... stub definitions ...
+#else
+#include <real_header.h>
+#endif
+```
+
+Test `.cpp` files include stubs via relative paths:
+```cpp
+#include "../../../include/stl/vector.h"
+```
+
+Stubs only need enough declarations for CodeQL to resolve types and function names — no implementations required.
+
+## Updating README Query Tables
+
+When a query is added, removed, or its metadata changes, regenerate the README tables:
+```sh
+python ./scripts/queries_table_generator.py 2>/dev/null
+```
+
+This reads query metadata from all "full" suites and outputs markdown tables. Copy-paste the output into `README.md` under the `## Queries` section.
+
+## Qlpack Versioning
+
+Each language has a library pack (`<lang>/lib/qlpack.yml`) and a queries pack (`<lang>/src/qlpack.yml`) with a `version:` field. Test packs have no version.
+
+Bump versions when adding new queries or libraries, removing queries, or making breaking changes to library APIs. Keep the library and queries pack versions in sync for the same language.
+
+Packs per language:
+- `trailofbits/cpp-all` (library) — `cpp/lib/qlpack.yml`
+- `trailofbits/cpp-queries` — `cpp/src/qlpack.yml`
+- `trailofbits/go-queries` — `go/src/qlpack.yml`
+- `trailofbits/java-queries` — `java/src/qlpack.yml`

README.md

@@ -52,6 +52,7 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
 |[Find all problematic implicit casts](./cpp/src/docs/security/UnsafeImplicitConversions/UnsafeImplicitConversions.md)|Find all implicit casts that may be problematic. That is, casts that may result in unexpected truncation, reinterpretation or widening of values.|error|high|
 |[Inconsistent handling of return values from a specific function](./cpp/src/docs/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.md)|Detects functions whose return values are compared inconsistently across call sites, which may indicate bugs|warning|medium|
 |[Invalid string size passed to string manipulation function](./cpp/src/docs/security/CStrnFinder/CStrnFinder.md)|Finds calls to functions that take as input a string and its size as separate arguments (e.g., `strncmp`, `strncat`, ...) and the size argument is wrong|error|low|
+|[Iterator invalidation](./cpp/src/docs/security/IteratorInvalidation/IteratorInvalidation.md)|Modifying a container while iterating over it can invalidate iterators, leading to undefined behavior.|warning|medium|
 |[Missing null terminator](./cpp/src/docs/security/NoNullTerminator/NoNullTerminator.md)|This query finds incorrectly initialized strings that are passed to functions expecting null-byte-terminated strings|error|high|
 
 ### Go

cpp/lib/qlpack.yml

@@ -1,7 +1,7 @@
 ---
 name: trailofbits/cpp-all
 authors: Trail of Bits
-version: 0.2.1
+version: 0.3.0
 license: AGPL
 library: true
 extractor: cpp

cpp/lib/trailofbits/itergator/DataFlow.qll

@@ -0,0 +1,62 @@
+private import cpp
+private import trailofbits.itergator.Iterators
+import semmle.code.cpp.dataflow.new.DataFlow
+
+private module IteratorFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source.asExpr() instanceof Access or
+    exists(source.asParameter())
+  }
+
+  predicate isSink(DataFlow::Node sink) { sink.asExpr().(Access).getTarget() instanceof Iterator }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node.asExpr().(FunctionCall).getTarget() instanceof CopyConstructor
+  }
+}
+
+module IteratorFlow = DataFlow::Global<IteratorFlowConfig>;
+
+private module IteratedFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source.asExpr() instanceof Access or
+    exists(source.asParameter())
+  }
+
+  predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof Iterated }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node.asExpr().(FunctionCall).getTarget() instanceof CopyConstructor
+  }
+}
+
+module IteratedFlow = DataFlow::Global<IteratedFlowConfig>;
+
+private module InvalidationFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    exists(Access a | a = source.asExpr()) or
+    exists(source.asParameter())
+  }
+
+  predicate isSink(DataFlow::Node sink) { exists(Invalidation i | sink.asExpr() = i.getAChild()) }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node.asExpr().(FunctionCall).getTarget() instanceof CopyConstructor
+  }
+}
+
+module InvalidationFlow = DataFlow::Global<InvalidationFlowConfig>;
+
+private module InvalidatorFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { exists(source) }
+
+  predicate isSink(DataFlow::Node sink) {
+    sink.asExpr().getEnclosingElement() instanceof Invalidator
+  }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node.asExpr().(FunctionCall).getTarget() instanceof CopyConstructor
+  }
+}
+
+module InvalidatorFlow = DataFlow::Global<InvalidatorFlowConfig>;

cpp/lib/trailofbits/itergator/Invalidations.qll

@@ -0,0 +1,12 @@
+private import cpp
+private import trailofbits.itergator.Iterators
+
+abstract class PotentialInvalidation extends Function {
+  cached
+  abstract predicate invalidates(Iterated i);
+
+  Expr invalidatedChild(Invalidation invd) {
+    // by default, invalidates object method is called on
+    result = invd.getQualifier()
+  }
+}

cpp/lib/trailofbits/itergator/Iterators.qll

@@ -0,0 +1,111 @@
+private import cpp
+private import trailofbits.itergator.Invalidations
+
+class Iterator extends Variable {
+  Iterator() {
+    this.getUnderlyingType().getName().matches("%iterator%") or
+    // getType is inconsistent
+    this.getAnAssignedValue()
+        .(FunctionCall)
+        .getTarget()
+        .(MemberFunction)
+        .getName()
+        .regexpMatch("c?r?begin") or
+    this.getAnAssignedValue()
+        .(FunctionCall)
+        .getTarget()
+        .(MemberFunction)
+        .getName()
+        .regexpMatch("c?r?end") or
+    this.getAnAssignedValue().(FunctionCall).getTarget().hasName("find")
+  }
+}
+
+// the location where a variable is being iterated over
+class Iterated extends VariableAccess {
+  Iterator iterator;
+
+  Iterated() {
+    iterator.getAnAssignedValue().getChild(-1) = this and
+    not this.getTarget().isCompilerGenerated()
+    or
+    // show the iterable assigned to __range in ranged based for loops
+    iterator.getAnAssignedValue().getChild(-1).(VariableAccess).getTarget().isCompilerGenerated() and
+    this =
+      iterator.getAnAssignedValue().getChild(-1).(VariableAccess).getTarget().getAnAssignedValue()
+  }
+
+  Iterator iterator() { result = iterator }
+}
+
+// function call with utility predicates
+private class FunctionCallR extends FunctionCall {
+  predicate containedBy(Stmt other) {
+    other.getASuccessor*() = this and
+    other.getAChild*() = this
+    or
+    // for destructors
+    exists(Function f | f.getBlock() = other and this.getEnclosingFunction() = f)
+  }
+
+  predicate callsPotentialInvalidation() {
+    this.getTarget().(PotentialInvalidation).invalidates(any(Iterated i))
+  }
+
+  predicate callsPotentialInvalidation(Iterated i) {
+    this.getTarget().(PotentialInvalidation).invalidates(i)
+  }
+}
+
+// a call to any function that could call a PotentialInvalidation
+class InvalidatorT extends FunctionCallR {
+  InvalidatorT() {
+    this.callsPotentialInvalidation()
+    or
+    exists(InvalidatorT i | i.containedBy(this.getTarget().getBlock()))
+  }
+
+  InvalidatorT child() {
+    result = this.getTarget().getBlock().getAChild+().(InvalidatorT)
+    or
+    exists(DestructorCall d |
+      d.getEnclosingFunction() = this.getTarget() and d.(InvalidatorT) = result
+    )
+  }
+
+  Iterated iterated_() {
+    this.callsPotentialInvalidation(result) or
+    result = this.child().iterated_()
+  }
+
+  InvalidatorT potentialInvalidation() {
+    this.callsPotentialInvalidation(this.iterated_()) and result = this
+    or
+    result = this.child().potentialInvalidation()
+  }
+}
+
+// calls that actually perform the invalidation
+class Invalidation extends InvalidatorT {
+  Invalidator invalidator;
+
+  Invalidation() {
+    this = invalidator.potentialInvalidation() and invalidator.iterated() = this.iterated_()
+  }
+
+  Invalidator invalidator() { result = invalidator }
+}
+
+// the top level invalidation calls (directly inside loop bodies)
+class Invalidator extends InvalidatorT {
+  Iterated iterated;
+
+  Invalidator() {
+    iterated = this.iterated_() and
+    this.containedBy(iterated.iterator().getParentScope())
+  }
+
+  Iterated iterated() { result = iterated }
+
+  Invalidation invalidation() { result = any(Invalidation i | i.invalidator() = this) }
+}

cpp/lib/trailofbits/itergator/invalidations/Destructor.qll

@@ -0,0 +1,11 @@
+private import trailofbits.itergator.Iterators
+private import cpp
+import trailofbits.itergator.Invalidations
+
+class PotentialInvalidationDestructor extends PotentialInvalidation {
+  PotentialInvalidationDestructor() {
+    this instanceof MemberFunction and this.getName().matches("~%")
+  }
+
+  override predicate invalidates(Iterated i) { i.getType().refersTo(this.getParentScope()) }
+}

cpp/lib/trailofbits/itergator/invalidations/STL.qll

@@ -0,0 +1,64 @@
+private import trailofbits.itergator.Iterators
+private import cpp
+import trailofbits.itergator.Invalidations
+
+private string typeName(Iterated i) { result = i.getTarget().getType().stripType().getName() }
+
+class PotentialInvalidationSTL extends PotentialInvalidation {
+  PotentialInvalidationSTL() { this instanceof MemberFunction }
+
+  override Expr invalidatedChild(Invalidation invd) {
+    result = super.invalidatedChild(invd)
+    or
+    // swap can also invalidate the first argument
+    this.hasName("swap") and result = invd.getArgument(0)
+  }
+
+  override predicate invalidates(Iterated i) {
+    i.getType().refersTo(this.getParentScope()) and
+    (
+      typeName(i).matches("vector<%") and this.vectorInvalidation()
+      or
+      typeName(i).matches("deque<%") and this.dequeInvalidation()
+      or
+      typeName(i).regexpMatch("unordered_(set|multiset)<.*") and this.setInvalidation()
+    )
+  }
+
+  predicate vectorInvalidation() {
+    this.hasName("push_back") or
+    this.hasName("reserve") or
+    this.hasName("insert") or
+    this.hasName("emplace_back") or
+    this.hasName("emplace") or
+    this.hasName("erase") or
+    this.hasName("pop_back") or
+    this.hasName("resize") or
+    this.hasName("shrink_to_fit") or
+    this.hasName("clear") or
+    this.hasName("swap")
+  }
+
+  predicate dequeInvalidation() {
+    this.hasName("push_back") or
+    this.hasName("push_front") or
+    this.hasName("pop_back") or
+    this.hasName("pop_front") or
+    this.hasName("insert") or
+    this.hasName("erase") or
+    this.hasName("emplace") or
+    this.hasName("emplace_front") or
+    this.hasName("emplace_back") or
+    this.hasName("resize") or
+    this.hasName("clear") or
+    this.hasName("shrink_to_fit") or
+    this.hasName("swap")
+  }
+
+  predicate setInvalidation() {
+    this.hasName("emplace") or
+    this.hasName("emplace_hint") or
+    this.hasName("insert") or
+    this.hasName("clear")
+  }
+}