From b463056002c168fb1586a24216b6fba68b54b059 Mon Sep 17 00:00:00 2001 From: alan <17283637+ex0dus-0x@users.noreply.github.com> Date: Mon, 15 Dec 2025 17:02:49 -0500 Subject: [PATCH 1/2] Fix TODOs for openssl queries, add additional support for BN_CTX* models and querying for unbalanced BN_CTX usage --- cpp/lib/trailofbits/crypto/openssl/bn.qll | 108 ++++++++++++++++++---- cpp/src/crypto/BnCtxFreeBeforeEnd.ql | 55 +++++++++++ cpp/src/crypto/MissingBnCtxEnd.ql | 29 ++++++ cpp/src/crypto/MissingZeroization.ql | 8 +- 4 files changed, 177 insertions(+), 23 deletions(-) create mode 100644 cpp/src/crypto/BnCtxFreeBeforeEnd.ql create mode 100644 cpp/src/crypto/MissingBnCtxEnd.ql diff --git a/cpp/lib/trailofbits/crypto/openssl/bn.qll b/cpp/lib/trailofbits/crypto/openssl/bn.qll index 4fafca5..f890786 100644 --- a/cpp/lib/trailofbits/crypto/openssl/bn.qll +++ b/cpp/lib/trailofbits/crypto/openssl/bn.qll @@ -4,7 +4,8 @@ import trailofbits.crypto.common // BIGNUM *BN_new(void); class BN_new extends CustomAllocator { BN_new() { - this.getQualifiedName() = "BN_new" and ( + this.getQualifiedName() = "BN_new" and + ( dealloc instanceof BN_free or dealloc instanceof BN_clear_free ) @@ -14,7 +15,8 @@ class BN_new extends CustomAllocator { // BIGNUM *BN_secure_new(void); class BN_secure_new extends CustomAllocator { BN_secure_new() { - this.getQualifiedName() = "BN_secure_new" and ( + this.getQualifiedName() = "BN_secure_new" and + ( dealloc instanceof BN_free or dealloc instanceof BN_clear_free ) @@ -23,24 +25,16 @@ class BN_secure_new extends CustomAllocator { // void BN_free(BIGNUM *a); class BN_free extends CustomDeallocator { - BN_free() { - this.getQualifiedName() = "BN_free" - } + BN_free() { this.getQualifiedName() = "BN_free" } - override int getPointer() { - result = 0 - } + override int getPointer() { result = 0 } } // void BN_clear_free(BIGNUM *a); class BN_clear_free extends CustomDeallocator { - BN_clear_free() { - this.getQualifiedName() = "BN_clear_free" - } + BN_clear_free() { this.getQualifiedName() = "BN_clear_free" } - override int getPointer() { - result = 0 - } + override int getPointer() { result = 0 } } // void BN_clear(BIGNUM *a); @@ -50,18 +44,92 @@ class BN_clear extends FunctionCall { Expr getBignum() { result = this.getArgument(0) } } -// int BN_rand(BIGNUM *rnd, int bits, int top, int bottom); +// int BN_rand(BIGNUM *rnd, int bits, int top, int bottom); (and variants) +/// Reference: https://docs.openssl.org/master/man3/BN_rand/#synopsis class BN_rand extends FunctionCall { - BN_rand() { this.getTarget().getName() = "BN_rand" } - - Expr getBignum() { - result = this.getArgument(0) + BN_rand() { + this.getTarget().getName().matches("BN\\_rand%") or + this.getTarget().getName().matches("BN\\_priv\\_rand%") or + this.getTarget().getName().matches("BN\\_pseudo\\_rand%") } + + Expr getBignum() { result = this.getArgument(0) } } class BIGNUM extends FunctionCall { - BIGNUM () { + BIGNUM() { this.getTarget() instanceof BN_new or this.getTarget() instanceof BN_secure_new } } + +// BN_CTX *BN_CTX_new(void); +class BN_CTX_new extends CustomAllocator { + BN_CTX_new() { + this.getName() = "BN_CTX_new" and + dealloc instanceof BN_CTX_free + } +} + +// BN_CTX *BN_CTX_secure_new(void); +class BN_CTX_secure_new extends CustomAllocator { + BN_CTX_secure_new() { + this.getName() = "BN_CTX_secure_new" and + dealloc instanceof BN_CTX_free + } +} + +// void BN_CTX_free(BN_CTX *c); +class BN_CTX_free extends CustomDeallocator { + BN_CTX_free() { this.getName() = "BN_CTX_free" } + + override int getPointer() { result = 0 } +} + +// void BN_CTX_start(BN_CTX *ctx); +class BN_CTX_start extends Expr { + BN_CTX_start() { + exists(FunctionCall fc | + fc = this and + fc.getTarget().getName() = "BN_CTX_start" + ) + } + + Expr getContext() { result = this.(FunctionCall).getArgument(0) } +} + +// void BN_CTX_end(BN_CTX *ctx); +class BN_CTX_end extends Expr { + BN_CTX_end() { + exists(FunctionCall fc | + fc = this and + fc.getTarget().getName() = "BN_CTX_end" + ) + } + + Expr getContext() { result = this.(FunctionCall).getArgument(0) } +} + +// BIGNUM *BN_CTX_get(BN_CTX *ctx); +class BN_CTX_get extends Expr { + BN_CTX_get() { + exists(FunctionCall fc | + fc = this and + fc.getTarget().getName() = "BN_CTX_get" + ) + } + + Expr getContext() { result = this.(FunctionCall).getArgument(0) } +} + +class BN_CTX extends Expr { + BN_CTX() { + exists(FunctionCall fc | + fc = this and + ( + fc.getTarget() instanceof BN_CTX_new or + fc.getTarget() instanceof BN_CTX_secure_new + ) + ) + } +} diff --git a/cpp/src/crypto/BnCtxFreeBeforeEnd.ql b/cpp/src/crypto/BnCtxFreeBeforeEnd.ql new file mode 100644 index 0000000..4be07c7 --- /dev/null +++ b/cpp/src/crypto/BnCtxFreeBeforeEnd.ql @@ -0,0 +1,55 @@ +/** + * @name BN_CTX_free called before BN_CTX_end + * @id tob/cpp/bn-ctx-free-before-end + * @description Detects BN_CTX_free called before BN_CTX_end, which violates the required lifecycle + * @kind path-problem + * @tags correctness crypto + * @problem.severity error + * @precision high + * @group cryptography + */ + +import cpp +import trailofbits.crypto.libraries +import semmle.code.cpp.dataflow.new.DataFlow +import semmle.code.cpp.controlflow.ControlFlowGraph +import DataFlow::PathGraph + +/** + * Tracks BN_CTX from BN_CTX_start to BN_CTX_free without going through BN_CTX_end + */ +module BnCtxFreeBeforeEndConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + exists(BN_CTX_start start | + source.asExpr() = start.getContext() + ) + } + + predicate isSink(DataFlow::Node sink) { + exists(CustomDeallocatorCall free | + free.getTarget() instanceof BN_CTX_free and + sink.asExpr() = free.getPointer() + ) + } + + predicate isBarrier(DataFlow::Node node) { + // If the context flows through BN_CTX_end, it's properly handled + exists(BN_CTX_end end | + node.asExpr() = end.getContext() + ) + } +} + +module BnCtxFreeBeforeEndFlow = DataFlow::Global; + +from BnCtxFreeBeforeEndFlow::PathNode source, BnCtxFreeBeforeEndFlow::PathNode sink, + BN_CTX_start start, CustomDeallocatorCall free +where + BnCtxFreeBeforeEndFlow::flowPath(source, sink) and + source.getNode().asExpr() = start.getContext() and + sink.getNode().asExpr() = free.getPointer() and + free.getTarget() instanceof BN_CTX_free +select free, source, sink, + "BN_CTX_free called at line " + free.getLocation().getStartLine().toString() + + " before BN_CTX_end after BN_CTX_start at line " + + start.getLocation().getStartLine().toString() diff --git a/cpp/src/crypto/MissingBnCtxEnd.ql b/cpp/src/crypto/MissingBnCtxEnd.ql new file mode 100644 index 0000000..f350f01 --- /dev/null +++ b/cpp/src/crypto/MissingBnCtxEnd.ql @@ -0,0 +1,29 @@ +/** + * @name Missing BN_CTX_end after BN_CTX_start + * @id tob/cpp/missing-bn-ctx-end + * @description Detects BN_CTX_start calls without corresponding BN_CTX_end calls + * @kind problem + * @tags correctness crypto + * @problem.severity warning + * @precision medium + * @group cryptography + */ + +import cpp +import trailofbits.crypto.libraries +import semmle.code.cpp.dataflow.new.DataFlow +import semmle.code.cpp.controlflow.Guards + +predicate hasMatchingEnd(BN_CTX_start start) { + exists(BN_CTX_end end, Function f | + start.getEnclosingFunction() = f and + end.getEnclosingFunction() = f and + DataFlow::localFlow(DataFlow::exprNode(start.getContext()), + DataFlow::exprNode(end.getContext())) + ) +} + +from BN_CTX_start start +where not hasMatchingEnd(start) +select start.getLocation(), + "BN_CTX_start called without corresponding BN_CTX_end" diff --git a/cpp/src/crypto/MissingZeroization.ql b/cpp/src/crypto/MissingZeroization.ql index a2b0513..46dad32 100644 --- a/cpp/src/crypto/MissingZeroization.ql +++ b/cpp/src/crypto/MissingZeroization.ql @@ -13,14 +13,15 @@ import cpp import trailofbits.crypto.libraries import semmle.code.cpp.dataflow.new.DataFlow -// TODO: Handle `BN_clear_free` as well. predicate isCleared(Expr bignum) { exists(BN_clear clear | DataFlow::localFlow(DataFlow::exprNode(bignum), DataFlow::exprNode(clear.getArgument(0))) + ) or + exists(BN_clear_free clear_free | + DataFlow::localFlow(DataFlow::exprNode(bignum), DataFlow::exprNode(clear_free.getArgument(0))) ) } -// TODO: Add support for remaining OpenSSL PRNG functions. predicate isRandom(Expr bignum) { exists(BN_rand rand | DataFlow::localFlow(DataFlow::exprNode(bignum), DataFlow::exprNode(rand.getArgument(0))) @@ -29,4 +30,5 @@ predicate isRandom(Expr bignum) { from BIGNUM bignum where isRandom(bignum) and not isCleared(bignum) -select bignum.getLocation(), "Bignum is initialized with random data but is not zeroized before it goes out of scope" +select bignum.getLocation(), + "Bignum is initialized with random data but is not zeroized before it goes out of scope" From d9193104bbd56e4af63a93b42cf52a03865cd260 Mon Sep 17 00:00:00 2001 From: alan <17283637+ex0dus-0x@users.noreply.github.com> Date: Tue, 16 Dec 2025 17:12:40 -0500 Subject: [PATCH 2/2] Fix up queries and add tests --- cpp/lib/trailofbits/crypto/openssl/bn.qll | 59 +++++------------- cpp/src/crypto/BnCtxFreeBeforeEnd.ql | 21 +++---- cpp/src/crypto/MissingBnCtxEnd.ql | 29 --------- cpp/src/crypto/MissingZeroization.ql | 12 ++-- cpp/src/crypto/UnbalancedBnCtx.ql | 45 ++++++++++++++ cpp/test/include/openssl/bn.h | 26 ++++++++ .../BnCtxFreeBeforeEnd.expected | 12 ++++ .../BnCtxFreeBeforeEnd.qlref | 1 + .../crypto/BnCtxFreeBeforeEnd/test.c | 43 +++++++++++++ .../UnbalancedBnCtx/UnbalancedBnCtx.expected | 2 + .../UnbalancedBnCtx/UnbalancedBnCtx.qlref | 1 + .../query-tests/crypto/UnbalancedBnCtx/test.c | 62 +++++++++++++++++++ 12 files changed, 223 insertions(+), 90 deletions(-) delete mode 100644 cpp/src/crypto/MissingBnCtxEnd.ql create mode 100644 cpp/src/crypto/UnbalancedBnCtx.ql create mode 100644 cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/BnCtxFreeBeforeEnd.expected create mode 100644 cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/BnCtxFreeBeforeEnd.qlref create mode 100644 cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/test.c create mode 100644 cpp/test/query-tests/crypto/UnbalancedBnCtx/UnbalancedBnCtx.expected create mode 100644 cpp/test/query-tests/crypto/UnbalancedBnCtx/UnbalancedBnCtx.qlref create mode 100644 cpp/test/query-tests/crypto/UnbalancedBnCtx/test.c diff --git a/cpp/lib/trailofbits/crypto/openssl/bn.qll b/cpp/lib/trailofbits/crypto/openssl/bn.qll index f890786..9eaaa3a 100644 --- a/cpp/lib/trailofbits/crypto/openssl/bn.qll +++ b/cpp/lib/trailofbits/crypto/openssl/bn.qll @@ -66,70 +66,41 @@ class BIGNUM extends FunctionCall { // BN_CTX *BN_CTX_new(void); class BN_CTX_new extends CustomAllocator { BN_CTX_new() { - this.getName() = "BN_CTX_new" and - dealloc instanceof BN_CTX_free - } -} - -// BN_CTX *BN_CTX_secure_new(void); -class BN_CTX_secure_new extends CustomAllocator { - BN_CTX_secure_new() { - this.getName() = "BN_CTX_secure_new" and + this.getQualifiedName().matches("BN\\_CTX_new%") + or + this.getQualifiedName().matches("BN\\_CTX\\_secure\\_new%") and dealloc instanceof BN_CTX_free } } // void BN_CTX_free(BN_CTX *c); class BN_CTX_free extends CustomDeallocator { - BN_CTX_free() { this.getName() = "BN_CTX_free" } + BN_CTX_free() { this.getQualifiedName() = "BN_CTX_free" } override int getPointer() { result = 0 } } // void BN_CTX_start(BN_CTX *ctx); -class BN_CTX_start extends Expr { - BN_CTX_start() { - exists(FunctionCall fc | - fc = this and - fc.getTarget().getName() = "BN_CTX_start" - ) - } +class BN_CTX_start extends FunctionCall { + BN_CTX_start() { this.getTarget().getName() = "BN_CTX_start" } - Expr getContext() { result = this.(FunctionCall).getArgument(0) } + Expr getContext() { result = this.getArgument(0) } } // void BN_CTX_end(BN_CTX *ctx); -class BN_CTX_end extends Expr { - BN_CTX_end() { - exists(FunctionCall fc | - fc = this and - fc.getTarget().getName() = "BN_CTX_end" - ) - } +class BN_CTX_end extends FunctionCall { + BN_CTX_end() { this.getTarget().getName() = "BN_CTX_end" } - Expr getContext() { result = this.(FunctionCall).getArgument(0) } + Expr getContext() { result = this.getArgument(0) } } // BIGNUM *BN_CTX_get(BN_CTX *ctx); -class BN_CTX_get extends Expr { - BN_CTX_get() { - exists(FunctionCall fc | - fc = this and - fc.getTarget().getName() = "BN_CTX_get" - ) - } +class BN_CTX_get extends FunctionCall { + BN_CTX_get() { this.getTarget().getName() = "BN_CTX_get" } - Expr getContext() { result = this.(FunctionCall).getArgument(0) } + Expr getContext() { result = this.getArgument(0) } } -class BN_CTX extends Expr { - BN_CTX() { - exists(FunctionCall fc | - fc = this and - ( - fc.getTarget() instanceof BN_CTX_new or - fc.getTarget() instanceof BN_CTX_secure_new - ) - ) - } +class BN_CTX extends FunctionCall { + BN_CTX() { this.getTarget() instanceof BN_CTX_new } } diff --git a/cpp/src/crypto/BnCtxFreeBeforeEnd.ql b/cpp/src/crypto/BnCtxFreeBeforeEnd.ql index 4be07c7..a1ad6ef 100644 --- a/cpp/src/crypto/BnCtxFreeBeforeEnd.ql +++ b/cpp/src/crypto/BnCtxFreeBeforeEnd.ql @@ -5,7 +5,7 @@ * @kind path-problem * @tags correctness crypto * @problem.severity error - * @precision high + * @precision medium * @group cryptography */ @@ -13,16 +13,13 @@ import cpp import trailofbits.crypto.libraries import semmle.code.cpp.dataflow.new.DataFlow import semmle.code.cpp.controlflow.ControlFlowGraph -import DataFlow::PathGraph /** * Tracks BN_CTX from BN_CTX_start to BN_CTX_free without going through BN_CTX_end */ module BnCtxFreeBeforeEndConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { - exists(BN_CTX_start start | - source.asExpr() = start.getContext() - ) + exists(BN_CTX_start start | source.asExpr() = start.getContext()) } predicate isSink(DataFlow::Node sink) { @@ -34,16 +31,17 @@ module BnCtxFreeBeforeEndConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { // If the context flows through BN_CTX_end, it's properly handled - exists(BN_CTX_end end | - node.asExpr() = end.getContext() - ) + exists(BN_CTX_end end | node.asExpr() = end.getContext()) } } module BnCtxFreeBeforeEndFlow = DataFlow::Global; -from BnCtxFreeBeforeEndFlow::PathNode source, BnCtxFreeBeforeEndFlow::PathNode sink, - BN_CTX_start start, CustomDeallocatorCall free +import BnCtxFreeBeforeEndFlow::PathGraph + +from + BnCtxFreeBeforeEndFlow::PathNode source, BnCtxFreeBeforeEndFlow::PathNode sink, + BN_CTX_start start, CustomDeallocatorCall free where BnCtxFreeBeforeEndFlow::flowPath(source, sink) and source.getNode().asExpr() = start.getContext() and @@ -51,5 +49,4 @@ where free.getTarget() instanceof BN_CTX_free select free, source, sink, "BN_CTX_free called at line " + free.getLocation().getStartLine().toString() + - " before BN_CTX_end after BN_CTX_start at line " + - start.getLocation().getStartLine().toString() + " before BN_CTX_end after BN_CTX_start at line " + start.getLocation().getStartLine().toString() diff --git a/cpp/src/crypto/MissingBnCtxEnd.ql b/cpp/src/crypto/MissingBnCtxEnd.ql deleted file mode 100644 index f350f01..0000000 --- a/cpp/src/crypto/MissingBnCtxEnd.ql +++ /dev/null @@ -1,29 +0,0 @@ -/** - * @name Missing BN_CTX_end after BN_CTX_start - * @id tob/cpp/missing-bn-ctx-end - * @description Detects BN_CTX_start calls without corresponding BN_CTX_end calls - * @kind problem - * @tags correctness crypto - * @problem.severity warning - * @precision medium - * @group cryptography - */ - -import cpp -import trailofbits.crypto.libraries -import semmle.code.cpp.dataflow.new.DataFlow -import semmle.code.cpp.controlflow.Guards - -predicate hasMatchingEnd(BN_CTX_start start) { - exists(BN_CTX_end end, Function f | - start.getEnclosingFunction() = f and - end.getEnclosingFunction() = f and - DataFlow::localFlow(DataFlow::exprNode(start.getContext()), - DataFlow::exprNode(end.getContext())) - ) -} - -from BN_CTX_start start -where not hasMatchingEnd(start) -select start.getLocation(), - "BN_CTX_start called without corresponding BN_CTX_end" diff --git a/cpp/src/crypto/MissingZeroization.ql b/cpp/src/crypto/MissingZeroization.ql index 46dad32..dcd7b16 100644 --- a/cpp/src/crypto/MissingZeroization.ql +++ b/cpp/src/crypto/MissingZeroization.ql @@ -15,16 +15,18 @@ import semmle.code.cpp.dataflow.new.DataFlow predicate isCleared(Expr bignum) { exists(BN_clear clear | - DataFlow::localFlow(DataFlow::exprNode(bignum), DataFlow::exprNode(clear.getArgument(0))) - ) or - exists(BN_clear_free clear_free | - DataFlow::localFlow(DataFlow::exprNode(bignum), DataFlow::exprNode(clear_free.getArgument(0))) + DataFlow::localFlow(DataFlow::exprNode(bignum), DataFlow::exprNode(clear.getBignum())) + ) + or + exists(CustomDeallocatorCall clear_free | + clear_free.getTarget() instanceof BN_clear_free and + DataFlow::localFlow(DataFlow::exprNode(bignum), DataFlow::exprNode(clear_free.getPointer())) ) } predicate isRandom(Expr bignum) { exists(BN_rand rand | - DataFlow::localFlow(DataFlow::exprNode(bignum), DataFlow::exprNode(rand.getArgument(0))) + DataFlow::localFlow(DataFlow::exprNode(bignum), DataFlow::exprNode(rand.getBignum())) ) } diff --git a/cpp/src/crypto/UnbalancedBnCtx.ql b/cpp/src/crypto/UnbalancedBnCtx.ql new file mode 100644 index 0000000..e13f2b4 --- /dev/null +++ b/cpp/src/crypto/UnbalancedBnCtx.ql @@ -0,0 +1,45 @@ +/** + * @name Unbalanced BN_CTX_start and BN_CTX_end pair + * @id tob/cpp/missing-bn-ctx-end + * @description Detects if one call in the BN_CTX_start/BN_CTX_end pair is missing + * @kind problem + * @tags correctness crypto + * @problem.severity warning + * @precision medium + * @group cryptography + */ + +import cpp +import trailofbits.crypto.libraries +import semmle.code.cpp.dataflow.new.DataFlow +import semmle.code.cpp.controlflow.Guards + +predicate hasMatchingEnd(BN_CTX_start start) { + exists(BN_CTX_end end, Function f | + start.getEnclosingFunction() = f and + end.getEnclosingFunction() = f and + DataFlow::localFlow(DataFlow::exprNode(start.getContext()), DataFlow::exprNode(end.getContext())) + ) +} + +predicate hasMatchingStart(BN_CTX_end end) { + exists(BN_CTX_start start, Function f | + end.getEnclosingFunction() = f and + start.getEnclosingFunction() = f and + DataFlow::localFlow(DataFlow::exprNode(start.getContext()), DataFlow::exprNode(end.getContext())) + ) +} + +from FunctionCall call, string message +where + ( + call instanceof BN_CTX_start and + not hasMatchingEnd(call) and + message = "BN_CTX_start called without corresponding BN_CTX_end" + ) or + ( + call instanceof BN_CTX_end and + not hasMatchingStart(call) and + message = "BN_CTX_end called without corresponding BN_CTX_start" + ) +select call.getLocation(), message diff --git a/cpp/test/include/openssl/bn.h b/cpp/test/include/openssl/bn.h index 3e6e3f9..f05102a 100644 --- a/cpp/test/include/openssl/bn.h +++ b/cpp/test/include/openssl/bn.h @@ -32,6 +32,32 @@ int BN_rand(BIGNUM *rnd, int bits, int top, int bottom) { return 1; } +// BN_CTX functions +#define BN_CTX int + +BN_CTX *BN_CTX_new(void) { + static BN_CTX ctx = 1; + return &ctx; +} + +BN_CTX *BN_CTX_secure_new(void) { + static BN_CTX ctx = 1; + return &ctx; +} + +void BN_CTX_free(BN_CTX *c) { +} + +void BN_CTX_start(BN_CTX *ctx) { +} + +void BN_CTX_end(BN_CTX *ctx) { +} + +BIGNUM *BN_CTX_get(BN_CTX *ctx) { + return &BN; +} + # ifdef __cplusplus } #endif diff --git a/cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/BnCtxFreeBeforeEnd.expected b/cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/BnCtxFreeBeforeEnd.expected new file mode 100644 index 0000000..a5527d7 --- /dev/null +++ b/cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/BnCtxFreeBeforeEnd.expected @@ -0,0 +1,12 @@ +edges +| test.c:13:16:13:18 | ctx | test.c:15:15:15:17 | ctx | provenance | | +| test.c:34:16:34:18 | ctx | test.c:40:15:40:17 | ctx | provenance | | +nodes +| test.c:13:16:13:18 | ctx | semmle.label | ctx | +| test.c:15:15:15:17 | ctx | semmle.label | ctx | +| test.c:34:16:34:18 | ctx | semmle.label | ctx | +| test.c:40:15:40:17 | ctx | semmle.label | ctx | +subpaths +#select +| test.c:15:3:15:13 | call to BN_CTX_free | test.c:13:16:13:18 | ctx | test.c:15:15:15:17 | ctx | BN_CTX_free called at line 15 before BN_CTX_end after BN_CTX_start at line 13 | +| test.c:40:3:40:13 | call to BN_CTX_free | test.c:34:16:34:18 | ctx | test.c:40:15:40:17 | ctx | BN_CTX_free called at line 40 before BN_CTX_end after BN_CTX_start at line 34 | diff --git a/cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/BnCtxFreeBeforeEnd.qlref b/cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/BnCtxFreeBeforeEnd.qlref new file mode 100644 index 0000000..1f356fa --- /dev/null +++ b/cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/BnCtxFreeBeforeEnd.qlref @@ -0,0 +1 @@ +crypto/BnCtxFreeBeforeEnd.ql diff --git a/cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/test.c b/cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/test.c new file mode 100644 index 0000000..d80831c --- /dev/null +++ b/cpp/test/query-tests/crypto/BnCtxFreeBeforeEnd/test.c @@ -0,0 +1,43 @@ +#include "../../../include/openssl/bn.h" + +void good_proper_lifecycle() { + BN_CTX *ctx = BN_CTX_new(); + BN_CTX_start(ctx); + BIGNUM *a = BN_CTX_get(ctx); + BN_CTX_end(ctx); // Properly call end before free + BN_CTX_free(ctx); +} + +void bad_free_before_end() { + BN_CTX *ctx = BN_CTX_new(); + BN_CTX_start(ctx); + BIGNUM *a = BN_CTX_get(ctx); + BN_CTX_free(ctx); // BUG: free before end +} + +void good_mult_ctx() { + BN_CTX *ctx = BN_CTX_new(); + + BN_CTX_start(ctx); + BIGNUM *a = BN_CTX_get(ctx); + BN_CTX_end(ctx); + + BN_CTX_start(ctx); + BIGNUM *b = BN_CTX_get(ctx); + BN_CTX_end(ctx); + + BN_CTX_free(ctx); +} + +void bad_conditional_end(int condition) { + BN_CTX *ctx = BN_CTX_new(); + BN_CTX_start(ctx); + BIGNUM *a = BN_CTX_get(ctx); + + if (condition) { + BN_CTX_end(ctx); + } + BN_CTX_free(ctx); // BUG: May free before end if condition is false +} + +int main(void) {} \ No newline at end of file diff --git a/cpp/test/query-tests/crypto/UnbalancedBnCtx/UnbalancedBnCtx.expected b/cpp/test/query-tests/crypto/UnbalancedBnCtx/UnbalancedBnCtx.expected new file mode 100644 index 0000000..98ba061 --- /dev/null +++ b/cpp/test/query-tests/crypto/UnbalancedBnCtx/UnbalancedBnCtx.expected @@ -0,0 +1,2 @@ +| test.c:13:3:13:14 | test.c:13:3:13:14 | BN_CTX_start called without corresponding BN_CTX_end | +| test.c:21:3:21:12 | test.c:21:3:21:12 | BN_CTX_end called without corresponding BN_CTX_start | diff --git a/cpp/test/query-tests/crypto/UnbalancedBnCtx/UnbalancedBnCtx.qlref b/cpp/test/query-tests/crypto/UnbalancedBnCtx/UnbalancedBnCtx.qlref new file mode 100644 index 0000000..9aae5a7 --- /dev/null +++ b/cpp/test/query-tests/crypto/UnbalancedBnCtx/UnbalancedBnCtx.qlref @@ -0,0 +1 @@ +crypto/UnbalancedBnCtx.ql diff --git a/cpp/test/query-tests/crypto/UnbalancedBnCtx/test.c b/cpp/test/query-tests/crypto/UnbalancedBnCtx/test.c new file mode 100644 index 0000000..606cf04 --- /dev/null +++ b/cpp/test/query-tests/crypto/UnbalancedBnCtx/test.c @@ -0,0 +1,62 @@ +#include "../../../include/openssl/bn.h" + +void good_balanced() { + BN_CTX *ctx = BN_CTX_new(); + BN_CTX_start(ctx); + BIGNUM *a = BN_CTX_get(ctx); + BN_CTX_end(ctx); + BN_CTX_free(ctx); +} + +void bad_start_without_end() { + BN_CTX *ctx = BN_CTX_new(); + BN_CTX_start(ctx); // BUG: Missing corresponding end + BIGNUM *a = BN_CTX_get(ctx); + BN_CTX_free(ctx); +} + +void bad_end_without_start() { + BN_CTX *ctx = BN_CTX_new(); + BIGNUM *a = BN_CTX_get(ctx); + BN_CTX_end(ctx); // BUG: No corresponding start + BN_CTX_free(ctx); +} + +void good_conditional_balanced(int condition) { + BN_CTX *ctx = BN_CTX_new(); + if (condition) { + BN_CTX_start(ctx); + BIGNUM *a = BN_CTX_get(ctx); + BN_CTX_end(ctx); + } else { + BN_CTX_start(ctx); + BIGNUM *b = BN_CTX_get(ctx); + BN_CTX_end(ctx); + } + BN_CTX_free(ctx); +} + +void bad_conditional_start(int condition) { + BN_CTX *ctx = BN_CTX_new(); + + if (condition) { + BN_CTX_start(ctx); + BIGNUM *a = BN_CTX_get(ctx); + } + + BN_CTX_end(ctx); // BUG: End without start if condition is false + BN_CTX_free(ctx); +} + +void bad_conditional_end(int condition) { + BN_CTX *ctx = BN_CTX_new(); + + BN_CTX_start(ctx); + BIGNUM *a = BN_CTX_get(ctx); + + if (condition) { + BN_CTX_end(ctx); + } + // BUG: Start without end if condition is false + BN_CTX_free(ctx); +} \ No newline at end of file