From 6209ec9a5f7c1d5041d3da976932b0f94ac0c271 Mon Sep 17 00:00:00 2001 From: alan <17283637+ex0dus-0x@users.noreply.github.com> Date: Wed, 14 Jan 2026 16:23:57 -0500 Subject: [PATCH] Update documentation with new C++ crypto queries --- README.md | 2 + cpp/src/docs/crypto/BnCtxFreeBeforeEnd.md | 42 ++++++++++++++++++ cpp/src/docs/crypto/UnbalancedBnCtx.md | 53 +++++++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 cpp/src/docs/crypto/BnCtxFreeBeforeEnd.md create mode 100644 cpp/src/docs/crypto/UnbalancedBnCtx.md diff --git a/README.md b/README.md index 660a339..5ddebd1 100644 --- a/README.md +++ b/README.md @@ -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| @@ -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 diff --git a/cpp/src/docs/crypto/BnCtxFreeBeforeEnd.md b/cpp/src/docs/crypto/BnCtxFreeBeforeEnd.md new file mode 100644 index 0000000..a1a8256 --- /dev/null +++ b/cpp/src/docs/crypto/BnCtxFreeBeforeEnd.md @@ -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 +} +``` diff --git a/cpp/src/docs/crypto/UnbalancedBnCtx.md b/cpp/src/docs/crypto/UnbalancedBnCtx.md new file mode 100644 index 0000000..6ec9659 --- /dev/null +++ b/cpp/src/docs/crypto/UnbalancedBnCtx.md @@ -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; +} +```