diff --git a/rust/ql/lib/codeql/rust/security/AccessAfterLifetimeExtensions.qll b/rust/ql/lib/codeql/rust/security/AccessAfterLifetimeExtensions.qll index c99d1e0d0c2..c404f13b531 100644 --- a/rust/ql/lib/codeql/rust/security/AccessAfterLifetimeExtensions.qll +++ b/rust/ql/lib/codeql/rust/security/AccessAfterLifetimeExtensions.qll @@ -47,90 +47,6 @@ module AccessAfterLifetime { valueScope(source.getTarget(), target, scope) } - /** - * Holds if the pair `(source, sink)` represents a flow from a pointer or reference - * to a dereference. - */ - signature predicate dereferenceAfterLifetimeCandSig(DataFlow::Node source, DataFlow::Node sink); - - /** Provides logic for identifying dereferences after lifetime. */ - module DereferenceAfterLifetime { - private newtype TTcNode = - TSource(Source s, Variable target) { - dereferenceAfterLifetimeCand(s, _) and sourceValueScope(s, target, _) - } or - TBlockExpr(BlockExpr be) or - TSink(Sink s) { dereferenceAfterLifetimeCand(_, s) } - - private class TcNode extends TTcNode { - Source asSource(Variable target) { this = TSource(result, target) } - - BlockExpr asBlockExpr() { this = TBlockExpr(result) } - - Sink asSink() { this = TSink(result) } - - string toString() { - result = this.asSource(_).toString() - or - result = this.asBlockExpr().toString() - or - result = this.asSink().toString() - } - - Location getLocation() { - result = this.asSource(_).getLocation() - or - result = this.asBlockExpr().getLocation() - or - result = this.asSink().getLocation() - } - } - - pragma[nomagic] - private predicate tcStep(TcNode a, TcNode b) { - // `b` is a child of `a` - exists(Source source, Variable target, BlockExpr be | - source = a.asSource(target) and - be = b.asBlockExpr().getEnclosingBlock*() and - sourceValueScope(source, target, be) and - dereferenceAfterLifetimeCand(source, _) - ) - or - // propagate through function calls - exists(Call call | - a.asBlockExpr() = call.getEnclosingBlock() and - call.getARuntimeTarget() = b.asBlockExpr().getEnclosingCallable() - ) - or - a.asBlockExpr() = b.asSink().asExpr().getEnclosingBlock() - } - - private predicate isTcSource(TcNode n) { n instanceof TSource } - - private predicate isTcSink(TcNode n) { n instanceof TSink } - - /** - * Holds if block `a` contains block `b`, in the sense that a stack allocated variable in - * `a` may still be on the stack during execution of `b`. This is interprocedural, - * but is an overapproximation that doesn't accurately track call contexts - * (for example if `f` and `g` both call `b`, then then depending on the - * caller a variable in `f` or `g` may or may-not be on the stack during `b`). - */ - private predicate mayEncloseOnStack(TcNode a, TcNode b) = - doublyBoundedFastTC(tcStep/2, isTcSource/1, isTcSink/1)(a, b) - - /** - * Holds if the pair `(source, sink)`, that represents a flow from a - * pointer or reference to a dereference, has its dereference outside the - * lifetime of the target variable `target`. - */ - predicate dereferenceAfterLifetime(Source source, Sink sink, Variable target) { - dereferenceAfterLifetimeCand(source, sink) and - sourceValueScope(source, target, _) and - not mayEncloseOnStack(TSource(source, target), TSink(sink)) - } - } - /** * Holds if `var` has scope `scope`. */ diff --git a/rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql b/rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql index 9e698d40a1c..fb278185b19 100644 --- a/rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql +++ b/rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql @@ -15,7 +15,7 @@ import rust import codeql.rust.dataflow.DataFlow import codeql.rust.dataflow.TaintTracking -import codeql.rust.security.AccessAfterLifetimeExtensions +import codeql.rust.security.AccessAfterLifetimeExtensions::AccessAfterLifetime import AccessAfterLifetimeFlow::PathGraph /** @@ -24,14 +24,14 @@ import AccessAfterLifetimeFlow::PathGraph */ module AccessAfterLifetimeConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node node) { - node instanceof AccessAfterLifetime::Source and + node instanceof Source and // exclude cases with sources in macros, since these results are difficult to interpret not node.asExpr().isFromMacroExpansion() and - AccessAfterLifetime::sourceValueScope(node, _, _) + sourceValueScope(node, _, _) } predicate isSink(DataFlow::Node node) { - node instanceof AccessAfterLifetime::Sink and + node instanceof Sink and // Exclude cases with sinks in macros, since these results are difficult to interpret not node.asExpr().isFromMacroExpansion() and // TODO: Remove this condition if it can be done without negatively @@ -40,13 +40,13 @@ module AccessAfterLifetimeConfig implements DataFlow::ConfigSig { exists(node.asExpr()) } - predicate isBarrier(DataFlow::Node barrier) { barrier instanceof AccessAfterLifetime::Barrier } + predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier } predicate observeDiffInformedIncrementalMode() { any() } Location getASelectedSourceLocation(DataFlow::Node source) { exists(Variable target | - AccessAfterLifetime::sourceValueScope(source, target, _) and + sourceValueScope(source, target, _) and result = [target.getLocation(), source.getLocation()] ) } @@ -54,6 +54,81 @@ module AccessAfterLifetimeConfig implements DataFlow::ConfigSig { module AccessAfterLifetimeFlow = TaintTracking::Global; +private newtype TTcNode = + TSource(Source s, Variable target) { + AccessAfterLifetimeFlow::flow(s, _) and sourceValueScope(s, target, _) + } or + TBlockExpr(BlockExpr be) or + TSink(Sink s) { AccessAfterLifetimeFlow::flow(_, s) } + +private class TcNode extends TTcNode { + Source asSource(Variable target) { this = TSource(result, target) } + + BlockExpr asBlockExpr() { this = TBlockExpr(result) } + + Sink asSink() { this = TSink(result) } + + string toString() { + result = this.asSource(_).toString() + or + result = this.asBlockExpr().toString() + or + result = this.asSink().toString() + } + + Location getLocation() { + result = this.asSource(_).getLocation() + or + result = this.asBlockExpr().getLocation() + or + result = this.asSink().getLocation() + } +} + +pragma[nomagic] +private predicate tcStep(TcNode a, TcNode b) { + // `b` is a child of `a` + exists(Source source, Variable target, BlockExpr be | + source = a.asSource(target) and + be = b.asBlockExpr().getEnclosingBlock*() and + sourceValueScope(source, target, be) and + AccessAfterLifetimeFlow::flow(source, _) + ) + or + // propagate through function calls + exists(Call call | + a.asBlockExpr() = call.getEnclosingBlock() and + call.getARuntimeTarget() = b.asBlockExpr().getEnclosingCallable() + ) + or + a.asBlockExpr() = b.asSink().asExpr().getEnclosingBlock() +} + +private predicate isTcSource(TcNode n) { n instanceof TSource } + +private predicate isTcSink(TcNode n) { n instanceof TSink } + +/** + * Holds if block `a` contains block `b`, in the sense that a stack allocated variable in + * `a` may still be on the stack during execution of `b`. This is interprocedural, + * but is an overapproximation that doesn't accurately track call contexts + * (for example if `f` and `g` both call `b`, then then depending on the + * caller a variable in `f` or `g` may or may-not be on the stack during `b`). + */ +private predicate mayEncloseOnStack(TcNode a, TcNode b) = + doublyBoundedFastTC(tcStep/2, isTcSource/1, isTcSink/1)(a, b) + +/** + * Holds if the pair `(source, sink)`, that represents a flow from a + * pointer or reference to a dereference, has its dereference outside the + * lifetime of the target variable `target`. + */ +predicate dereferenceAfterLifetime(Source source, Sink sink, Variable target) { + AccessAfterLifetimeFlow::flow(source, sink) and + sourceValueScope(source, target, _) and + not mayEncloseOnStack(TSource(source, target), TSink(sink)) +} + from AccessAfterLifetimeFlow::PathNode sourceNode, AccessAfterLifetimeFlow::PathNode sinkNode, Variable target @@ -61,7 +136,6 @@ where // flow from a pointer or reference to the dereference AccessAfterLifetimeFlow::flowPath(sourceNode, sinkNode) and // check that the dereference is outside the lifetime of the target - AccessAfterLifetime::DereferenceAfterLifetime::dereferenceAfterLifetime(sourceNode - .getNode(), sinkNode.getNode(), target) + dereferenceAfterLifetime(sourceNode.getNode(), sinkNode.getNode(), target) select sinkNode.getNode(), sourceNode, sinkNode, "Access of a pointer to $@ after its lifetime has ended.", target, target.toString()