Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -

| Name | Description | Severity | Precision |
| --- | ----------- | :----: | :--------: |
|[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|
|[Crypto variable initialized using static key](./cpp/src/docs/crypto/StaticKeyFlow.md)|Finds crypto variables initialized using static keys|error|high|
|[Crypto variable initialized using static password](./cpp/src/docs/crypto/StaticPasswordFlow.md)|Finds crypto variables initialized using static passwords|error|high|
|[Crypto variable initialized using weak randomness](./cpp/src/docs/crypto/WeakRandomnessTaint.md)|Finds crypto variables initialized using weak randomness|error|high|
Expand All @@ -39,6 +40,7 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
|[Missing error handling](./cpp/src/docs/crypto/MissingErrorHandling.md)|Checks if returned error codes are properly checked|warning|high|
|[Missing zeroization of potentially sensitive random BIGNUM](./cpp/src/docs/crypto/MissingZeroization.md)|Determines if random bignums are properly zeroized|warning|medium|
|[Random buffer too small](./cpp/src/docs/crypto/RandomBufferTooSmall.md)|Finds buffer overflows in calls to CSPRNGs|warning|high|
|[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|
|[Use of legacy cryptographic algorithm](./cpp/src/docs/crypto/UseOfLegacyAlgorithm.md)|Detects potential instantiations of legacy cryptographic algorithms|warning|medium|

#### Security
Expand Down
42 changes: 42 additions & 0 deletions cpp/src/docs/crypto/BnCtxFreeBeforeEnd.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# BN_CTX_free called before BN_CTX_end

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.

In OpenSSL, the proper lifecycle for using `BN_CTX` with nested allocations is:

1. `BN_CTX_start(ctx)` - marks the start of a nested allocation
2. Use `BN_CTX_get(ctx)` to get temporary BIGNUMs
3. `BN_CTX_end(ctx)` - releases the temporary BIGNUMs allocated since the matching `BN_CTX_start`
4. `BN_CTX_free(ctx)` - frees the entire context

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.

The following example would be flagged as an issue by the query:

```cpp
void compute(BIGNUM *result) {
BN_CTX *ctx = BN_CTX_new();
BN_CTX_start(ctx);

BIGNUM *tmp = BN_CTX_get(ctx);
// Perform computation using tmp

// ERROR: BN_CTX_free called without calling BN_CTX_end first
BN_CTX_free(ctx);
}
```

The correct version should call `BN_CTX_end` before `BN_CTX_free`:

```cpp
void compute(BIGNUM *result) {
BN_CTX *ctx = BN_CTX_new();
BN_CTX_start(ctx);

BIGNUM *tmp = BN_CTX_get(ctx);
// Perform computation using tmp

BN_CTX_end(ctx); // Properly release temporary BIGNUMs
BN_CTX_free(ctx); // Now safe to free the context
}
```
53 changes: 53 additions & 0 deletions cpp/src/docs/crypto/UnbalancedBnCtx.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Unbalanced BN_CTX_start and BN_CTX_end pair

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.

`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.

Common issues include:

- Calling `BN_CTX_start` without a corresponding `BN_CTX_end` (memory leak of temporary allocations)
- Calling `BN_CTX_end` without a corresponding `BN_CTX_start` (undefined behavior)
- Missing `BN_CTX_end` in error paths

The following example would be flagged for missing `BN_CTX_end`:

```cpp
int compute(BN_CTX *ctx, BIGNUM *result) {
BN_CTX_start(ctx);

BIGNUM *tmp1 = BN_CTX_get(ctx);
BIGNUM *tmp2 = BN_CTX_get(ctx);

if (!tmp1 || !tmp2) {
// ERROR: Missing BN_CTX_end on error path
return 0;
}

// Perform computation

BN_CTX_end(ctx);
return 1;
}
```

The correct version ensures `BN_CTX_end` is called on all code paths:

```cpp
int compute(BN_CTX *ctx, BIGNUM *result) {
BN_CTX_start(ctx);

BIGNUM *tmp1 = BN_CTX_get(ctx);
BIGNUM *tmp2 = BN_CTX_get(ctx);

if (!tmp1 || !tmp2) {
BN_CTX_end(ctx); // Properly clean up on error path
return 0;
}

// Perform computation

BN_CTX_end(ctx);
return 1;
}
```