Skip to content

Commit 6209ec9

Browse files
committed
Update documentation with new C++ crypto queries
1 parent 25082ab commit 6209ec9

File tree

3 files changed

+97
-0
lines changed

3 files changed

+97
-0
lines changed

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
2929

3030
| Name | Description | Severity | Precision |
3131
| --- | ----------- | :----: | :--------: |
32+
|[BN_CTX_free called before BN_CTX_end](./cpp/src/docs/crypto/BnCtxFreeBeforeEnd.md)|Detects BN_CTX_free called before BN_CTX_end, which violates the required lifecycle|error|medium|
3233
|[Crypto variable initialized using static key](./cpp/src/docs/crypto/StaticKeyFlow.md)|Finds crypto variables initialized using static keys|error|high|
3334
|[Crypto variable initialized using static password](./cpp/src/docs/crypto/StaticPasswordFlow.md)|Finds crypto variables initialized using static passwords|error|high|
3435
|[Crypto variable initialized using weak randomness](./cpp/src/docs/crypto/WeakRandomnessTaint.md)|Finds crypto variables initialized using weak randomness|error|high|
@@ -39,6 +40,7 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
3940
|[Missing error handling](./cpp/src/docs/crypto/MissingErrorHandling.md)|Checks if returned error codes are properly checked|warning|high|
4041
|[Missing zeroization of potentially sensitive random BIGNUM](./cpp/src/docs/crypto/MissingZeroization.md)|Determines if random bignums are properly zeroized|warning|medium|
4142
|[Random buffer too small](./cpp/src/docs/crypto/RandomBufferTooSmall.md)|Finds buffer overflows in calls to CSPRNGs|warning|high|
43+
|[Unbalanced BN_CTX_start and BN_CTX_end pair](./cpp/src/docs/crypto/UnbalancedBnCtx.md)|Detects if one call in the BN_CTX_start/BN_CTX_end pair is missing|warning|medium|
4244
|[Use of legacy cryptographic algorithm](./cpp/src/docs/crypto/UseOfLegacyAlgorithm.md)|Detects potential instantiations of legacy cryptographic algorithms|warning|medium|
4345

4446
#### Security
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# BN_CTX_free called before BN_CTX_end
2+
3+
This query identifies instances where `BN_CTX_free` is called before `BN_CTX_end`, which violates the required lifecycle of OpenSSL's `BN_CTX` objects.
4+
5+
In OpenSSL, the proper lifecycle for using `BN_CTX` with nested allocations is:
6+
7+
1. `BN_CTX_start(ctx)` - marks the start of a nested allocation
8+
2. Use `BN_CTX_get(ctx)` to get temporary BIGNUMs
9+
3. `BN_CTX_end(ctx)` - releases the temporary BIGNUMs allocated since the matching `BN_CTX_start`
10+
4. `BN_CTX_free(ctx)` - frees the entire context
11+
12+
Calling `BN_CTX_free` before `BN_CTX_end` can lead to corrupted state or undefined behavior, as temporary BIGNUMs allocated via `BN_CTX_get` are not properly released.
13+
14+
The following example would be flagged as an issue by the query:
15+
16+
```cpp
17+
void compute(BIGNUM *result) {
18+
BN_CTX *ctx = BN_CTX_new();
19+
BN_CTX_start(ctx);
20+
21+
BIGNUM *tmp = BN_CTX_get(ctx);
22+
// Perform computation using tmp
23+
24+
// ERROR: BN_CTX_free called without calling BN_CTX_end first
25+
BN_CTX_free(ctx);
26+
}
27+
```
28+
29+
The correct version should call `BN_CTX_end` before `BN_CTX_free`:
30+
31+
```cpp
32+
void compute(BIGNUM *result) {
33+
BN_CTX *ctx = BN_CTX_new();
34+
BN_CTX_start(ctx);
35+
36+
BIGNUM *tmp = BN_CTX_get(ctx);
37+
// Perform computation using tmp
38+
39+
BN_CTX_end(ctx); // Properly release temporary BIGNUMs
40+
BN_CTX_free(ctx); // Now safe to free the context
41+
}
42+
```
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# Unbalanced BN_CTX_start and BN_CTX_end pair
2+
3+
This query detects unbalanced pairs of `BN_CTX_start` and `BN_CTX_end` calls in OpenSSL code. These functions must be used in matching pairs to properly manage temporary BIGNUM allocations within a `BN_CTX` context.
4+
5+
`BN_CTX_start` marks the beginning of a nested allocation scope, and `BN_CTX_end` releases all temporary BIGNUMs allocated via `BN_CTX_get` since the matching `BN_CTX_start`. Each call to `BN_CTX_start` must have a corresponding `BN_CTX_end` call, and vice versa.
6+
7+
Common issues include:
8+
9+
- Calling `BN_CTX_start` without a corresponding `BN_CTX_end` (memory leak of temporary allocations)
10+
- Calling `BN_CTX_end` without a corresponding `BN_CTX_start` (undefined behavior)
11+
- Missing `BN_CTX_end` in error paths
12+
13+
The following example would be flagged for missing `BN_CTX_end`:
14+
15+
```cpp
16+
int compute(BN_CTX *ctx, BIGNUM *result) {
17+
BN_CTX_start(ctx);
18+
19+
BIGNUM *tmp1 = BN_CTX_get(ctx);
20+
BIGNUM *tmp2 = BN_CTX_get(ctx);
21+
22+
if (!tmp1 || !tmp2) {
23+
// ERROR: Missing BN_CTX_end on error path
24+
return 0;
25+
}
26+
27+
// Perform computation
28+
29+
BN_CTX_end(ctx);
30+
return 1;
31+
}
32+
```
33+
34+
The correct version ensures `BN_CTX_end` is called on all code paths:
35+
36+
```cpp
37+
int compute(BN_CTX *ctx, BIGNUM *result) {
38+
BN_CTX_start(ctx);
39+
40+
BIGNUM *tmp1 = BN_CTX_get(ctx);
41+
BIGNUM *tmp2 = BN_CTX_get(ctx);
42+
43+
if (!tmp1 || !tmp2) {
44+
BN_CTX_end(ctx); // Properly clean up on error path
45+
return 0;
46+
}
47+
48+
// Perform computation
49+
50+
BN_CTX_end(ctx);
51+
return 1;
52+
}
53+
```

0 commit comments

Comments
 (0)