Rust: Move logic from AccessAfterLifetimeExtensions.qll to AccessAfterLifetime.ql

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<dereferenceAfterLifetimeCandSig/2 dereferenceAfterLifetimeCand> {
-    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`.
    */

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<AccessAfterLifetimeConfig>;
 
+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<AccessAfterLifetimeFlow::flow/2>::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()