Skip to content

Commit 8b2adfc

Browse files
GrosQuilduGrosQuildu
authored
Merge itergator (#38)
* merge itergator * formatting * update readme, add claude * version bump
1 parent 1f36bc2 commit 8b2adfc

File tree

17 files changed

+800
-2
lines changed

17 files changed

+800
-2
lines changed

‎CLAUDE.md‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# CodeQL Queries Repository
2+
3+
## CI Checks
4+
5+
CI runs two steps (see `.github/workflows/test.yml`):
6+
7+
1. `make format-check` — verifies all `.ql` and `.qll` files are properly formatted
8+
2. `make test` — runs all CodeQL tests across all languages
9+
10+
After editing any `.ql` or `.qll` file, run format check:
11+
```sh
12+
codeql query format --check-only <file> # check only
13+
codeql query format --in-place <file> # auto-fix
14+
```
15+
16+
Or check/fix all files at once:
17+
```sh
18+
make format-check # check all
19+
make format # fix all
20+
```
21+
22+
After significant changes (new queries, modified library logic, changed tests), run the full test suite:
23+
```sh
24+
make test
25+
```
26+
27+
To run tests for a single query:
28+
```sh
29+
codeql test run cpp/test/query-tests/security/IteratorInvalidation/
30+
```
31+
32+
## C/C++ Test Stubs
33+
34+
Tests cannot use real system headers. Minimal stub headers live in `cpp/test/include/` organized by library:
35+
- `libc/` — C standard library stubs (signal.h, stdlib.h, string_stubs.h, etc.)
36+
- `stl/` — C++ STL stubs (vector.h, deque.h, unordered_set.h)
37+
- `openssl/` — OpenSSL stubs (evp.h, bn.h, rand.h, etc.)
38+
- `mbedtls/` — mbed TLS stubs (bignum.h)
39+
40+
Stubs use a `USE_HEADERS` guard pattern to optionally fall back to real headers:
41+
```c
42+
#ifndef USE_HEADERS
43+
// ... stub definitions ...
44+
#else
45+
#include <real_header.h>
46+
#endif
47+
```
48+
49+
Test `.cpp` files include stubs via relative paths:
50+
```cpp
51+
#include "../../../include/stl/vector.h"
52+
```
53+
54+
Stubs only need enough declarations for CodeQL to resolve types and function names — no implementations required.
55+
56+
## Updating README Query Tables
57+
58+
When a query is added, removed, or its metadata changes, regenerate the README tables:
59+
```sh
60+
python ./scripts/queries_table_generator.py 2>/dev/null
61+
```
62+
63+
This reads query metadata from all "full" suites and outputs markdown tables. Copy-paste the output into `README.md` under the `## Queries` section.
64+
65+
## Qlpack Versioning
66+
67+
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.
68+
69+
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.
70+
71+
Packs per language:
72+
- `trailofbits/cpp-all` (library) — `cpp/lib/qlpack.yml`
73+
- `trailofbits/cpp-queries` — `cpp/src/qlpack.yml`
74+
- `trailofbits/go-queries` — `go/src/qlpack.yml`
75+
- `trailofbits/java-queries` — `java/src/qlpack.yml`

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
5252
|[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|
5353
|[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|
5454
|[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|
55+
|[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|
5556
|[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|
5657

5758
### Go

‎cpp/lib/qlpack.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
name: trailofbits/cpp-all
33
authors: Trail of Bits
4-
version: 0.2.1
4+
version: 0.3.0
55
license: AGPL
66
library: true
77
extractor: cpp

‎cpp/lib/trailofbits/itergator/DataFlow.qll‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
private import cpp
2+
private import trailofbits.itergator.Iterators
3+
import semmle.code.cpp.dataflow.new.DataFlow
4+
5+
private module IteratorFlowConfig implements DataFlow::ConfigSig {
6+
predicate isSource(DataFlow::Node source) {
7+
source.asExpr() instanceof Access or
8+
exists(source.asParameter())
9+
}
10+
11+
predicate isSink(DataFlow::Node sink) { sink.asExpr().(Access).getTarget() instanceof Iterator }
12+
13+
predicate isBarrier(DataFlow::Node node) {
14+
node.asExpr().(FunctionCall).getTarget() instanceof CopyConstructor
15+
}
16+
}
17+
18+
module IteratorFlow = DataFlow::Global<IteratorFlowConfig>;
19+
20+
private module IteratedFlowConfig implements DataFlow::ConfigSig {
21+
predicate isSource(DataFlow::Node source) {
22+
source.asExpr() instanceof Access or
23+
exists(source.asParameter())
24+
}
25+
26+
predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof Iterated }
27+
28+
predicate isBarrier(DataFlow::Node node) {
29+
node.asExpr().(FunctionCall).getTarget() instanceof CopyConstructor
30+
}
31+
}
32+
33+
module IteratedFlow = DataFlow::Global<IteratedFlowConfig>;
34+
35+
private module InvalidationFlowConfig implements DataFlow::ConfigSig {
36+
predicate isSource(DataFlow::Node source) {
37+
exists(Access a | a = source.asExpr()) or
38+
exists(source.asParameter())
39+
}
40+
41+
predicate isSink(DataFlow::Node sink) { exists(Invalidation i | sink.asExpr() = i.getAChild()) }
42+
43+
predicate isBarrier(DataFlow::Node node) {
44+
node.asExpr().(FunctionCall).getTarget() instanceof CopyConstructor
45+
}
46+
}
47+
48+
module InvalidationFlow = DataFlow::Global<InvalidationFlowConfig>;
49+
50+
private module InvalidatorFlowConfig implements DataFlow::ConfigSig {
51+
predicate isSource(DataFlow::Node source) { exists(source) }
52+
53+
predicate isSink(DataFlow::Node sink) {
54+
sink.asExpr().getEnclosingElement() instanceof Invalidator
55+
}
56+
57+
predicate isBarrier(DataFlow::Node node) {
58+
node.asExpr().(FunctionCall).getTarget() instanceof CopyConstructor
59+
}
60+
}
61+
62+
module InvalidatorFlow = DataFlow::Global<InvalidatorFlowConfig>;

‎cpp/lib/trailofbits/itergator/Invalidations.qll‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
private import cpp
2+
private import trailofbits.itergator.Iterators
3+
4+
abstract class PotentialInvalidation extends Function {
5+
cached
6+
abstract predicate invalidates(Iterated i);
7+
8+
Expr invalidatedChild(Invalidation invd) {
9+
// by default, invalidates object method is called on
10+
result = invd.getQualifier()
11+
}
12+
}

‎cpp/lib/trailofbits/itergator/Iterators.qll‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
private import cpp
2+
private import trailofbits.itergator.Invalidations
3+
4+
class Iterator extends Variable {
5+
Iterator() {
6+
this.getUnderlyingType().getName().matches("%iterator%") or
7+
// getType is inconsistent
8+
this.getAnAssignedValue()
9+
.(FunctionCall)
10+
.getTarget()
11+
.(MemberFunction)
12+
.getName()
13+
.regexpMatch("c?r?begin") or
14+
this.getAnAssignedValue()
15+
.(FunctionCall)
16+
.getTarget()
17+
.(MemberFunction)
18+
.getName()
19+
.regexpMatch("c?r?end") or
20+
this.getAnAssignedValue().(FunctionCall).getTarget().hasName("find")
21+
}
22+
}
23+
24+
// the location where a variable is being iterated over
25+
class Iterated extends VariableAccess {
26+
Iterator iterator;
27+
28+
Iterated() {
29+
iterator.getAnAssignedValue().getChild(-1) = this and
30+
not this.getTarget().isCompilerGenerated()
31+
or
32+
// show the iterable assigned to __range in ranged based for loops
33+
iterator.getAnAssignedValue().getChild(-1).(VariableAccess).getTarget().isCompilerGenerated() and
34+
this =
35+
iterator.getAnAssignedValue().getChild(-1).(VariableAccess).getTarget().getAnAssignedValue()
36+
}
37+
38+
Iterator iterator() { result = iterator }
39+
}
40+
41+
// function call with utility predicates
42+
private class FunctionCallR extends FunctionCall {
43+
predicate containedBy(Stmt other) {
44+
other.getASuccessor*() = this and
45+
other.getAChild*() = this
46+
or
47+
// for destructors
48+
exists(Function f | f.getBlock() = other and this.getEnclosingFunction() = f)
49+
}
50+
51+
predicate callsPotentialInvalidation() {
52+
this.getTarget().(PotentialInvalidation).invalidates(any(Iterated i))
53+
}
54+
55+
predicate callsPotentialInvalidation(Iterated i) {
56+
this.getTarget().(PotentialInvalidation).invalidates(i)
57+
}
58+
}
59+
60+
// a call to any function that could call a PotentialInvalidation
61+
class InvalidatorT extends FunctionCallR {
62+
InvalidatorT() {
63+
this.callsPotentialInvalidation()
64+
or
65+
exists(InvalidatorT i | i.containedBy(this.getTarget().getBlock()))
66+
}
67+
68+
InvalidatorT child() {
69+
result = this.getTarget().getBlock().getAChild+().(InvalidatorT)
70+
or
71+
exists(DestructorCall d |
72+
d.getEnclosingFunction() = this.getTarget() and d.(InvalidatorT) = result
73+
)
74+
}
75+
76+
Iterated iterated_() {
77+
this.callsPotentialInvalidation(result) or
78+
result = this.child().iterated_()
79+
}
80+
81+
InvalidatorT potentialInvalidation() {
82+
this.callsPotentialInvalidation(this.iterated_()) and result = this
83+
or
84+
result = this.child().potentialInvalidation()
85+
}
86+
}
87+
88+
// calls that actually perform the invalidation
89+
class Invalidation extends InvalidatorT {
90+
Invalidator invalidator;
91+
92+
Invalidation() {
93+
this = invalidator.potentialInvalidation() and invalidator.iterated() = this.iterated_()
94+
}
95+
96+
Invalidator invalidator() { result = invalidator }
97+
}
98+
99+
// the top level invalidation calls (directly inside loop bodies)
100+
class Invalidator extends InvalidatorT {
101+
Iterated iterated;
102+
103+
Invalidator() {
104+
iterated = this.iterated_() and
105+
this.containedBy(iterated.iterator().getParentScope())
106+
}
107+
108+
Iterated iterated() { result = iterated }
109+
110+
Invalidation invalidation() { result = any(Invalidation i | i.invalidator() = this) }
111+
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
private import trailofbits.itergator.Iterators
2+
private import cpp
3+
import trailofbits.itergator.Invalidations
4+
5+
class PotentialInvalidationDestructor extends PotentialInvalidation {
6+
PotentialInvalidationDestructor() {
7+
this instanceof MemberFunction and this.getName().matches("~%")
8+
}
9+
10+
override predicate invalidates(Iterated i) { i.getType().refersTo(this.getParentScope()) }
11+
}

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
private import trailofbits.itergator.Iterators
2+
private import cpp
3+
import trailofbits.itergator.Invalidations
4+
5+
private string typeName(Iterated i) { result = i.getTarget().getType().stripType().getName() }
6+
7+
class PotentialInvalidationSTL extends PotentialInvalidation {
8+
PotentialInvalidationSTL() { this instanceof MemberFunction }
9+
10+
override Expr invalidatedChild(Invalidation invd) {
11+
result = super.invalidatedChild(invd)
12+
or
13+
// swap can also invalidate the first argument
14+
this.hasName("swap") and result = invd.getArgument(0)
15+
}
16+
17+
override predicate invalidates(Iterated i) {
18+
i.getType().refersTo(this.getParentScope()) and
19+
(
20+
typeName(i).matches("vector<%") and this.vectorInvalidation()
21+
or
22+
typeName(i).matches("deque<%") and this.dequeInvalidation()
23+
or
24+
typeName(i).regexpMatch("unordered_(set|multiset)<.*") and this.setInvalidation()
25+
)
26+
}
27+
28+
predicate vectorInvalidation() {
29+
this.hasName("push_back") or
30+
this.hasName("reserve") or
31+
this.hasName("insert") or
32+
this.hasName("emplace_back") or
33+
this.hasName("emplace") or
34+
this.hasName("erase") or
35+
this.hasName("pop_back") or
36+
this.hasName("resize") or
37+
this.hasName("shrink_to_fit") or
38+
this.hasName("clear") or
39+
this.hasName("swap")
40+
}
41+
42+
predicate dequeInvalidation() {
43+
this.hasName("push_back") or
44+
this.hasName("push_front") or
45+
this.hasName("pop_back") or
46+
this.hasName("pop_front") or
47+
this.hasName("insert") or
48+
this.hasName("erase") or
49+
this.hasName("emplace") or
50+
this.hasName("emplace_front") or
51+
this.hasName("emplace_back") or
52+
this.hasName("resize") or
53+
this.hasName("clear") or
54+
this.hasName("shrink_to_fit") or
55+
this.hasName("swap")
56+
}
57+
58+
predicate setInvalidation() {
59+
this.hasName("emplace") or
60+
this.hasName("emplace_hint") or
61+
this.hasName("insert") or
62+
this.hasName("clear")
63+
}
64+
}

0 commit comments

Comments
 (0)