unified: Move ad-hoc aliasing logic into a step

Author: asgerf

unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll

@@ -71,7 +71,7 @@ module CaptureSsaInput implements InputSig<Location, BasicBlock> {
     node.(BuilderNode).isCallable(callable)
     or
     exists(Stage2Node prev | callableHasLocalAlias(callable, prev) |
-      step(prev, any(Step s | s.value()), node)
+      step(prev, any(Step s | s.value() or s.callGraph()), node)
       or
       localSsaStep(prev, node, _)
     )
@@ -101,19 +101,6 @@ module CaptureSsaInput implements InputSig<Location, BasicBlock> {
     predicate hasAliasedAccess(Expr f) {
       // TODO: Restrict to places where the function is called or escapes into a call where it might get called.
       callableHasLocalAlias(callable, f) and not f.(LocalVariableRefNode).getRefKind().isWrite()
-      or
-      // Record the callee argument passed into a local capturing getter/setter as an alias for that getter/setter
-      // so it can carry capture data in and out of the getter/setter call.
-      exists(ComputedLocalVariable local, DataFlowCall call |
-        callable = local.getAnAccessor() and
-        f = TImplicitArgumentNode(call, any(ArgumentPosition p | p.isCallee()), false)
-      |
-        callable instanceof GetterDeclaration and
-        call.asGetterCall() = local.getAnAccess()
-        or
-        callable instanceof SetterDeclaration and
-        call.asSetterCall() = local.getAnAccess()
-      )
     }
   }
 }

unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll

@@ -161,6 +161,20 @@ predicate step(BuilderNode node1, Step step, BuilderNode node2) {
     node2.isIncomingValue(arg.getValue())
   )
   or
+  // For local getters/setters, pass the callable into the callee argument.
+  // Capture SSA depends on this.
+  exists(ComputedLocalVariable local, AccessorDeclaration accessor, DataFlowCall call |
+    accessor = local.getAnAccessor() and
+    node1.isCallable(accessor) and
+    step.callGraph() and
+    (
+      call.asGetterCall() = local.getAnAccess() and accessor instanceof GetterDeclaration
+      or
+      call.asSetterCall() = local.getAnAccess() and accessor instanceof SetterDeclaration
+    ) and
+    node2.isImplicitArgument(call, any(ArgumentPosition pos | pos.isCallee()), false)
+  )
+  or
   exists(SwitchExpr expr |
     node1.isResultValue(expr.getValue()) and
     step.value() and

unified/ql/lib/codeql/unified/internal/dataflow/Step.qll

@@ -7,7 +7,8 @@ private newtype TStep =
   TJumpStep() or
   TTaintStep() or
   TReadStep(ContentSet contents) or
-  TStoreStep(ContentSet contents)
+  TStoreStep(ContentSet contents) or
+  TCallGraphStep()
 
 /**
  * A type of data flow step, used during construction of the data flow graph.
@@ -47,6 +48,9 @@ class Step extends TStep {
   pragma[nomagic]
   predicate storeArrayElement() { this.store(ContentSet::arrayElement()) }
 
+  /** Holds if this step can only be used for call graph construction. */
+  predicate callGraph() { this = TCallGraphStep() }
+
   string toString() {
     this.value() and result = "value"
     or
@@ -61,5 +65,7 @@ class Step extends TStep {
       or
       this.store(contents) and result = "store[" + contents + "]"
     )
+    or
+    this.callGraph() and result = "call-graph"
   }
 }