Merge pull request #21071 from hvitved/rust/access-after-lifetime-perf

Rust: Speedup `AccessAfterLifetime.ql`

Author: hvitved

rust/ql/lib/codeql/rust/security/AccessAfterLifetimeExtensions.qll

@@ -47,20 +47,6 @@ module AccessAfterLifetime {
     valueScope(source.getTarget(), target, scope)
   }
 
-  /**
-   * 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`.
-   */
-  bindingset[source, sink]
-  predicate dereferenceAfterLifetime(Source source, Sink sink, Variable target) {
-    exists(BlockExpr valueScope, BlockExpr accessScope |
-      sourceValueScope(source, target, valueScope) and
-      accessScope = sink.asExpr().getEnclosingBlock() and
-      not mayEncloseOnStack(valueScope, accessScope)
-    )
-  }
-
   /**
    * Holds if `var` has scope `scope`.
    */
@@ -88,24 +74,6 @@ module AccessAfterLifetime {
     valueScope(value.(FieldExpr).getContainer(), target, scope)
   }
 
-  /**
-   * 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(BlockExpr a, BlockExpr b) {
-    // `b` is a child of `a`
-    a = b.getEnclosingBlock*()
-    or
-    // propagate through function calls
-    exists(Call call |
-      mayEncloseOnStack(a, call.getEnclosingBlock()) and
-      call.getARuntimeTarget() = b.getEnclosingCallable()
-    )
-  }
-
   /**
    * A source that is a `RefExpr`.
    */

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,27 +40,77 @@ 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()]
     )
   }
 }
 
 module AccessAfterLifetimeFlow = TaintTracking::Global<AccessAfterLifetimeConfig>;
 
+predicate sourceBlock(Source s, Variable target, BlockExpr be) {
+  AccessAfterLifetimeFlow::flow(s, _) and
+  sourceValueScope(s, target, be.getEnclosingBlock*())
+}
+
+predicate sinkBlock(Sink s, BlockExpr be) {
+  AccessAfterLifetimeFlow::flow(_, s) and
+  be = s.asExpr().getEnclosingBlock()
+}
+
+private predicate tcStep(BlockExpr a, BlockExpr b) {
+  // propagate through function calls
+  exists(Call call |
+    a = call.getEnclosingBlock() and
+    call.getARuntimeTarget() = b.getEnclosingCallable()
+  )
+}
+
+private predicate isTcSource(BlockExpr be) { sourceBlock(_, _, be) }
+
+private predicate isTcSink(BlockExpr be) { sinkBlock(_, be) }
+
+/**
+ * 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 depending on the
+ * caller a variable in `f` or `g` may or may-not be on the stack during `b`).
+ */
+private predicate mayEncloseOnStack(BlockExpr a, BlockExpr 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 exists(BlockExpr beSource, BlockExpr beSink |
+    sourceBlock(source, target, beSource) and
+    sinkBlock(sink, beSink)
+  |
+    beSource = beSink
+    or
+    mayEncloseOnStack(beSource, beSink)
+  )
+}
+
 from
   AccessAfterLifetimeFlow::PathNode sourceNode, AccessAfterLifetimeFlow::PathNode sinkNode,
   Variable target
 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(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()