From 050b8e682f806187c28802dc23d3fd47fdc21e31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Tue, 29 Aug 2023 16:47:41 +0200
Subject: [PATCH 01/32] Swift: add failing inline expectation test based on
 closure AST tests.

---
 .../ql/test/TestUtilities/InlineFlowTest.qll  | 117 +++++++++++++
 .../dataflow/capture/closures.swift           | 155 ++++++++++++++++++
 .../dataflow/capture/inlinetest.expected      |   2 +
 .../dataflow/capture/inlinetest.ql            |   2 +
 4 files changed, 276 insertions(+)
 create mode 100644 swift/ql/test/TestUtilities/InlineFlowTest.qll
 create mode 100644 swift/ql/test/library-tests/dataflow/capture/closures.swift
 create mode 100644 swift/ql/test/library-tests/dataflow/capture/inlinetest.expected
 create mode 100644 swift/ql/test/library-tests/dataflow/capture/inlinetest.ql

diff --git a/swift/ql/test/TestUtilities/InlineFlowTest.qll b/swift/ql/test/TestUtilities/InlineFlowTest.qll
new file mode 100644
index 000000000000..4dcd1fb8bc57
--- /dev/null
+++ b/swift/ql/test/TestUtilities/InlineFlowTest.qll
@@ -0,0 +1,117 @@
+/**
+ * Provides a simple base test for flow-related tests using inline expectations.
+ *
+ * Example for a test.ql:
+ * ```ql
+ * import swift
+ * import TestUtilities.InlineFlowTest
+ * import DefaultFlowTest
+ * import PathGraph
+ *
+ * from PathNode source, PathNode sink
+ * where flowPath(source, sink)
+ * select sink, source, sink, "$@", source, source.toString()
+ * ```
+ *
+ * To declare expectations, you can use the $hasTaintFlow or $hasValueFlow comments within the test source files.
+ * Example of the corresponding test file, e.g. Test.java
+ * ```swift
+ * func source() -> Any { return nil }
+ * func taint() -> Any { return nil }
+ * func sink(_ o: Any) { }
+ *
+ * func test() {
+ *     let s = source()
+ *     sink(s) // $ hasValueFlow
+ *     let t = "foo" + taint()
+ *     sink(t); // $ hasTaintFlow
+ * }
+ * ```
+ *
+ * If you are only interested in value flow, then instead of importing `DefaultFlowTest`, you can import
+ * `ValueFlowTest<DefaultFlowConfig>`. Similarly, if you are only interested in taint flow, then instead of
+ * importing `DefaultFlowTest`, you can import `TaintFlowTest<DefaultFlowConfig>`. In both cases
+ * `DefaultFlowConfig` can be replaced by another implementation of `DataFlow::ConfigSig`.
+ *
+ * If you need more fine-grained tuning, consider implementing a test using `InlineExpectationsTest`.
+ */
+
+import codeql.swift.dataflow.DataFlow
+import codeql.swift.dataflow.ExternalFlow
+import codeql.swift.dataflow.TaintTracking
+import TestUtilities.InlineExpectationsTest
+
+private predicate defaultSource(DataFlow::Node source) {
+  source.asExpr().(MethodCallExpr).getStaticTarget().getName() = ["source", "taint"]
+}
+
+private predicate defaultSink(DataFlow::Node sink) {
+  exists(MethodCallExpr ma | ma.getStaticTarget().hasName("sink") |
+    sink.asExpr() = ma.getAnArgument().getExpr()
+  )
+}
+
+module DefaultFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { defaultSource(source) }
+
+  predicate isSink(DataFlow::Node sink) { defaultSink(sink) }
+
+  int fieldFlowBranchLimit() { result = 1000 }
+}
+
+private module NoFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { none() }
+
+  predicate isSink(DataFlow::Node sink) { none() }
+}
+
+private string getSourceArgString(DataFlow::Node src) {
+  defaultSource(src) and
+  src.asExpr().(MethodCallExpr).getAnArgument().getExpr().(StringLiteralExpr).getValue() = result
+}
+
+module FlowTest<DataFlow::ConfigSig ValueFlowConfig, DataFlow::ConfigSig TaintFlowConfig> {
+  module ValueFlow = DataFlow::Global<ValueFlowConfig>;
+
+  module TaintFlow = TaintTracking::Global<TaintFlowConfig>;
+
+  private module InlineTest implements TestSig {
+    string getARelevantTag() { result = ["hasValueFlow", "hasTaintFlow"] }
+
+    predicate hasActualResult(Location location, string element, string tag, string value) {
+      tag = "hasValueFlow" and
+      exists(DataFlow::Node src, DataFlow::Node sink | ValueFlow::flow(src, sink) |
+        sink.getLocation() = location and
+        element = sink.toString() and
+        if exists(getSourceArgString(src)) then value = getSourceArgString(src) else value = ""
+      )
+      or
+      tag = "hasTaintFlow" and
+      exists(DataFlow::Node src, DataFlow::Node sink |
+        TaintFlow::flow(src, sink) and not ValueFlow::flow(src, sink)
+      |
+        sink.getLocation() = location and
+        element = sink.toString() and
+        if exists(getSourceArgString(src)) then value = getSourceArgString(src) else value = ""
+      )
+    }
+  }
+
+  import MakeTest<InlineTest>
+  import DataFlow::MergePathGraph<ValueFlow::PathNode, TaintFlow::PathNode, ValueFlow::PathGraph, TaintFlow::PathGraph>
+
+  predicate flowPath(PathNode source, PathNode sink) {
+    ValueFlow::flowPath(source.asPathNode1(), sink.asPathNode1()) or
+    TaintFlow::flowPath(source.asPathNode2(), sink.asPathNode2())
+  }
+}
+
+module DefaultFlowTest = FlowTest<DefaultFlowConfig, DefaultFlowConfig>;
+
+module ValueFlowTest<DataFlow::ConfigSig ValueFlowConfig> {
+  import FlowTest<ValueFlowConfig, NoFlowConfig>
+}
+
+module TaintFlowTest<DataFlow::ConfigSig TaintFlowConfig> {
+  import FlowTest<NoFlowConfig, TaintFlowConfig>
+}
diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
new file mode 100644
index 000000000000..8260782e129c
--- /dev/null
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -0,0 +1,155 @@
+func sink<T>(_ value: T) { print("sink:", value) }
+func source<T>(_ label: String, _ value: T) -> T { return value }
+func taint<T>(_ label: String, _ value: T) -> T { return value }
+
+func hello() -> String {
+  let value = "Hello world!"
+  return source("hello", value)
+}
+
+func captureList() {
+  let y: Int = source("captureList", 123);
+  { [x = hello()] () in
+     sink(x) // $ MISSING: hasValueFlow=hello
+     sink(y) // $ MISSING: hasValueFlow=captureList
+  }()
+}
+
+var escape: (() -> Int)? = nil
+
+func setEscape() {
+  var x = source("setEscape", 0)
+  escape = {
+    sink(x) // $ MISSING: hasValueFlow=setEscape
+    return x + 1
+  }
+}
+
+func callEscape() {
+  setEscape()
+  sink(escape?()) // $ MISSING: hasTaintFlow=setEscape
+}
+
+func logical() -> Bool {
+  let f: ((Int) -> Int)? = { x in x + 1 }
+  let x: Int? = source("logical", 42)
+  return f != nil
+      && (x != nil
+          && f!(x!) == 43) // $ MISSING: hasValueFlow=logical
+}
+
+func asyncTest() {
+  func withCallback(_ callback: @escaping (Int) async -> Int) {
+    @Sendable
+    func wrapper(_ x: Int) async -> Int {
+      return await callback(x + 1) // $ MISSING: hasValueFlow=asyncTest
+    }
+    Task {
+      print("asyncTest():", await wrapper(source("asyncTest", 40)))
+    }
+  }
+  withCallback { x in
+    x + 1 // $ MISSING: hasTaintFlow=asyncTest
+  }
+}
+
+func foo() -> Int {
+  var x = 1
+  let f = { y in x += y }
+  x = source("foo", 41)
+  let r = { x }
+  sink(r()) // $ MISSING: hasValueFlow=foo
+  f(1)
+  return r() // $ MISSING: hasTaintFlow=foo
+}
+
+func bar() -> () -> Int {
+  var x = 1
+  let f = { y in x += y }
+  x = source("bar", 41)
+  let r = { x }
+  f(1)
+  return r // constantly 42
+}
+
+var g: ((Int) -> Void)? = nil
+func baz() -> () -> Int {
+  var x = 1
+  g = { y in x += y }
+  x = source("baz", 41)
+  let r = { x }
+  g!(1)
+  return r
+}
+
+func sharedCapture() -> Int {
+  let (incrX, getX) = {
+    var x = source("sharedCapture", 0)
+    return ({ x += 1 }, { x })
+  }()
+
+  let doubleIncrX = {
+    incrX()
+    incrX()
+  }
+
+  sink(getX()) // $ MISSING: hasValueFlow=sharedCapture
+  doubleIncrX()
+  sink(getX()) // $ MISSING: hasTaintFlow=sharedCapture
+  doubleIncrX()
+  return getX()
+}
+
+func sharedCaptureMultipleWriters() {
+  var x = 123
+
+  let callSink1 = { sink(x) } // $ MISSING: hasValueFlow=setter1
+  let callSink2 = { sink(x) } // $ MISSING: hasValueFlow=setter2
+
+  let makeSetter = { y in
+    let setter = { x = y }
+    return setter
+  }
+
+  let setter1 = makeSetter(source("setter1", 1))
+  let setter2 = makeSetter(source("setter2", 2))
+
+  setter1()
+  callSink1()
+
+  setter2()
+  callSink2()
+}
+
+
+func main() {
+  print("captureList():")
+  captureList() // Hello world! 123
+
+  print("callEscape():")
+  callEscape() // 1
+
+  print("logical():", logical()) // true
+
+  print("asyncTest():")
+  asyncTest() // 42
+
+  print("foo():", foo()) // 42
+
+  let a = bar()
+  let b = baz()
+
+  print("bar():", a(), a()) // $ MISSING: hasTaintFlow=bar
+
+  print("baz():", b(), b()) // $ MISSING: hasTaintFlow=baz
+
+  g!(1)
+  print("g!(1):", b(), b()) // $ MISSING: hasTaintFlow=baz
+
+  print("sharedCapture():", sharedCapture()) // 4
+
+  print("sharedCaptureMultipleWriters():")
+  sharedCaptureMultipleWriters() // 42, -1
+}
+
+main()
diff --git a/swift/ql/test/library-tests/dataflow/capture/inlinetest.expected b/swift/ql/test/library-tests/dataflow/capture/inlinetest.expected
new file mode 100644
index 000000000000..48de9172b362
--- /dev/null
+++ b/swift/ql/test/library-tests/dataflow/capture/inlinetest.expected
@@ -0,0 +1,2 @@
+failures
+testFailures
diff --git a/swift/ql/test/library-tests/dataflow/capture/inlinetest.ql b/swift/ql/test/library-tests/dataflow/capture/inlinetest.ql
new file mode 100644
index 000000000000..50e3f8d2f7de
--- /dev/null
+++ b/swift/ql/test/library-tests/dataflow/capture/inlinetest.ql
@@ -0,0 +1,2 @@
+import TestUtilities.InlineFlowTest
+import DefaultFlowTest

From 3253c0425c9a91635da9a2607c5acf1ee3dae0d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Tue, 29 Aug 2023 17:32:33 +0200
Subject: [PATCH 02/32] Swift: s/getName/getShortName/ in InlineFlowTest.qll

---
 swift/ql/test/TestUtilities/InlineFlowTest.qll | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/swift/ql/test/TestUtilities/InlineFlowTest.qll b/swift/ql/test/TestUtilities/InlineFlowTest.qll
index 4dcd1fb8bc57..5ddad0abab66 100644
--- a/swift/ql/test/TestUtilities/InlineFlowTest.qll
+++ b/swift/ql/test/TestUtilities/InlineFlowTest.qll
@@ -42,11 +42,11 @@ import codeql.swift.dataflow.TaintTracking
 import TestUtilities.InlineExpectationsTest
 
 private predicate defaultSource(DataFlow::Node source) {
-  source.asExpr().(MethodCallExpr).getStaticTarget().getName() = ["source", "taint"]
+  source.asExpr().(MethodCallExpr).getStaticTarget().getShortName() = ["source", "taint"]
 }
 
 private predicate defaultSink(DataFlow::Node sink) {
-  exists(MethodCallExpr ma | ma.getStaticTarget().hasName("sink") |
+  exists(MethodCallExpr ma | ma.getStaticTarget().getShortName() = "sink" |
     sink.asExpr() = ma.getAnArgument().getExpr()
   )
 }

From 95a7d6559c8855e38aff088751d8fd8267118028 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Wed, 30 Aug 2023 20:47:50 +0200
Subject: [PATCH 03/32] Swift: initial version of a swift port of most of the
 java code

---
 .../dataflow/internal/DataFlowPrivate.qll     | 151 +++++++++++++++++-
 .../dataflow/internal/DataFlowPublic.qll      |  34 +++-
 2 files changed, 177 insertions(+), 8 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index ae25463b21ac..b9e77404c5d2 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -10,6 +10,7 @@ private import codeql.swift.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
 private import codeql.swift.frameworks.StandardLibrary.PointerTypes
 private import codeql.swift.frameworks.StandardLibrary.Array
 private import codeql.swift.frameworks.StandardLibrary.Dictionary
+private import codeql.dataflow.VariableCapture as VariableCapture
 
 /** Gets the callable in which this node occurs. */
 DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.(NodeImpl).getEnclosingCallable() }
@@ -98,6 +99,16 @@ private class SsaDefinitionNodeImpl extends SsaDefinitionNode, NodeImpl {
   }
 }
 
+private class CaptureNodeImpl extends CaptureNode, NodeImpl {
+  override Location getLocationImpl() { result = this.getSynthesizedCaptureNode().getLocation() }
+
+  override string toStringImpl() { result = this.getSynthesizedCaptureNode().toString() }
+
+  override DataFlowCallable getEnclosingCallable() {
+    result.asSourceCallable() = this.getSynthesizedCaptureNode().getEnclosingCallable()
+  }
+}
+
 private predicate localFlowSsaInput(Node nodeFrom, Ssa::Definition def, Ssa::Definition next) {
   exists(BasicBlock bb, int i | def.lastRefRedef(bb, i, next) |
     def.definesAt(_, bb, i) and
@@ -145,7 +156,8 @@ private module Cached {
     } or
     TDictionarySubscriptNode(SubscriptExpr e) {
       e.getBase().getType().getCanonicalType() instanceof CanonicalDictionaryType
-    }
+    } or
+    TCaptureNode(CaptureFlow::SynthesizedCaptureNode cn)
 
   private predicate localSsaFlowStepUseUse(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
     def.adjacentReadPair(nodeFrom.getCfgNode(), nodeTo.getCfgNode()) and
@@ -305,7 +317,8 @@ private module Cached {
     TFieldContent(FieldDecl f) or
     TTupleContent(int index) { exists(any(TupleExpr te).getElement(index)) } or
     TEnumContent(ParamDecl f) { exists(EnumElementDecl d | d.getAParam() = f) } or
-    TCollectionContent()
+    TCollectionContent() or
+    TCapturedVariableContent(CapturedVariable v)
 }
 
 /**
@@ -371,7 +384,7 @@ private module ParameterNodes {
     predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { none() }
 
     /** Gets the parameter associated with this node, if any. */
-    ParamDecl getParameter() { none() }
+    override ParamDecl getParameter() { none() }
   }
 
   class SourceParameterNode extends ParameterNodeImpl, TSourceParameterNode {
@@ -658,7 +671,7 @@ private module ReturnNodes {
       result = TDataFlowFunc(param.getDeclaringFunction())
     }
 
-    ParamDecl getParameter() { result = param }
+    override ParamDecl getParameter() { result = param }
 
     override Location getLocationImpl() { result = exit.getLocation() }
 
@@ -785,6 +798,136 @@ private module OutNodes {
 
 import OutNodes
 
+private predicate closureFlowStep(Expr e1, Expr e2) {
+  // simpleLocalFlowStep(exprNode(e1), exprNode(e2)) // TODO: find out why the java version uses simpleAstFlowStep... probably due to non-monotonic recursion
+  // or
+  exists(Ssa::WriteDefinition def |
+    def.getARead().getNode().asAstNode() = e2 and
+    def.assigns(any(CfgNode cfg | cfg.getNode().asAstNode() = e1))
+  )
+}
+
+private module CaptureInput implements VariableCapture::InputSig {
+  private import swift as S
+  private import codeql.swift.controlflow.BasicBlocks as B
+
+  class Location = S::Location;
+
+  class BasicBlock instanceof B::BasicBlock {
+    string toString() { result = super.toString() }
+
+    Callable getEnclosingCallable() { result = super.getScope() }
+
+    Location getLocation() { result = super.getLocation() }
+  }
+
+  BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.(B::BasicBlock).dominates(bb) }
+
+  BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.(B::BasicBlock).getASuccessor() }
+
+  //TODO: support capture of `this` in lambdas
+  class CapturedVariable instanceof S::CapturedDecl {
+    string toString() { result = super.toString() }
+
+    Callable getCallable() { result = super.getScope() }
+
+    Location getLocation() { result = super.getLocation() }
+  }
+
+  class CapturedParameter extends CapturedVariable {
+    CapturedParameter() { this.(S::CapturedDecl).getDecl() instanceof S::ParamDecl }
+  }
+
+  class Expr instanceof S::AstNode {
+    string toString() { result = super.toString() }
+
+    Location getLocation() { result = super.getLocation() }
+
+    predicate hasCfgNode(BasicBlock bb, int i) {
+      this = bb.(B::BasicBlock).getNode(i).getNode().asAstNode()
+    }
+  }
+
+  class VariableWrite extends Expr {
+    CapturedVariable variable;
+    Expr source;
+
+    VariableWrite() {
+      exists(S::VarDecl varDecl |
+        variable.(S::CapturedDecl).getDecl() = varDecl and
+        variable.getCallable() = this.(S::AstNode).getEnclosingCallable()
+      |
+        exists(S::Assignment a | this = a |
+          a.getDest().(DeclRefExpr).getDecl() = varDecl and
+          source = a.getSource()
+        )
+        or
+        exists(S::PatternBindingDecl pbd, S::NamedPattern np |
+          this = pbd and pbd.getAPattern() = np
+        |
+          np.getVarDecl() = varDecl and
+          source = np.getMatchingExpr()
+        )
+        // TODO: support multiple variables in LHS of =, in both of above cases.
+      )
+    }
+
+    CapturedVariable getVariable() { result = variable }
+
+    Expr getSource() { result = source }
+  }
+
+  class VariableRead extends Expr instanceof S::DeclRefExpr {
+    CapturedVariable v;
+
+    VariableRead() { this.getCapturedDecl() = v /* TODO: this should be an R-value only. */ }
+
+    CapturedVariable getVariable() { result = v }
+  }
+
+  class ClosureExpr extends Expr instanceof S::Callable {
+    ClosureExpr() { any(S::CapturedDecl c).getScope() = this }
+
+    predicate hasBody(Callable body) { this = body }
+
+    predicate hasAliasedAccess(Expr f) {
+      closureFlowStep+(this, f) and not closureFlowStep(f, _)
+    /* TODO: understand why this is intra-procedural */ }
+  }
+
+  class Callable extends S::Callable {
+    predicate isConstructor() { this instanceof S::Initializer }
+  }
+}
+
+class CapturedVariable = CaptureInput::CapturedVariable;
+
+class CapturedParameter = CaptureInput::CapturedParameter;
+
+module CaptureFlow = VariableCapture::Flow<CaptureInput>;
+
+private CaptureFlow::ClosureNode asClosureNode(Node n) {
+  result = n.(CaptureNode).getSynthesizedCaptureNode() or
+  result.(CaptureFlow::ExprNode).getExpr() = n.asExpr() or
+  result.(CaptureFlow::ExprPostUpdateNode).getExpr() =
+    n.(PostUpdateNode).getPreUpdateNode().asExpr() or
+  result.(CaptureFlow::ParameterNode).getParameter().(CapturedDecl).getDecl() = n.getParameter() or
+  result.(CaptureFlow::ThisParameterNode).getCallable().getSelfParam() = n.getParameter() or
+  result.(CaptureFlow::MallocNode).getClosureExpr() = n.getCfgNode().getNode().asAstNode() // TODO: figure out why the java version had PostUpdateNode logic here
+}
+
+private predicate captureStoreStep(Node node1, Content::CapturedVariableContent c, Node node2) {
+  CaptureFlow::storeStep(asClosureNode(node1), c.getVariable(), asClosureNode(node2))
+}
+
+private predicate captureReadStep(Node node1, Content::CapturedVariableContent c, Node node2) {
+  CaptureFlow::readStep(asClosureNode(node1), c.getVariable(), asClosureNode(node2))
+}
+
+predicate captureValueStep(Node node1, Node node2) {
+  CaptureFlow::localFlowStep(asClosureNode(node1), asClosureNode(node2))
+}
+
 predicate jumpStep(Node pred, Node succ) {
   FlowSummaryImpl::Private::Steps::summaryJumpStep(pred.(FlowSummaryNode).getSummaryNode(),
     succ.(FlowSummaryNode).getSummaryNode())
diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll
index f8887071451e..07279d633937 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll
@@ -51,6 +51,11 @@ class Node extends TNode {
    * Gets this node's underlying SSA definition, if any.
    */
   Ssa::Definition asDefinition() { none() }
+
+  /**
+   * Gets the parameter that corresponds to this node, if any.
+   */
+  ParamDecl getParameter() { none() }
 }
 
 /**
@@ -96,7 +101,7 @@ class ParameterNode extends Node instanceof ParameterNodeImpl {
     result = this.(ParameterNodeImpl).getEnclosingCallable()
   }
 
-  ParamDecl getParameter() { result = this.(ParameterNodeImpl).getParameter() }
+  override ParamDecl getParameter() { result = this.(ParameterNodeImpl).getParameter() }
 }
 
 /**
@@ -109,9 +114,7 @@ class SsaDefinitionNode extends Node, TSsaDefinitionNode {
   override Ssa::Definition asDefinition() { result = def }
 }
 
-class InoutReturnNode extends Node instanceof InoutReturnNodeImpl {
-  ParamDecl getParameter() { result = super.getParameter() }
-}
+class InoutReturnNode extends Node instanceof InoutReturnNodeImpl { }
 
 /**
  * A node associated with an object after an operation that might have
@@ -129,6 +132,18 @@ class PostUpdateNode extends Node instanceof PostUpdateNodeImpl {
   Node getPreUpdateNode() { result = super.getPreUpdateNode() }
 }
 
+/**
+ * A synthesized data flow node representing a closure object that tracks
+ * captured variables.
+ */
+class CaptureNode extends Node, TCaptureNode {
+  private CaptureFlow::SynthesizedCaptureNode cn;
+
+  CaptureNode() { this = TCaptureNode(cn) }
+
+  CaptureFlow::SynthesizedCaptureNode getSynthesizedCaptureNode() { result = cn }
+}
+
 /**
  * Gets a node corresponding to expression `e`.
  */
@@ -234,6 +249,17 @@ module Content {
    * DEPRECATED: An element of a collection. This is an alias for the general CollectionContent.
    */
   deprecated class ArrayContent = CollectionContent;
+
+  /** A captured variable. */
+  class CapturedVariableContent extends Content, TCapturedVariableContent {
+    CapturedVariable v;
+
+    CapturedVariableContent() { this = TCapturedVariableContent(v) }
+
+    CapturedVariable getVariable() { result = v }
+
+    override string toString() { result = v.toString() }
+  }
 }
 
 /**

From c04654d8f9d4e7f4f0b99866b235330b719c116e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Tue, 5 Sep 2023 14:28:56 +0200
Subject: [PATCH 04/32] Swift: getImmediateBasicBlockDominator/2 should use
 immediatelyDominates/0.

---
 .../ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index b9e77404c5d2..06ffdc797a71 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -821,7 +821,9 @@ private module CaptureInput implements VariableCapture::InputSig {
     Location getLocation() { result = super.getLocation() }
   }
 
-  BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.(B::BasicBlock).dominates(bb) }
+  BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) {
+    result.(B::BasicBlock).immediatelyDominates(bb)
+  }
 
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.(B::BasicBlock).getASuccessor() }
 

From 21a369de1353c2522c3d944e3f5e159cd1125e7d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Mon, 11 Sep 2023 14:28:07 +0200
Subject: [PATCH 05/32] Swift: Add closure content read-write steps

---
 .../ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index 06ffdc797a71..7d0165436968 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -1042,6 +1042,8 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
     node2.(FlowSummaryNode).getSummaryNode())
+  or
+  captureStoreStep(node1, any(Content::CapturedVariableContent cvc | c.isSingleton(cvc)), node2)
 }
 
 predicate isLValue(Expr e) { any(AssignExpr assign).getDest() = e }
@@ -1146,6 +1148,8 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
     node2.(FlowSummaryNode).getSummaryNode())
+  or
+  captureReadStep(node1, any(Content::CapturedVariableContent cvc | c.isSingleton(cvc)), node2)
 }
 
 /**

From 4e1b44a05938dcf13e2c195772e27dcd984d646c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Mon, 11 Sep 2023 17:15:21 +0200
Subject: [PATCH 06/32] Swift: port simpleAstFlowStep/hasAliasedAccess

---
 .../dataflow/internal/DataFlowPrivate.qll     | 20 ++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index 7d0165436968..89e16b278630 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -798,9 +798,21 @@ private module OutNodes {
 
 import OutNodes
 
+/**
+ * Holds if there is a data flow step from `e1` to `e2` that only steps from
+ * child to parent in the AST.
+ */
+private predicate simpleAstFlowStep(Expr e1, Expr e2) {
+  e2.(IfExpr).getBranch(_) = e1
+  or
+  e2.(AssignExpr).getSource() = e1
+  or
+  e2.(ArrayExpr).getAnElement() = e1
+}
+
 private predicate closureFlowStep(Expr e1, Expr e2) {
-  // simpleLocalFlowStep(exprNode(e1), exprNode(e2)) // TODO: find out why the java version uses simpleAstFlowStep... probably due to non-monotonic recursion
-  // or
+  simpleAstFlowStep(e1, e2)
+  or
   exists(Ssa::WriteDefinition def |
     def.getARead().getNode().asAstNode() = e2 and
     def.assigns(any(CfgNode cfg | cfg.getNode().asAstNode() = e1))
@@ -892,9 +904,7 @@ private module CaptureInput implements VariableCapture::InputSig {
 
     predicate hasBody(Callable body) { this = body }
 
-    predicate hasAliasedAccess(Expr f) {
-      closureFlowStep+(this, f) and not closureFlowStep(f, _)
-    /* TODO: understand why this is intra-procedural */ }
+    predicate hasAliasedAccess(Expr f) { closureFlowStep+(this, f) and not closureFlowStep(f, _) }
   }
 
   class Callable extends S::Callable {

From 8115774a7a19fd237c59064de523ba1b9781a0ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Mon, 11 Sep 2023 17:42:13 +0200
Subject: [PATCH 07/32] Swift: Add the capture flow step as part of the normal
 data flow relation

TODO: see if we need to exclude duplicate SSA steps
---
 .../ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll  | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index 89e16b278630..ea839ef1818a 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -291,6 +291,9 @@ private module Cached {
     // flow through a flow summary (extension of `SummaryModelCsv`)
     FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom.(FlowSummaryNode).getSummaryNode(),
       nodeTo.(FlowSummaryNode).getSummaryNode(), true)
+    or
+    // flow step according to the closure capture library
+    captureValueStep(nodeFrom, nodeTo)
   }
 
   /**

From af49a3aa648f1c2a0e52db736415a681ea91d833 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Mon, 11 Sep 2023 18:31:08 +0200
Subject: [PATCH 08/32] Swift: accept new results in old tests

---
 .../test/library-tests/dataflow/taint/libraries/int.swift  | 2 +-
 .../Security/CWE-134/UncontrolledFormatString.expected     | 7 +++++++
 .../Security/CWE-134/UncontrolledFormatString.swift        | 2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/int.swift b/swift/ql/test/library-tests/dataflow/taint/libraries/int.swift
index 45b6f7515878..07ffec6355ef 100644
--- a/swift/ql/test/library-tests/dataflow/taint/libraries/int.swift
+++ b/swift/ql/test/library-tests/dataflow/taint/libraries/int.swift
@@ -147,7 +147,7 @@ func taintCollections(array: inout Array<Int>, contiguousArray: inout Contiguous
     sink(arg: buffer) // $ tainted=142
     sink(arg: buffer[0]) // $ tainted=142
     sink(arg: array)
-    sink(arg: array[0]) // $ MISSING: tainted=142
+    sink(arg: array[0]) // $ tainted=142
   })
 
   contiguousArray[0] = source2()
diff --git a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
index 3a2d4eb80c6a..7d03fb6562e3 100644
--- a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
+++ b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
@@ -1,4 +1,5 @@
 edges
+| UncontrolledFormatString.swift:57:12:57:22 | format | UncontrolledFormatString.swift:59:16:59:16 | format |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:70:28:70:28 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:73:28:73:28 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:74:28:74:28 | tainted |
@@ -11,12 +12,16 @@ edges
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:84:54:84:54 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:85:72:85:72 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:88:11:88:11 | tainted |
+| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:89:11:89:11 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:91:61:91:61 | tainted |
 | UncontrolledFormatString.swift:81:47:81:47 | tainted | UncontrolledFormatString.swift:81:30:81:54 | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:82:65:82:65 | tainted | UncontrolledFormatString.swift:82:48:82:72 | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:84:54:84:54 | tainted | UncontrolledFormatString.swift:84:37:84:61 | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:85:72:85:72 | tainted | UncontrolledFormatString.swift:85:55:85:79 | call to NSString.init(string:) |
+| UncontrolledFormatString.swift:89:11:89:11 | tainted | UncontrolledFormatString.swift:57:12:57:22 | format |
 nodes
+| UncontrolledFormatString.swift:57:12:57:22 | format | semmle.label | format |
+| UncontrolledFormatString.swift:59:16:59:16 | format | semmle.label | format |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | semmle.label | call to String.init(contentsOf:) |
 | UncontrolledFormatString.swift:70:28:70:28 | tainted | semmle.label | tainted |
 | UncontrolledFormatString.swift:73:28:73:28 | tainted | semmle.label | tainted |
@@ -34,9 +39,11 @@ nodes
 | UncontrolledFormatString.swift:85:55:85:79 | call to NSString.init(string:) | semmle.label | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:85:72:85:72 | tainted | semmle.label | tainted |
 | UncontrolledFormatString.swift:88:11:88:11 | tainted | semmle.label | tainted |
+| UncontrolledFormatString.swift:89:11:89:11 | tainted | semmle.label | tainted |
 | UncontrolledFormatString.swift:91:61:91:61 | tainted | semmle.label | tainted |
 subpaths
 #select
+| UncontrolledFormatString.swift:59:16:59:16 | format | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:59:16:59:16 | format | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
 | UncontrolledFormatString.swift:70:28:70:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:70:28:70:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
 | UncontrolledFormatString.swift:73:28:73:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:73:28:73:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
 | UncontrolledFormatString.swift:74:28:74:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:74:28:74:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
diff --git a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift
index e5665eedeac7..05fc1cb25648 100644
--- a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift
+++ b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift
@@ -56,7 +56,7 @@ func getVaList(_ args: [CVarArg]) -> CVaListPointer { return (nil as CVaListPoin
 
 func MyLog(_ format: String, _ args: CVarArg...) {
     withVaList(args) { arglist in
-        NSLogv(format, arglist) // BAD [NOT DETECTED]
+        NSLogv(format, arglist) // BAD
     }
 }
 

From 5418d39a0dfb5f82bb91f7ef847bf181e7306efc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Thu, 14 Sep 2023 16:46:23 +0200
Subject: [PATCH 09/32] Swift: add and accept a few new simple test cases

---
 .../ql/test/TestUtilities/InlineFlowTest.qll  |  8 +++---
 .../dataflow/capture/closures.swift           | 25 ++++++++++++++++---
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/swift/ql/test/TestUtilities/InlineFlowTest.qll b/swift/ql/test/TestUtilities/InlineFlowTest.qll
index 5ddad0abab66..4ab2a7cdf38f 100644
--- a/swift/ql/test/TestUtilities/InlineFlowTest.qll
+++ b/swift/ql/test/TestUtilities/InlineFlowTest.qll
@@ -42,12 +42,12 @@ import codeql.swift.dataflow.TaintTracking
 import TestUtilities.InlineExpectationsTest
 
 private predicate defaultSource(DataFlow::Node source) {
-  source.asExpr().(MethodCallExpr).getStaticTarget().getShortName() = ["source", "taint"]
+  source.asExpr().(CallExpr).getStaticTarget().(Function).getShortName() = ["source", "taint"]
 }
 
 private predicate defaultSink(DataFlow::Node sink) {
-  exists(MethodCallExpr ma | ma.getStaticTarget().getShortName() = "sink" |
-    sink.asExpr() = ma.getAnArgument().getExpr()
+  exists(CallExpr ca | ca.getStaticTarget().(Function).getShortName() = "sink" |
+    sink.asExpr() = ca.getAnArgument().getExpr()
   )
 }
 
@@ -67,7 +67,7 @@ private module NoFlowConfig implements DataFlow::ConfigSig {
 
 private string getSourceArgString(DataFlow::Node src) {
   defaultSource(src) and
-  src.asExpr().(MethodCallExpr).getAnArgument().getExpr().(StringLiteralExpr).getValue() = result
+  src.asExpr().(CallExpr).getAnArgument().getExpr().(StringLiteralExpr).getValue() = result
 }
 
 module FlowTest<DataFlow::ConfigSig ValueFlowConfig, DataFlow::ConfigSig TaintFlowConfig> {
diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index 8260782e129c..cb99f9fbbd5e 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -18,7 +18,7 @@ func captureList() {
 var escape: (() -> Int)? = nil
 
 func setEscape() {
-  var x = source("setEscape", 0)
+  let x = source("setEscape", 0)
   escape = {
     sink(x) // $ MISSING: hasValueFlow=setEscape
     return x + 1
@@ -31,11 +31,15 @@ func callEscape() {
 }
 
 func logical() -> Bool {
-  let f: ((Int) -> Int)? = { x in x + 1 }
+  let f: ((Int) -> Int)? = { x in
+    sink(x) // $ hasValueFlow=logical
+    return x + 1
+  }
+
   let x: Int? = source("logical", 42)
   return f != nil
       && (x != nil
-          && f!(x!) == 43) // $ MISSING: hasValueFlow=logical
+          && f!(x!) == 43)
 }
 
 func asyncTest() {
@@ -121,6 +125,21 @@ func sharedCaptureMultipleWriters() {
   callSink2()
 }
 
+func taintCollections(array: inout Array<Int>) {
+  array[0] = source("array", 0)
+  sink(array)
+  sink(array[0]) // $ hasValueFlow=array
+  array.withContiguousStorageIfAvailable({
+    buffer in
+    sink(array)
+    sink(array[0]) // $ hasValueFlow=array
+  })
+}
+
+func simplestTest() {
+  let x = source("simplestTest", 0)
+  sink(x) // $ hasValueFlow=simplestTest
+}
 
 func main() {
   print("captureList():")

From 9de3cc703ade9befdd3f37b861db57fe62a26537 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Mon, 25 Sep 2023 18:50:41 +0200
Subject: [PATCH 10/32] Swift: add CapturePostUpdateNode

However, this doesn't change any of the test results.
---
 .../swift/dataflow/internal/DataFlowPrivate.qll       | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index ea839ef1818a..a5769ce0c610 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -1256,6 +1256,17 @@ private module PostUpdateNodes {
         result.(FlowSummaryNode).getSummaryNode())
     }
   }
+
+  class CapturePostUpdateNode extends PostUpdateNodeImpl, CaptureNode {
+    private CaptureNode pre;
+
+    CapturePostUpdateNode() {
+      CaptureFlow::capturePostUpdateNode(this.getSynthesizedCaptureNode(),
+        pre.getSynthesizedCaptureNode())
+    }
+
+    override Node getPreUpdateNode() { result = pre }
+  }
 }
 
 private import PostUpdateNodes

From 9dbf7e818d5c4735602a571512ac8b529d47064a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= <d10c@users.noreply.github.com>
Date: Mon, 18 Sep 2023 20:34:53 +0200
Subject: [PATCH 11/32] Swift: align definition of InputSig slightly closer to
 Java version

Though there is a regression in the tests, so more work is needed.
---
 .../dataflow/internal/DataFlowPrivate.qll     | 42 +++++++++----------
 .../dataflow/capture/closures.swift           | 13 +++++-
 2 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index a5769ce0c610..593e28f9bedc 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -843,17 +843,20 @@ private module CaptureInput implements VariableCapture::InputSig {
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.(B::BasicBlock).getASuccessor() }
 
   //TODO: support capture of `this` in lambdas
-  class CapturedVariable instanceof S::CapturedDecl {
+  class CapturedVariable instanceof S::VarDecl {
+    CapturedVariable() {
+      any(S::CapturedDecl capturedDecl).getDecl() = this and
+      exists(this.getEnclosingCallable())
+    }
+
     string toString() { result = super.toString() }
 
-    Callable getCallable() { result = super.getScope() }
+    Callable getCallable() { result = super.getEnclosingCallable() }
 
     Location getLocation() { result = super.getLocation() }
   }
 
-  class CapturedParameter extends CapturedVariable {
-    CapturedParameter() { this.(S::CapturedDecl).getDecl() instanceof S::ParamDecl }
-  }
+  class CapturedParameter extends CapturedVariable instanceof S::ParamDecl { }
 
   class Expr instanceof S::AstNode {
     string toString() { result = super.toString() }
@@ -870,23 +873,16 @@ private module CaptureInput implements VariableCapture::InputSig {
     Expr source;
 
     VariableWrite() {
-      exists(S::VarDecl varDecl |
-        variable.(S::CapturedDecl).getDecl() = varDecl and
-        variable.getCallable() = this.(S::AstNode).getEnclosingCallable()
-      |
-        exists(S::Assignment a | this = a |
-          a.getDest().(DeclRefExpr).getDecl() = varDecl and
-          source = a.getSource()
-        )
-        or
-        exists(S::PatternBindingDecl pbd, S::NamedPattern np |
-          this = pbd and pbd.getAPattern() = np
-        |
-          np.getVarDecl() = varDecl and
-          source = np.getMatchingExpr()
-        )
-        // TODO: support multiple variables in LHS of =, in both of above cases.
+      exists(S::Assignment a | this = a |
+        a.getDest().(DeclRefExpr).getDecl() = variable and
+        source = a.getSource()
+      )
+      or
+      exists(S::PatternBindingDecl pbd, S::NamedPattern np | this = pbd and pbd.getAPattern() = np |
+        np.getVarDecl() = variable and
+        source = np.getMatchingExpr()
       )
+      // TODO: support multiple variables in LHS of =, in both of above cases.
     }
 
     CapturedVariable getVariable() { result = variable }
@@ -897,7 +893,7 @@ private module CaptureInput implements VariableCapture::InputSig {
   class VariableRead extends Expr instanceof S::DeclRefExpr {
     CapturedVariable v;
 
-    VariableRead() { this.getCapturedDecl() = v /* TODO: this should be an R-value only. */ }
+    VariableRead() { this.getDecl() = v /* TODO: this should be an R-value only. */ }
 
     CapturedVariable getVariable() { result = v }
   }
@@ -926,7 +922,7 @@ private CaptureFlow::ClosureNode asClosureNode(Node n) {
   result.(CaptureFlow::ExprNode).getExpr() = n.asExpr() or
   result.(CaptureFlow::ExprPostUpdateNode).getExpr() =
     n.(PostUpdateNode).getPreUpdateNode().asExpr() or
-  result.(CaptureFlow::ParameterNode).getParameter().(CapturedDecl).getDecl() = n.getParameter() or
+  result.(CaptureFlow::ParameterNode).getParameter() = n.getParameter() or
   result.(CaptureFlow::ThisParameterNode).getCallable().getSelfParam() = n.getParameter() or
   result.(CaptureFlow::MallocNode).getClosureExpr() = n.getCfgNode().getNode().asAstNode() // TODO: figure out why the java version had PostUpdateNode logic here
 }
diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index cb99f9fbbd5e..d72a337973b7 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -15,6 +15,17 @@ func captureList() {
   }()
 }
 
+func setAndCallEscape() {
+  let x = source("setAndCallEscape", 0)
+
+  let escape = {
+    sink(x) // $ MISSING: hasValueFlow=setAndCallEscape
+    return x + 1
+  }
+
+  sink(escape()) // $ MISSING: hasTaintFlow=setAndCallEscape
+}
+
 var escape: (() -> Int)? = nil
 
 func setEscape() {
@@ -132,7 +143,7 @@ func taintCollections(array: inout Array<Int>) {
   array.withContiguousStorageIfAvailable({
     buffer in
     sink(array)
-    sink(array[0]) // $ hasValueFlow=array
+    sink(array[0]) // $ MISSING: hasValueFlow=array
   })
 }
 

From 310ebe47b3544ba5d0069cb8a1c0205074d6559e Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 24 Oct 2023 15:26:27 +0100
Subject: [PATCH 12/32] Swift: Clean up test file.

---
 .../dataflow/capture/closures.swift           | 34 +------------------
 .../flowsources/FlowSourcesInline.expected    |  2 +-
 2 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index d72a337973b7..430aea6f5366 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -75,7 +75,7 @@ func foo() -> Int {
   let r = { x }
   sink(r()) // $ MISSING: hasValueFlow=foo
   f(1)
-  return r() // $ MISSING: hasTaintFlow=foo
+  return r()
 }
 
 func bar() -> () -> Int {
@@ -151,35 +151,3 @@ func simplestTest() {
   let x = source("simplestTest", 0)
   sink(x) // $ hasValueFlow=simplestTest
 }
-
-func main() {
-  print("captureList():")
-  captureList() // Hello world! 123
-
-  print("callEscape():")
-  callEscape() // 1
-
-  print("logical():", logical()) // true
-
-  print("asyncTest():")
-  asyncTest() // 42
-
-  print("foo():", foo()) // 42
-
-  let a = bar()
-  let b = baz()
-
-  print("bar():", a(), a()) // $ MISSING: hasTaintFlow=bar
-
-  print("baz():", b(), b()) // $ MISSING: hasTaintFlow=baz
-
-  g!(1)
-  print("g!(1):", b(), b()) // $ MISSING: hasTaintFlow=baz
-
-  print("sharedCapture():", sharedCapture()) // 4
-
-  print("sharedCaptureMultipleWriters():")
-  sharedCaptureMultipleWriters() // 42, -1
-}
-
-main()
diff --git a/swift/ql/test/library-tests/dataflow/flowsources/FlowSourcesInline.expected b/swift/ql/test/library-tests/dataflow/flowsources/FlowSourcesInline.expected
index 48de9172b362..8ec8033d086e 100644
--- a/swift/ql/test/library-tests/dataflow/flowsources/FlowSourcesInline.expected
+++ b/swift/ql/test/library-tests/dataflow/flowsources/FlowSourcesInline.expected
@@ -1,2 +1,2 @@
-failures
 testFailures
+failures

From 1c298e6001b34f4c22ce1eaf50420b6ae1a8961c Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 24 Oct 2023 15:23:00 +0100
Subject: [PATCH 13/32] Swift: Fix 'parameter' -> 'argument' flow into
 closures.

---
 .../lib/codeql/swift/controlflow/CfgNodes.qll |  2 +-
 .../dataflow/internal/DataFlowDispatch.qll    | 17 ++++++-
 .../dataflow/internal/DataFlowPrivate.qll     | 49 ++++++++++++++++---
 .../dataflow/capture/closures.swift           | 16 +++---
 .../dataflow/capture/inlinetest.expected      |  2 +-
 .../dataflow/dataflow/DataFlow.expected       | 10 ++++
 .../dataflow/dataflow/DataFlowInline.expected |  2 +-
 .../dataflow/dataflow/LocalFlow.expected      |  6 +++
 .../dataflow/dataflow/test.swift              |  2 +-
 .../taint/libraries/TaintInline.expected      |  2 +-
 .../dataflow/taint/libraries/int.swift        |  6 +--
 .../dataflow/taint/libraries/url.swift        |  2 +-
 12 files changed, 90 insertions(+), 26 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/controlflow/CfgNodes.qll b/swift/ql/lib/codeql/swift/controlflow/CfgNodes.qll
index b8467b098f2f..eab8385be366 100644
--- a/swift/ql/lib/codeql/swift/controlflow/CfgNodes.qll
+++ b/swift/ql/lib/codeql/swift/controlflow/CfgNodes.qll
@@ -173,7 +173,7 @@ class ApplyExprCfgNode extends ExprCfgNode {
 
   Callable getStaticTarget() { result = e.getStaticTarget() }
 
-  Expr getFunction() { result = e.getFunction() }
+  CfgNode getFunction() { result.getAst() = e.getFunction() }
 }
 
 class CallExprCfgNode extends ApplyExprCfgNode {
diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll
index c8ecc47e0fb5..933f32a61732 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll
@@ -293,12 +293,14 @@ private module Cached {
   newtype TArgumentPosition =
     TThisArgument() or
     // we rely on default exprs generated in the caller for ordering
-    TPositionalArgument(int n) { n = any(Argument arg).getIndex() }
+    TPositionalArgument(int n) { n = any(Argument arg).getIndex() } or
+    TClosureSelfArgument()
 
   cached
   newtype TParameterPosition =
     TThisParameter() or
-    TPositionalParameter(int n) { n = any(Argument arg).getIndex() }
+    TPositionalParameter(int n) { n = any(Argument arg).getIndex() } or
+    TClosureSelfParameter()
 }
 
 import Cached
@@ -331,6 +333,10 @@ class ThisParameterPosition extends ParameterPosition, TThisParameter {
   override string toString() { result = "this" }
 }
 
+class ClosureSelfParameter extends ParameterPosition, TClosureSelfParameter {
+  override string toString() { result = "{ ... }" }
+}
+
 /** An argument position. */
 class ArgumentPosition extends TArgumentPosition {
   /** Gets a textual representation of this position. */
@@ -347,6 +353,10 @@ class ThisArgumentPosition extends ArgumentPosition, TThisArgument {
   override string toString() { result = "this" }
 }
 
+class ClosureSelfArgument extends ArgumentPosition, TClosureSelfArgument {
+  override string toString() { result = "{ ... }" }
+}
+
 /** Holds if arguments at position `apos` match parameters at position `ppos`. */
 pragma[inline]
 predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
@@ -354,4 +364,7 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
   apos instanceof TThisArgument
   or
   ppos.(PositionalParameterPosition).getIndex() = apos.(PositionalArgumentPosition).getIndex()
+  or
+  ppos instanceof TClosureSelfParameter and
+  apos instanceof TClosureSelfArgument
 }
diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index 593e28f9bedc..a7117dadb13c 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -382,6 +382,12 @@ private class DictionarySubscriptNode extends NodeImpl, TDictionarySubscriptNode
   SubscriptExpr getExpr() { result = expr }
 }
 
+private class ClosureSelfReferenceNode extends ExprNodeImpl {
+  override ClosureExpr expr;
+
+  ClosureExpr getClosure() { result = expr }
+}
+
 private module ParameterNodes {
   abstract class ParameterNodeImpl extends NodeImpl {
     predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { none() }
@@ -412,6 +418,13 @@ private module ParameterNodes {
     override ParamDecl getParameter() { result = param }
   }
 
+  class ClosureSelfParameterNode extends ParameterNodeImpl, ClosureSelfReferenceNode {
+    override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
+      c.asSourceCallable() = this.getClosure() and
+      pos instanceof ClosureSelfParameter
+    }
+  }
+
   class SummaryParameterNode extends ParameterNodeImpl, FlowSummaryNode {
     SummaryParameterNode() {
       FlowSummaryImpl::Private::summaryParameterNode(this.getSummaryNode(), _)
@@ -626,6 +639,18 @@ private module ArgumentNodes {
       pos = TPositionalArgument(0)
     }
   }
+
+  class SelfClosureArgumentNode extends ExprNode, ArgumentNode {
+    ApplyExprCfgNode apply;
+
+    SelfClosureArgumentNode() { n = apply.getFunction() }
+
+    override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+      apply = call.asCall() and
+      not exists(apply.getStaticTarget()) and
+      pos instanceof ClosureSelfArgument
+    }
+  }
 }
 
 import ArgumentNodes
@@ -878,8 +903,8 @@ private module CaptureInput implements VariableCapture::InputSig {
         source = a.getSource()
       )
       or
-      exists(S::PatternBindingDecl pbd, S::NamedPattern np | this = pbd and pbd.getAPattern() = np |
-        np.getVarDecl() = variable and
+      exists(S::NamedPattern np | this = np |
+        variable = np.getVarDecl() and
         source = np.getMatchingExpr()
       )
       // TODO: support multiple variables in LHS of =, in both of above cases.
@@ -918,13 +943,23 @@ class CapturedParameter = CaptureInput::CapturedParameter;
 module CaptureFlow = VariableCapture::Flow<CaptureInput>;
 
 private CaptureFlow::ClosureNode asClosureNode(Node n) {
-  result = n.(CaptureNode).getSynthesizedCaptureNode() or
-  result.(CaptureFlow::ExprNode).getExpr() = n.asExpr() or
+  result = n.(CaptureNode).getSynthesizedCaptureNode()
+  or
+  result.(CaptureFlow::ExprNode).getExpr() = n.asExpr()
+  or
   result.(CaptureFlow::ExprPostUpdateNode).getExpr() =
-    n.(PostUpdateNode).getPreUpdateNode().asExpr() or
-  result.(CaptureFlow::ParameterNode).getParameter() = n.getParameter() or
-  result.(CaptureFlow::ThisParameterNode).getCallable().getSelfParam() = n.getParameter() or
+    n.(PostUpdateNode).getPreUpdateNode().asExpr()
+  or
+  result.(CaptureFlow::ParameterNode).getParameter() = n.getParameter()
+  or
+  result.(CaptureFlow::ThisParameterNode).getCallable() = n.(ClosureSelfParameterNode).getClosure()
+  or
   result.(CaptureFlow::MallocNode).getClosureExpr() = n.getCfgNode().getNode().asAstNode() // TODO: figure out why the java version had PostUpdateNode logic here
+  or
+  exists(CaptureInput::VariableWrite write |
+    result.(CaptureFlow::VariableWriteSourceNode).getVariableWrite() = write and
+    n.asExpr() = write.getSource()
+  )
 }
 
 private predicate captureStoreStep(Node node1, Content::CapturedVariableContent c, Node node2) {
diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index 430aea6f5366..dc9edfe052ee 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -11,7 +11,7 @@ func captureList() {
   let y: Int = source("captureList", 123);
   { [x = hello()] () in
      sink(x) // $ MISSING: hasValueFlow=hello
-     sink(y) // $ MISSING: hasValueFlow=captureList
+     sink(y) // $ hasValueFlow=captureList
   }()
 }
 
@@ -19,11 +19,11 @@ func setAndCallEscape() {
   let x = source("setAndCallEscape", 0)
 
   let escape = {
-    sink(x) // $ MISSING: hasValueFlow=setAndCallEscape
+    sink(x) // $ hasValueFlow=setAndCallEscape
     return x + 1
   }
 
-  sink(escape()) // $ MISSING: hasTaintFlow=setAndCallEscape
+  sink(escape()) // $ hasTaintFlow=setAndCallEscape
 }
 
 var escape: (() -> Int)? = nil
@@ -31,7 +31,7 @@ var escape: (() -> Int)? = nil
 func setEscape() {
   let x = source("setEscape", 0)
   escape = {
-    sink(x) // $ MISSING: hasValueFlow=setEscape
+    sink(x) // $ hasValueFlow=setEscape
     return x + 1
   }
 }
@@ -73,7 +73,7 @@ func foo() -> Int {
   let f = { y in x += y }
   x = source("foo", 41)
   let r = { x }
-  sink(r()) // $ MISSING: hasValueFlow=foo
+  sink(r()) // $ hasValueFlow=foo
   f(1)
   return r()
 }
@@ -138,12 +138,12 @@ func sharedCaptureMultipleWriters() {
 
 func taintCollections(array: inout Array<Int>) {
   array[0] = source("array", 0)
-  sink(array)
+  sink(array) // $ hasTaintFlow=array
   sink(array[0]) // $ hasValueFlow=array
   array.withContiguousStorageIfAvailable({
     buffer in
-    sink(array)
-    sink(array[0]) // $ MISSING: hasValueFlow=array
+    sink(array) // $ hasTaintFlow=array
+    sink(array[0]) // $ hasValueFlow=array
   })
 }
 
diff --git a/swift/ql/test/library-tests/dataflow/capture/inlinetest.expected b/swift/ql/test/library-tests/dataflow/capture/inlinetest.expected
index 48de9172b362..8ec8033d086e 100644
--- a/swift/ql/test/library-tests/dataflow/capture/inlinetest.expected
+++ b/swift/ql/test/library-tests/dataflow/capture/inlinetest.expected
@@ -1,2 +1,2 @@
-failures
 testFailures
+failures
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected b/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected
index c12038c26cfb..e5cf7620719f 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected
+++ b/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected
@@ -47,6 +47,7 @@ edges
 | test.swift:113:14:113:19 | arg | test.swift:114:19:114:19 | arg |
 | test.swift:113:14:113:19 | arg | test.swift:114:19:114:19 | arg |
 | test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg |
+| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg |
 | test.swift:114:19:114:19 | arg | test.swift:114:12:114:22 | call to ... |
 | test.swift:114:19:114:19 | arg | test.swift:114:12:114:22 | call to ... |
 | test.swift:114:19:114:19 | arg | test.swift:123:10:123:13 | i |
@@ -58,6 +59,9 @@ edges
 | test.swift:122:31:122:38 | call to source() | test.swift:113:14:113:19 | arg |
 | test.swift:122:31:122:38 | call to source() | test.swift:122:18:125:6 | call to forward(arg:lambda:) |
 | test.swift:123:10:123:13 | i | test.swift:124:16:124:16 | i |
+| test.swift:128:22:131:6 | call to forward(arg:lambda:) | test.swift:132:15:132:15 | clean |
+| test.swift:128:35:128:42 | call to source() | test.swift:113:14:113:19 | arg |
+| test.swift:128:35:128:42 | call to source() | test.swift:128:22:131:6 | call to forward(arg:lambda:) |
 | test.swift:142:10:142:13 | i | test.swift:143:16:143:16 | i |
 | test.swift:145:23:145:30 | call to source() | test.swift:142:10:142:13 | i |
 | test.swift:145:23:145:30 | call to source() | test.swift:145:15:145:31 | call to ... |
@@ -693,6 +697,9 @@ nodes
 | test.swift:123:10:123:13 | i | semmle.label | i |
 | test.swift:124:16:124:16 | i | semmle.label | i |
 | test.swift:126:15:126:15 | z | semmle.label | z |
+| test.swift:128:22:131:6 | call to forward(arg:lambda:) | semmle.label | call to forward(arg:lambda:) |
+| test.swift:128:35:128:42 | call to source() | semmle.label | call to source() |
+| test.swift:132:15:132:15 | clean | semmle.label | clean |
 | test.swift:138:19:138:26 | call to source() | semmle.label | call to source() |
 | test.swift:142:10:142:13 | i | semmle.label | i |
 | test.swift:143:16:143:16 | i | semmle.label | i |
@@ -1257,9 +1264,11 @@ nodes
 subpaths
 | test.swift:75:22:75:22 | x | test.swift:65:16:65:28 | arg1 | test.swift:65:1:70:1 | arg2[return] | test.swift:75:32:75:32 | [post] y |
 | test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg | test.swift:110:12:110:12 | arg | test.swift:114:12:114:22 | call to ... |
+| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg | test.swift:110:12:110:12 | arg | test.swift:114:12:114:22 | call to ... |
 | test.swift:114:19:114:19 | arg | test.swift:123:10:123:13 | i | test.swift:124:16:124:16 | i | test.swift:114:12:114:22 | call to ... |
 | test.swift:119:31:119:31 | x | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:119:18:119:44 | call to forward(arg:lambda:) |
 | test.swift:122:31:122:38 | call to source() | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:122:18:125:6 | call to forward(arg:lambda:) |
+| test.swift:128:35:128:42 | call to source() | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:128:22:131:6 | call to forward(arg:lambda:) |
 | test.swift:145:23:145:30 | call to source() | test.swift:142:10:142:13 | i | test.swift:143:16:143:16 | i | test.swift:145:15:145:31 | call to ... |
 | test.swift:170:9:170:9 | value | test.swift:163:7:163:7 | value | file://:0:0:0:0 | [post] self [x] | test.swift:170:5:170:5 | [post] self [x] |
 | test.swift:174:12:174:12 | self [x] | test.swift:163:7:163:7 | self [x] | file://:0:0:0:0 | .x | test.swift:174:12:174:12 | .x |
@@ -1338,6 +1347,7 @@ subpaths
 | test.swift:105:19:105:19 | x | test.swift:89:15:89:22 | call to source() | test.swift:105:19:105:19 | x | result |
 | test.swift:120:15:120:15 | y | test.swift:118:18:118:25 | call to source() | test.swift:120:15:120:15 | y | result |
 | test.swift:126:15:126:15 | z | test.swift:122:31:122:38 | call to source() | test.swift:126:15:126:15 | z | result |
+| test.swift:132:15:132:15 | clean | test.swift:128:35:128:42 | call to source() | test.swift:132:15:132:15 | clean | result |
 | test.swift:138:19:138:26 | call to source() | test.swift:138:19:138:26 | call to source() | test.swift:138:19:138:26 | call to source() | result |
 | test.swift:145:15:145:31 | call to ... | test.swift:145:23:145:30 | call to source() | test.swift:145:15:145:31 | call to ... | result |
 | test.swift:151:15:151:28 | call to ... | test.swift:149:16:149:23 | call to source() | test.swift:151:15:151:28 | call to ... | result |
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/DataFlowInline.expected b/swift/ql/test/library-tests/dataflow/dataflow/DataFlowInline.expected
index 48de9172b362..8ec8033d086e 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/DataFlowInline.expected
+++ b/swift/ql/test/library-tests/dataflow/dataflow/DataFlowInline.expected
@@ -1,2 +1,2 @@
-failures
 testFailures
+failures
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected b/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
index 5ec7f6dc2aad..472723cc7379 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
+++ b/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
@@ -1131,12 +1131,18 @@
 | test.swift:888:9:888:9 | SSA def(stream) | test.swift:898:24:898:24 | stream |
 | test.swift:888:9:888:9 | stream | test.swift:888:9:888:9 | SSA def(stream) |
 | test.swift:888:18:896:6 | call to AsyncStream<Element>.init(_:bufferingPolicy:_:) | test.swift:888:9:888:9 | stream |
+| test.swift:889:9:889:9 | continuation | test.swift:890:27:895:13 | continuation |
+| test.swift:890:27:895:13 | { ... } | test.swift:891:17:891:17 | phi(this) |
 | test.swift:891:17:891:17 | $generator | test.swift:891:17:891:17 | &... |
 | test.swift:891:17:891:17 | &... | test.swift:891:17:891:17 | $generator |
 | test.swift:891:17:891:17 | [post] $generator | test.swift:891:17:891:17 | &... |
+| test.swift:891:17:891:17 | phi(this) | test.swift:892:21:892:21 | this |
+| test.swift:891:17:891:17 | phi(this) | test.swift:894:17:894:17 | this |
 | test.swift:891:26:891:26 | $generator | test.swift:891:26:891:26 | SSA def($generator) |
 | test.swift:891:26:891:26 | SSA def($generator) | test.swift:891:17:891:17 | $generator |
 | test.swift:891:26:891:30 | call to makeIterator() | test.swift:891:26:891:26 | $generator |
+| test.swift:892:21:892:21 | this | test.swift:891:17:891:17 | phi(this) |
+| test.swift:892:21:892:21 | this | test.swift:891:17:891:17 | phi(this) |
 | test.swift:898:5:898:5 | $i$generator | test.swift:898:5:898:5 | &... |
 | test.swift:898:5:898:5 | &... | test.swift:898:5:898:5 | $i$generator |
 | test.swift:898:5:898:5 | [post] $i$generator | test.swift:898:5:898:5 | &... |
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/test.swift b/swift/ql/test/library-tests/dataflow/dataflow/test.swift
index 26dcb621ebb4..9a6023f99726 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/test.swift
+++ b/swift/ql/test/library-tests/dataflow/dataflow/test.swift
@@ -129,7 +129,7 @@ func forwarder() {
         (i: Int) -> Int in
         return 0
     })
-    sink(arg: clean)
+    sink(arg: clean) // $ SPURIOUS: flow=128
 }
 
 func lambdaFlows() {
diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/TaintInline.expected b/swift/ql/test/library-tests/dataflow/taint/libraries/TaintInline.expected
index 48de9172b362..8ec8033d086e 100644
--- a/swift/ql/test/library-tests/dataflow/taint/libraries/TaintInline.expected
+++ b/swift/ql/test/library-tests/dataflow/taint/libraries/TaintInline.expected
@@ -1,2 +1,2 @@
-failures
 testFailures
+failures
diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/int.swift b/swift/ql/test/library-tests/dataflow/taint/libraries/int.swift
index 07ffec6355ef..3ea65132e870 100644
--- a/swift/ql/test/library-tests/dataflow/taint/libraries/int.swift
+++ b/swift/ql/test/library-tests/dataflow/taint/libraries/int.swift
@@ -103,8 +103,8 @@ func taintThroughMutablePointer() {
     ptr4 in
     let return5 = myArray5.withUnsafeBytes({
       ptr5 in
-      sink(arg: ptr5)
-      sink(arg: ptr5[0]) // $ MISSING: tainted=97
+      sink(arg: ptr5) // $ tainted=97
+      sink(arg: ptr5[0]) // $ tainted=97
       ptr4.copyBytes(from: ptr5)
       sink(arg: ptr4)
       sink(arg: ptr4[0]) // $ MISSING: tainted=97
@@ -146,7 +146,7 @@ func taintCollections(array: inout Array<Int>, contiguousArray: inout Contiguous
     buffer in
     sink(arg: buffer) // $ tainted=142
     sink(arg: buffer[0]) // $ tainted=142
-    sink(arg: array)
+    sink(arg: array) // $ tainted=142
     sink(arg: array[0]) // $ tainted=142
   })
 
diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift b/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift
index 694e98c81324..799eaa85807f 100644
--- a/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift
+++ b/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift
@@ -287,7 +287,7 @@ func taintThroughURL() {
 	let _ = clean.withCString({
 		ptrClean in
 		sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: nil))
-		sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: urlTainted)) // $ MISSING: tainted=210
+		sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: urlTainted)) // $ tainted=210
 	});
 	sink(arg: URL(fileURLWithFileSystemRepresentation: 0 as! UnsafePointer<Int8>, isDirectory: false, relativeTo: urlTainted)) // $ tainted=210
 	let _ = tainted.withCString({

From 3d5098aaebf3aefc4560f700ac69f5f1b8faf12f Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 24 Oct 2023 15:25:56 +0100
Subject: [PATCH 14/32] Swift: Add failing test.

---
 .../ql/test/library-tests/dataflow/capture/closures.swift  | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index dc9edfe052ee..8d3b7f3519ce 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -151,3 +151,10 @@ func simplestTest() {
   let x = source("simplestTest", 0)
   sink(x) // $ hasValueFlow=simplestTest
 }
+
+func sideEffects() {
+  var x = 0
+  var f = { () in x = source("sideEffects", 1) }
+  f()
+  sink(x) // $ MISSING: hasValueFlow=sideEffects
+}

From 56b49a4de3653c956f2fafc9557da102e6e9977f Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 24 Oct 2023 15:18:49 +0100
Subject: [PATCH 15/32] Swift: Add a closure flow step from the right-hand side
 of variable declarations to the underlying pattern.

---
 .../lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll | 7 +++++--
 .../ql/test/library-tests/dataflow/capture/closures.swift  | 2 +-
 .../library-tests/dataflow/dataflow/LocalFlow.expected     | 2 ++
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index a7117dadb13c..78eeb9ec569f 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -151,7 +151,8 @@ private module Cached {
         [
           any(Argument arg | modifiable(arg)).getExpr(), any(MemberRefExpr ref).getBase(),
           any(ApplyExpr apply).getQualifier(), any(TupleElementExpr te).getSubExpr(),
-          any(SubscriptExpr se).getBase()
+          any(SubscriptExpr se).getBase(),
+          any(ApplyExpr apply | not exists(apply.getStaticTarget())).getFunction()
         ])
     } or
     TDictionarySubscriptNode(SubscriptExpr e) {
@@ -838,13 +839,15 @@ private predicate simpleAstFlowStep(Expr e1, Expr e2) {
   e2.(ArrayExpr).getAnElement() = e1
 }
 
-private predicate closureFlowStep(Expr e1, Expr e2) {
+private predicate closureFlowStep(CaptureInput::Expr e1, CaptureInput::Expr e2) {
   simpleAstFlowStep(e1, e2)
   or
   exists(Ssa::WriteDefinition def |
     def.getARead().getNode().asAstNode() = e2 and
     def.assigns(any(CfgNode cfg | cfg.getNode().asAstNode() = e1))
   )
+  or
+  e2.(Pattern).getImmediateMatchingExpr() = e1
 }
 
 private module CaptureInput implements VariableCapture::InputSig {
diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index 8d3b7f3519ce..47dc5b5be7fe 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -156,5 +156,5 @@ func sideEffects() {
   var x = 0
   var f = { () in x = source("sideEffects", 1) }
   f()
-  sink(x) // $ MISSING: hasValueFlow=sideEffects
+  sink(x) // $ hasValueFlow=sideEffects
 }
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected b/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
index 472723cc7379..74eb04203723 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
+++ b/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
@@ -131,12 +131,14 @@
 | test.swift:147:9:147:9 | SSA def(lambdaSource) | test.swift:151:15:151:15 | lambdaSource |
 | test.swift:147:9:147:9 | lambdaSource | test.swift:147:9:147:9 | SSA def(lambdaSource) |
 | test.swift:147:24:150:5 | { ... } | test.swift:147:9:147:9 | lambdaSource |
+| test.swift:151:15:151:15 | [post] lambdaSource | test.swift:159:16:159:16 | lambdaSource |
 | test.swift:151:15:151:15 | lambdaSource | test.swift:159:16:159:16 | lambdaSource |
 | test.swift:153:9:153:9 | SSA def(lambdaSink) | test.swift:157:5:157:5 | lambdaSink |
 | test.swift:153:9:153:9 | lambdaSink | test.swift:153:9:153:9 | SSA def(lambdaSink) |
 | test.swift:153:22:156:5 | { ... } | test.swift:153:9:153:9 | lambdaSink |
 | test.swift:154:10:154:13 | SSA def(i) | test.swift:155:19:155:19 | i |
 | test.swift:154:10:154:13 | i | test.swift:154:10:154:13 | SSA def(i) |
+| test.swift:157:5:157:5 | [post] lambdaSink | test.swift:159:5:159:5 | lambdaSink |
 | test.swift:157:5:157:5 | lambdaSink | test.swift:159:5:159:5 | lambdaSink |
 | test.swift:162:7:162:7 | SSA def(self) | test.swift:162:7:162:7 | self[return] |
 | test.swift:162:7:162:7 | self | test.swift:162:7:162:7 | SSA def(self) |

From 6f37d7c374d63c91a8d5776e7ce7c35a21f47cff Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 24 Oct 2023 15:39:19 +0100
Subject: [PATCH 16/32] Swift: Accept changes in paths.

---
 .../Security/CWE-134/UncontrolledFormatString.expected    | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
index 7d03fb6562e3..dd2e31475b42 100644
--- a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
+++ b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
@@ -1,5 +1,8 @@
 edges
-| UncontrolledFormatString.swift:57:12:57:22 | format | UncontrolledFormatString.swift:59:16:59:16 | format |
+| UncontrolledFormatString.swift:57:12:57:22 | format | UncontrolledFormatString.swift:58:22:60:5 | format |
+| UncontrolledFormatString.swift:58:22:60:5 | format | UncontrolledFormatString.swift:58:22:60:5 | { ... } [format] |
+| UncontrolledFormatString.swift:58:22:60:5 | { ... } [format] | UncontrolledFormatString.swift:59:16:59:16 | this [format] |
+| UncontrolledFormatString.swift:59:16:59:16 | this [format] | UncontrolledFormatString.swift:59:16:59:16 | format |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:70:28:70:28 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:73:28:73:28 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:74:28:74:28 | tainted |
@@ -21,7 +24,10 @@ edges
 | UncontrolledFormatString.swift:89:11:89:11 | tainted | UncontrolledFormatString.swift:57:12:57:22 | format |
 nodes
 | UncontrolledFormatString.swift:57:12:57:22 | format | semmle.label | format |
+| UncontrolledFormatString.swift:58:22:60:5 | format | semmle.label | format |
+| UncontrolledFormatString.swift:58:22:60:5 | { ... } [format] | semmle.label | { ... } [format] |
 | UncontrolledFormatString.swift:59:16:59:16 | format | semmle.label | format |
+| UncontrolledFormatString.swift:59:16:59:16 | this [format] | semmle.label | this [format] |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | semmle.label | call to String.init(contentsOf:) |
 | UncontrolledFormatString.swift:70:28:70:28 | tainted | semmle.label | tainted |
 | UncontrolledFormatString.swift:73:28:73:28 | tainted | semmle.label | tainted |

From 862de152a1308d02ee43fa01f3cf764357303f7f Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 24 Oct 2023 15:45:17 +0100
Subject: [PATCH 17/32] Swift: Add required qldoc.

---
 .../ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll
index 07279d633937..e03760590ee3 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll
@@ -141,6 +141,10 @@ class CaptureNode extends Node, TCaptureNode {
 
   CaptureNode() { this = TCaptureNode(cn) }
 
+  /**
+   * Gets the underlying synthesized capture node that is created by the
+   * variable capture library.
+   */
   CaptureFlow::SynthesizedCaptureNode getSynthesizedCaptureNode() { result = cn }
 }
 
@@ -256,6 +260,7 @@ module Content {
 
     CapturedVariableContent() { this = TCapturedVariableContent(v) }
 
+    /** Gets the underlying captured variable. */
     CapturedVariable getVariable() { result = v }
 
     override string toString() { result = v.toString() }

From 78e08cf63ccadde6cdb7f478421460bc816dcf7a Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 25 Oct 2023 13:55:07 +0100
Subject: [PATCH 18/32] Swift: Remove irrelevant TODO.

---
 swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll | 1 -
 1 file changed, 1 deletion(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index 78eeb9ec569f..3374f8bfd081 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -870,7 +870,6 @@ private module CaptureInput implements VariableCapture::InputSig {
 
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.(B::BasicBlock).getASuccessor() }
 
-  //TODO: support capture of `this` in lambdas
   class CapturedVariable instanceof S::VarDecl {
     CapturedVariable() {
       any(S::CapturedDecl capturedDecl).getDecl() = this and

From 951b6beeb150f5b0db1e20a50ce52a2094d76894 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 25 Oct 2023 14:44:09 +0100
Subject: [PATCH 19/32] Swift: Untangle the confusion between 'getParameter'
 and 'asParameter'.

---
 .../swift/dataflow/internal/DataFlowPrivate.qll  | 16 ++++++++--------
 .../swift/dataflow/internal/DataFlowPublic.qll   |  6 +++---
 .../swift/frameworks/StandardLibrary/WebView.qll |  4 ++--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index 3374f8bfd081..c4e1ea86d116 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -163,9 +163,9 @@ private module Cached {
   private predicate localSsaFlowStepUseUse(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
     def.adjacentReadPair(nodeFrom.getCfgNode(), nodeTo.getCfgNode()) and
     (
-      nodeTo instanceof InoutReturnNode
+      nodeTo instanceof InoutReturnNodeImpl
       implies
-      nodeTo.(InoutReturnNode).getParameter() = def.getSourceVariable().asVarDecl()
+      nodeTo.(InoutReturnNodeImpl).getParameter() = def.getSourceVariable().asVarDecl()
     )
   }
 
@@ -180,7 +180,7 @@ private module Cached {
    * Holds if `nodeFrom` is a parameter node, and `nodeTo` is a corresponding SSA node.
    */
   private predicate localFlowSsaParamInput(Node nodeFrom, Node nodeTo) {
-    nodeTo = getParameterDefNode(nodeFrom.(ParameterNode).getParameter())
+    nodeTo = getParameterDefNode(nodeFrom.asParameter())
   }
 
   private predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
@@ -193,9 +193,9 @@ private module Cached {
       nodeFrom.asDefinition() = def and
       nodeTo.getCfgNode() = def.getAFirstRead() and
       (
-        nodeTo instanceof InoutReturnNode
+        nodeTo instanceof InoutReturnNodeImpl
         implies
-        nodeTo.(InoutReturnNode).getParameter() = def.getSourceVariable().asVarDecl()
+        nodeTo.(InoutReturnNodeImpl).getParameter() = def.getSourceVariable().asVarDecl()
       )
       or
       // use-use flow
@@ -394,7 +394,7 @@ private module ParameterNodes {
     predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { none() }
 
     /** Gets the parameter associated with this node, if any. */
-    override ParamDecl getParameter() { none() }
+    ParamDecl getParameter() { none() }
   }
 
   class SourceParameterNode extends ParameterNodeImpl, TSourceParameterNode {
@@ -700,7 +700,7 @@ private module ReturnNodes {
       result = TDataFlowFunc(param.getDeclaringFunction())
     }
 
-    override ParamDecl getParameter() { result = param }
+    ParamDecl getParameter() { result = param }
 
     override Location getLocationImpl() { result = exit.getLocation() }
 
@@ -952,7 +952,7 @@ private CaptureFlow::ClosureNode asClosureNode(Node n) {
   result.(CaptureFlow::ExprPostUpdateNode).getExpr() =
     n.(PostUpdateNode).getPreUpdateNode().asExpr()
   or
-  result.(CaptureFlow::ParameterNode).getParameter() = n.getParameter()
+  result.(CaptureFlow::ParameterNode).getParameter() = n.asParameter()
   or
   result.(CaptureFlow::ThisParameterNode).getCallable() = n.(ClosureSelfParameterNode).getClosure()
   or
diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll
index e03760590ee3..abbb400904a6 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll
@@ -55,7 +55,7 @@ class Node extends TNode {
   /**
    * Gets the parameter that corresponds to this node, if any.
    */
-  ParamDecl getParameter() { none() }
+  ParamDecl asParameter() { none() }
 }
 
 /**
@@ -101,7 +101,7 @@ class ParameterNode extends Node instanceof ParameterNodeImpl {
     result = this.(ParameterNodeImpl).getEnclosingCallable()
   }
 
-  override ParamDecl getParameter() { result = this.(ParameterNodeImpl).getParameter() }
+  override ParamDecl asParameter() { result = this.(ParameterNodeImpl).getParameter() }
 }
 
 /**
@@ -156,7 +156,7 @@ ExprNode exprNode(DataFlowExpr e) { result.asExpr() = e }
 /**
  * Gets the node corresponding to the value of parameter `p` at function entry.
  */
-ParameterNode parameterNode(ParamDecl p) { result.getParameter() = p }
+ParameterNode parameterNode(ParamDecl p) { result.asParameter() = p }
 
 /**
  * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
diff --git a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll
index b845ee811049..da54683029f5 100644
--- a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll
+++ b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll
@@ -66,7 +66,7 @@ private class WKNavigationDelegateSource extends RemoteFlowSource {
         ] and
       p.getDeclaringFunction() = f and
       p.getIndex() = 1 and
-      this.(DataFlow::ParameterNode).getParameter() = p
+      this.asParameter() = p
     )
   }
 
@@ -173,7 +173,7 @@ private class JsExportedSource extends RemoteFlowSource {
       base.getEnclosingDecl().asNominalTypeDecl() instanceof JsExportedProto and
       adopter.getEnclosingDecl().asNominalTypeDecl() instanceof JsExportedType
     |
-      this.(DataFlow::ParameterNode).getParameter().getDeclaringFunction() = adopter and
+      this.asParameter().getDeclaringFunction() = adopter and
       pragma[only_bind_out](adopter.getName()) = pragma[only_bind_out](base.getName())
     )
     or

From 11194e574ce9241126cac1e09cb7310583cc16cd Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 25 Oct 2023 15:46:10 +0100
Subject: [PATCH 20/32] Swift: Get rid of the unnecessary parameter/argument
 position for the closure. Instead, we can just reuse the 'this' parameter and
 argument.

---
 .../dataflow/internal/DataFlowDispatch.qll      | 17 ++---------------
 .../swift/dataflow/internal/DataFlowPrivate.qll |  4 ++--
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll
index 933f32a61732..c8ecc47e0fb5 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll
@@ -293,14 +293,12 @@ private module Cached {
   newtype TArgumentPosition =
     TThisArgument() or
     // we rely on default exprs generated in the caller for ordering
-    TPositionalArgument(int n) { n = any(Argument arg).getIndex() } or
-    TClosureSelfArgument()
+    TPositionalArgument(int n) { n = any(Argument arg).getIndex() }
 
   cached
   newtype TParameterPosition =
     TThisParameter() or
-    TPositionalParameter(int n) { n = any(Argument arg).getIndex() } or
-    TClosureSelfParameter()
+    TPositionalParameter(int n) { n = any(Argument arg).getIndex() }
 }
 
 import Cached
@@ -333,10 +331,6 @@ class ThisParameterPosition extends ParameterPosition, TThisParameter {
   override string toString() { result = "this" }
 }
 
-class ClosureSelfParameter extends ParameterPosition, TClosureSelfParameter {
-  override string toString() { result = "{ ... }" }
-}
-
 /** An argument position. */
 class ArgumentPosition extends TArgumentPosition {
   /** Gets a textual representation of this position. */
@@ -353,10 +347,6 @@ class ThisArgumentPosition extends ArgumentPosition, TThisArgument {
   override string toString() { result = "this" }
 }
 
-class ClosureSelfArgument extends ArgumentPosition, TClosureSelfArgument {
-  override string toString() { result = "{ ... }" }
-}
-
 /** Holds if arguments at position `apos` match parameters at position `ppos`. */
 pragma[inline]
 predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
@@ -364,7 +354,4 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
   apos instanceof TThisArgument
   or
   ppos.(PositionalParameterPosition).getIndex() = apos.(PositionalArgumentPosition).getIndex()
-  or
-  ppos instanceof TClosureSelfParameter and
-  apos instanceof TClosureSelfArgument
 }
diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index c4e1ea86d116..dc63f586d425 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -422,7 +422,7 @@ private module ParameterNodes {
   class ClosureSelfParameterNode extends ParameterNodeImpl, ClosureSelfReferenceNode {
     override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
       c.asSourceCallable() = this.getClosure() and
-      pos instanceof ClosureSelfParameter
+      pos instanceof TThisParameter
     }
   }
 
@@ -649,7 +649,7 @@ private module ArgumentNodes {
     override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
       apply = call.asCall() and
       not exists(apply.getStaticTarget()) and
-      pos instanceof ClosureSelfArgument
+      pos instanceof ThisArgumentPosition
     }
   }
 }

From 2465cc20f0108a9b80c85e438695cb1f43b2f3eb Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 26 Oct 2023 11:56:27 +0100
Subject: [PATCH 21/32] Swift: Don't define 'ClosureSelfParameterNode' as the
 expression node of the closure.

---
 .../dataflow/internal/DataFlowPrivate.qll     | 29 ++++++++++++-------
 .../dataflow/capture/closures.swift           |  4 +--
 .../dataflow/dataflow/DataFlow.expected       | 10 -------
 .../dataflow/dataflow/LocalFlow.expected      |  2 +-
 .../dataflow/dataflow/test.swift              |  2 +-
 .../dataflow/taint/core/TaintInline.expected  |  2 +-
 .../dataflow/taint/libraries/url.swift        |  2 +-
 7 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index dc63f586d425..d9e0e8e6ca6f 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -158,7 +158,8 @@ private module Cached {
     TDictionarySubscriptNode(SubscriptExpr e) {
       e.getBase().getType().getCanonicalType() instanceof CanonicalDictionaryType
     } or
-    TCaptureNode(CaptureFlow::SynthesizedCaptureNode cn)
+    TCaptureNode(CaptureFlow::SynthesizedCaptureNode cn) or
+    TClosureSelfParameterNode(ClosureExpr closure)
 
   private predicate localSsaFlowStepUseUse(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
     def.adjacentReadPair(nodeFrom.getCfgNode(), nodeTo.getCfgNode()) and
@@ -359,7 +360,9 @@ private predicate hasPatternNode(PatternCfgNode n, Pattern p) {
 import Cached
 
 /** Holds if `n` should be hidden from path explanations. */
-predicate nodeIsHidden(Node n) { n instanceof FlowSummaryNode }
+predicate nodeIsHidden(Node n) {
+  n instanceof FlowSummaryNode or n instanceof ClosureSelfParameterNode
+}
 
 /**
  * The intermediate node for a dictionary subscript operation `dict[key]`. In a write, this is used
@@ -383,12 +386,6 @@ private class DictionarySubscriptNode extends NodeImpl, TDictionarySubscriptNode
   SubscriptExpr getExpr() { result = expr }
 }
 
-private class ClosureSelfReferenceNode extends ExprNodeImpl {
-  override ClosureExpr expr;
-
-  ClosureExpr getClosure() { result = expr }
-}
-
 private module ParameterNodes {
   abstract class ParameterNodeImpl extends NodeImpl {
     predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { none() }
@@ -419,11 +416,23 @@ private module ParameterNodes {
     override ParamDecl getParameter() { result = param }
   }
 
-  class ClosureSelfParameterNode extends ParameterNodeImpl, ClosureSelfReferenceNode {
+  class ClosureSelfParameterNode extends ParameterNodeImpl, TClosureSelfParameterNode {
+    ClosureExpr closure;
+
+    ClosureSelfParameterNode() { this = TClosureSelfParameterNode(closure) }
+
     override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
-      c.asSourceCallable() = this.getClosure() and
+      c.asSourceCallable() = closure and
       pos instanceof TThisParameter
     }
+
+    override Location getLocationImpl() { result = closure.getLocation() }
+
+    override string toStringImpl() { result = "closure self parameter" }
+
+    override DataFlowCallable getEnclosingCallable() { this.isParameterOf(result, _) }
+
+    ClosureExpr getClosure() { result = closure }
   }
 
   class SummaryParameterNode extends ParameterNodeImpl, FlowSummaryNode {
diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index 47dc5b5be7fe..43c7e262dc30 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -11,7 +11,7 @@ func captureList() {
   let y: Int = source("captureList", 123);
   { [x = hello()] () in
      sink(x) // $ MISSING: hasValueFlow=hello
-     sink(y) // $ hasValueFlow=captureList
+     sink(y) // $ MISSING: hasValueFlow=captureList
   }()
 }
 
@@ -31,7 +31,7 @@ var escape: (() -> Int)? = nil
 func setEscape() {
   let x = source("setEscape", 0)
   escape = {
-    sink(x) // $ hasValueFlow=setEscape
+    sink(x) // $ MISSING: hasValueFlow=setEscape
     return x + 1
   }
 }
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected b/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected
index e5cf7620719f..c12038c26cfb 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected
+++ b/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected
@@ -47,7 +47,6 @@ edges
 | test.swift:113:14:113:19 | arg | test.swift:114:19:114:19 | arg |
 | test.swift:113:14:113:19 | arg | test.swift:114:19:114:19 | arg |
 | test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg |
-| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg |
 | test.swift:114:19:114:19 | arg | test.swift:114:12:114:22 | call to ... |
 | test.swift:114:19:114:19 | arg | test.swift:114:12:114:22 | call to ... |
 | test.swift:114:19:114:19 | arg | test.swift:123:10:123:13 | i |
@@ -59,9 +58,6 @@ edges
 | test.swift:122:31:122:38 | call to source() | test.swift:113:14:113:19 | arg |
 | test.swift:122:31:122:38 | call to source() | test.swift:122:18:125:6 | call to forward(arg:lambda:) |
 | test.swift:123:10:123:13 | i | test.swift:124:16:124:16 | i |
-| test.swift:128:22:131:6 | call to forward(arg:lambda:) | test.swift:132:15:132:15 | clean |
-| test.swift:128:35:128:42 | call to source() | test.swift:113:14:113:19 | arg |
-| test.swift:128:35:128:42 | call to source() | test.swift:128:22:131:6 | call to forward(arg:lambda:) |
 | test.swift:142:10:142:13 | i | test.swift:143:16:143:16 | i |
 | test.swift:145:23:145:30 | call to source() | test.swift:142:10:142:13 | i |
 | test.swift:145:23:145:30 | call to source() | test.swift:145:15:145:31 | call to ... |
@@ -697,9 +693,6 @@ nodes
 | test.swift:123:10:123:13 | i | semmle.label | i |
 | test.swift:124:16:124:16 | i | semmle.label | i |
 | test.swift:126:15:126:15 | z | semmle.label | z |
-| test.swift:128:22:131:6 | call to forward(arg:lambda:) | semmle.label | call to forward(arg:lambda:) |
-| test.swift:128:35:128:42 | call to source() | semmle.label | call to source() |
-| test.swift:132:15:132:15 | clean | semmle.label | clean |
 | test.swift:138:19:138:26 | call to source() | semmle.label | call to source() |
 | test.swift:142:10:142:13 | i | semmle.label | i |
 | test.swift:143:16:143:16 | i | semmle.label | i |
@@ -1264,11 +1257,9 @@ nodes
 subpaths
 | test.swift:75:22:75:22 | x | test.swift:65:16:65:28 | arg1 | test.swift:65:1:70:1 | arg2[return] | test.swift:75:32:75:32 | [post] y |
 | test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg | test.swift:110:12:110:12 | arg | test.swift:114:12:114:22 | call to ... |
-| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg | test.swift:110:12:110:12 | arg | test.swift:114:12:114:22 | call to ... |
 | test.swift:114:19:114:19 | arg | test.swift:123:10:123:13 | i | test.swift:124:16:124:16 | i | test.swift:114:12:114:22 | call to ... |
 | test.swift:119:31:119:31 | x | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:119:18:119:44 | call to forward(arg:lambda:) |
 | test.swift:122:31:122:38 | call to source() | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:122:18:125:6 | call to forward(arg:lambda:) |
-| test.swift:128:35:128:42 | call to source() | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:128:22:131:6 | call to forward(arg:lambda:) |
 | test.swift:145:23:145:30 | call to source() | test.swift:142:10:142:13 | i | test.swift:143:16:143:16 | i | test.swift:145:15:145:31 | call to ... |
 | test.swift:170:9:170:9 | value | test.swift:163:7:163:7 | value | file://:0:0:0:0 | [post] self [x] | test.swift:170:5:170:5 | [post] self [x] |
 | test.swift:174:12:174:12 | self [x] | test.swift:163:7:163:7 | self [x] | file://:0:0:0:0 | .x | test.swift:174:12:174:12 | .x |
@@ -1347,7 +1338,6 @@ subpaths
 | test.swift:105:19:105:19 | x | test.swift:89:15:89:22 | call to source() | test.swift:105:19:105:19 | x | result |
 | test.swift:120:15:120:15 | y | test.swift:118:18:118:25 | call to source() | test.swift:120:15:120:15 | y | result |
 | test.swift:126:15:126:15 | z | test.swift:122:31:122:38 | call to source() | test.swift:126:15:126:15 | z | result |
-| test.swift:132:15:132:15 | clean | test.swift:128:35:128:42 | call to source() | test.swift:132:15:132:15 | clean | result |
 | test.swift:138:19:138:26 | call to source() | test.swift:138:19:138:26 | call to source() | test.swift:138:19:138:26 | call to source() | result |
 | test.swift:145:15:145:31 | call to ... | test.swift:145:23:145:30 | call to source() | test.swift:145:15:145:31 | call to ... | result |
 | test.swift:151:15:151:28 | call to ... | test.swift:149:16:149:23 | call to source() | test.swift:151:15:151:28 | call to ... | result |
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected b/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
index 74eb04203723..8615571b864c 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
+++ b/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
@@ -1134,7 +1134,7 @@
 | test.swift:888:9:888:9 | stream | test.swift:888:9:888:9 | SSA def(stream) |
 | test.swift:888:18:896:6 | call to AsyncStream<Element>.init(_:bufferingPolicy:_:) | test.swift:888:9:888:9 | stream |
 | test.swift:889:9:889:9 | continuation | test.swift:890:27:895:13 | continuation |
-| test.swift:890:27:895:13 | { ... } | test.swift:891:17:891:17 | phi(this) |
+| test.swift:890:27:895:13 | closure self parameter | test.swift:891:17:891:17 | phi(this) |
 | test.swift:891:17:891:17 | $generator | test.swift:891:17:891:17 | &... |
 | test.swift:891:17:891:17 | &... | test.swift:891:17:891:17 | $generator |
 | test.swift:891:17:891:17 | [post] $generator | test.swift:891:17:891:17 | &... |
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/test.swift b/swift/ql/test/library-tests/dataflow/dataflow/test.swift
index 9a6023f99726..735d1490e9c3 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/test.swift
+++ b/swift/ql/test/library-tests/dataflow/dataflow/test.swift
@@ -129,7 +129,7 @@ func forwarder() {
         (i: Int) -> Int in
         return 0
     })
-    sink(arg: clean) // $ SPURIOUS: flow=128
+    sink(arg: clean) // clean
 }
 
 func lambdaFlows() {
diff --git a/swift/ql/test/library-tests/dataflow/taint/core/TaintInline.expected b/swift/ql/test/library-tests/dataflow/taint/core/TaintInline.expected
index 48de9172b362..8ec8033d086e 100644
--- a/swift/ql/test/library-tests/dataflow/taint/core/TaintInline.expected
+++ b/swift/ql/test/library-tests/dataflow/taint/core/TaintInline.expected
@@ -1,2 +1,2 @@
-failures
 testFailures
+failures
diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift b/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift
index 799eaa85807f..694e98c81324 100644
--- a/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift
+++ b/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift
@@ -287,7 +287,7 @@ func taintThroughURL() {
 	let _ = clean.withCString({
 		ptrClean in
 		sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: nil))
-		sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: urlTainted)) // $ tainted=210
+		sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: urlTainted)) // $ MISSING: tainted=210
 	});
 	sink(arg: URL(fileURLWithFileSystemRepresentation: 0 as! UnsafePointer<Int8>, isDirectory: false, relativeTo: urlTainted)) // $ tainted=210
 	let _ = tainted.withCString({

From 2ad121a8a571e2f926ffb782ff6997c9f999101c Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 26 Oct 2023 14:46:59 +0100
Subject: [PATCH 22/32] Swift: Simplify test.

---
 swift/ql/test/library-tests/dataflow/capture/closures.swift | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index 43c7e262dc30..953f2fc6c87a 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -97,7 +97,7 @@ func baz() -> () -> Int {
   return r
 }
 
-func sharedCapture() -> Int {
+func sharedCapture() {
   let (incrX, getX) = {
     var x = source("sharedCapture", 0)
     return ({ x += 1 }, { x })
@@ -111,8 +111,6 @@ func sharedCapture() -> Int {
   sink(getX()) // $ MISSING: hasValueFlow=sharedCapture
   doubleIncrX()
   sink(getX()) // $ MISSING: hasTaintFlow=sharedCapture
-  doubleIncrX()
-  return getX()
 }
 
 func sharedCaptureMultipleWriters() {

From 96a37f3a3c34eca44c4a36bfeff762e1770b8086 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 26 Oct 2023 14:55:17 +0100
Subject: [PATCH 23/32] Swift: Simplify more tests.

---
 .../dataflow/capture/closures.swift           | 23 ++-----------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index 953f2fc6c87a..36fac5bd8153 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -68,33 +68,14 @@ func asyncTest() {
   }
 }
 
-func foo() -> Int {
+func foo() {
   var x = 1
   let f = { y in x += y }
   x = source("foo", 41)
   let r = { x }
   sink(r()) // $ hasValueFlow=foo
   f(1)
-  return r()
-}
-
-func bar() -> () -> Int {
-  var x = 1
-  let f = { y in x += y }
-  x = source("bar", 41)
-  let r = { x }
-  f(1)
-  return r // constantly 42
-}
-
-var g: ((Int) -> Void)? = nil
-func baz() -> () -> Int {
-  var x = 1
-  g = { y in x += y }
-  x = source("baz", 41)
-  let r = { x }
-  g!(1)
-  return r
+  sink(r()) // $ hasValueFlow=foo
 }
 
 func sharedCapture() {

From 784bb72b3340070a3525a18e7947dfeb3562598b Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 26 Oct 2023 17:29:26 +0100
Subject: [PATCH 24/32] Swift: Add some more tests.

---
 .../dataflow/capture/closures.swift           | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index 36fac5bd8153..0634ec2e6b75 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -137,3 +137,40 @@ func sideEffects() {
   f()
   sink(x) // $ hasValueFlow=sideEffects
 }
+
+class S {
+  var bf1 = 0
+  var bf2 = 0
+  func captureOther() {
+    var other = S()
+    var f = { x in
+      other.bf1 = x;
+    };
+
+    // no flow
+    sink(bf1);
+    sink(other.bf1);
+    sink(other.bf2);
+
+    f(source("captureOther", 2));
+
+    sink(other.bf1); // $ hasValueFlow=captureOther
+    sink(other.bf2);
+  }
+
+  func captureThis() {
+    var f = { [self] x in
+      self.bf1 = x;
+      bf2 = x;
+    };
+
+    // no flow
+    sink(bf1);
+    sink(self.bf2);
+
+    f(source("captureThis", 2));
+
+    sink(bf1); // $ MISSING: hasValueFlow
+    sink(self.bf2); // $ MISSING: hasValueFlow
+  }
+}

From 63525a9d9efb0a1683dd7d10c3eb455e5b6c2231 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 26 Oct 2023 21:48:41 +0100
Subject: [PATCH 25/32] Swift: Delete one TODO (it has been converted to an
 internal issue) and fix another.

---
 .../ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll  | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index d9e0e8e6ca6f..c8a373b99e41 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -918,7 +918,6 @@ private module CaptureInput implements VariableCapture::InputSig {
         variable = np.getVarDecl() and
         source = np.getMatchingExpr()
       )
-      // TODO: support multiple variables in LHS of =, in both of above cases.
     }
 
     CapturedVariable getVariable() { result = variable }
@@ -929,7 +928,7 @@ private module CaptureInput implements VariableCapture::InputSig {
   class VariableRead extends Expr instanceof S::DeclRefExpr {
     CapturedVariable v;
 
-    VariableRead() { this.getDecl() = v /* TODO: this should be an R-value only. */ }
+    VariableRead() { this.getDecl() = v and not isLValue(this) }
 
     CapturedVariable getVariable() { result = v }
   }

From 9e2dd09ddc2986c878ce729cde34cb754d2e1d23 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Fri, 27 Oct 2023 10:20:07 +0100
Subject: [PATCH 26/32] Swift: Accept test regression (caused by no model for
 'withVaList').

---
 .../CWE-134/UncontrolledFormatString.expected       | 13 -------------
 .../Security/CWE-134/UncontrolledFormatString.swift |  2 +-
 2 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
index dd2e31475b42..3a2d4eb80c6a 100644
--- a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
+++ b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
@@ -1,8 +1,4 @@
 edges
-| UncontrolledFormatString.swift:57:12:57:22 | format | UncontrolledFormatString.swift:58:22:60:5 | format |
-| UncontrolledFormatString.swift:58:22:60:5 | format | UncontrolledFormatString.swift:58:22:60:5 | { ... } [format] |
-| UncontrolledFormatString.swift:58:22:60:5 | { ... } [format] | UncontrolledFormatString.swift:59:16:59:16 | this [format] |
-| UncontrolledFormatString.swift:59:16:59:16 | this [format] | UncontrolledFormatString.swift:59:16:59:16 | format |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:70:28:70:28 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:73:28:73:28 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:74:28:74:28 | tainted |
@@ -15,19 +11,12 @@ edges
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:84:54:84:54 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:85:72:85:72 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:88:11:88:11 | tainted |
-| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:89:11:89:11 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:91:61:91:61 | tainted |
 | UncontrolledFormatString.swift:81:47:81:47 | tainted | UncontrolledFormatString.swift:81:30:81:54 | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:82:65:82:65 | tainted | UncontrolledFormatString.swift:82:48:82:72 | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:84:54:84:54 | tainted | UncontrolledFormatString.swift:84:37:84:61 | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:85:72:85:72 | tainted | UncontrolledFormatString.swift:85:55:85:79 | call to NSString.init(string:) |
-| UncontrolledFormatString.swift:89:11:89:11 | tainted | UncontrolledFormatString.swift:57:12:57:22 | format |
 nodes
-| UncontrolledFormatString.swift:57:12:57:22 | format | semmle.label | format |
-| UncontrolledFormatString.swift:58:22:60:5 | format | semmle.label | format |
-| UncontrolledFormatString.swift:58:22:60:5 | { ... } [format] | semmle.label | { ... } [format] |
-| UncontrolledFormatString.swift:59:16:59:16 | format | semmle.label | format |
-| UncontrolledFormatString.swift:59:16:59:16 | this [format] | semmle.label | this [format] |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | semmle.label | call to String.init(contentsOf:) |
 | UncontrolledFormatString.swift:70:28:70:28 | tainted | semmle.label | tainted |
 | UncontrolledFormatString.swift:73:28:73:28 | tainted | semmle.label | tainted |
@@ -45,11 +34,9 @@ nodes
 | UncontrolledFormatString.swift:85:55:85:79 | call to NSString.init(string:) | semmle.label | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:85:72:85:72 | tainted | semmle.label | tainted |
 | UncontrolledFormatString.swift:88:11:88:11 | tainted | semmle.label | tainted |
-| UncontrolledFormatString.swift:89:11:89:11 | tainted | semmle.label | tainted |
 | UncontrolledFormatString.swift:91:61:91:61 | tainted | semmle.label | tainted |
 subpaths
 #select
-| UncontrolledFormatString.swift:59:16:59:16 | format | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:59:16:59:16 | format | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
 | UncontrolledFormatString.swift:70:28:70:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:70:28:70:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
 | UncontrolledFormatString.swift:73:28:73:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:73:28:73:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
 | UncontrolledFormatString.swift:74:28:74:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:74:28:74:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
diff --git a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift
index 05fc1cb25648..e5665eedeac7 100644
--- a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift
+++ b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift
@@ -56,7 +56,7 @@ func getVaList(_ args: [CVarArg]) -> CVaListPointer { return (nil as CVaListPoin
 
 func MyLog(_ format: String, _ args: CVarArg...) {
     withVaList(args) { arglist in
-        NSLogv(format, arglist) // BAD
+        NSLogv(format, arglist) // BAD [NOT DETECTED]
     }
 }
 

From 93234c0b5c61ef780f8e11bbc88b4d0e4cbe809f Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Fri, 27 Oct 2023 10:21:12 +0100
Subject: [PATCH 27/32] Swift: Add model for 'withVaList' and accept test
 changes.

---
 .../swift/frameworks/StandardLibrary/CInterop.qll   |  6 +++++-
 .../CWE-134/UncontrolledFormatString.expected       | 13 +++++++++++++
 .../Security/CWE-134/UncontrolledFormatString.swift |  2 +-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/CInterop.qll b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/CInterop.qll
index 35a7cafe1e94..6310512cabe5 100644
--- a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/CInterop.qll
+++ b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/CInterop.qll
@@ -7,6 +7,10 @@ private import codeql.swift.dataflow.ExternalFlow
 
 private class CInteropSummaries extends SummaryModelCsv {
   override predicate row(string row) {
-    row = ";;false;getVaList(_:);;;Argument[0].CollectionElement;ReturnValue;value"
+    row =
+      [
+        ";;false;getVaList(_:);;;Argument[0].CollectionElement;ReturnValue;value",
+        ";;false;withVaList(_:_:);;;Argument[0];Argument[1].Parameter[0];value"
+      ]
   }
 }
diff --git a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
index 3a2d4eb80c6a..dd2e31475b42 100644
--- a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
+++ b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected
@@ -1,4 +1,8 @@
 edges
+| UncontrolledFormatString.swift:57:12:57:22 | format | UncontrolledFormatString.swift:58:22:60:5 | format |
+| UncontrolledFormatString.swift:58:22:60:5 | format | UncontrolledFormatString.swift:58:22:60:5 | { ... } [format] |
+| UncontrolledFormatString.swift:58:22:60:5 | { ... } [format] | UncontrolledFormatString.swift:59:16:59:16 | this [format] |
+| UncontrolledFormatString.swift:59:16:59:16 | this [format] | UncontrolledFormatString.swift:59:16:59:16 | format |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:70:28:70:28 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:73:28:73:28 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:74:28:74:28 | tainted |
@@ -11,12 +15,19 @@ edges
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:84:54:84:54 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:85:72:85:72 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:88:11:88:11 | tainted |
+| UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:89:11:89:11 | tainted |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:91:61:91:61 | tainted |
 | UncontrolledFormatString.swift:81:47:81:47 | tainted | UncontrolledFormatString.swift:81:30:81:54 | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:82:65:82:65 | tainted | UncontrolledFormatString.swift:82:48:82:72 | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:84:54:84:54 | tainted | UncontrolledFormatString.swift:84:37:84:61 | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:85:72:85:72 | tainted | UncontrolledFormatString.swift:85:55:85:79 | call to NSString.init(string:) |
+| UncontrolledFormatString.swift:89:11:89:11 | tainted | UncontrolledFormatString.swift:57:12:57:22 | format |
 nodes
+| UncontrolledFormatString.swift:57:12:57:22 | format | semmle.label | format |
+| UncontrolledFormatString.swift:58:22:60:5 | format | semmle.label | format |
+| UncontrolledFormatString.swift:58:22:60:5 | { ... } [format] | semmle.label | { ... } [format] |
+| UncontrolledFormatString.swift:59:16:59:16 | format | semmle.label | format |
+| UncontrolledFormatString.swift:59:16:59:16 | this [format] | semmle.label | this [format] |
 | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | semmle.label | call to String.init(contentsOf:) |
 | UncontrolledFormatString.swift:70:28:70:28 | tainted | semmle.label | tainted |
 | UncontrolledFormatString.swift:73:28:73:28 | tainted | semmle.label | tainted |
@@ -34,9 +45,11 @@ nodes
 | UncontrolledFormatString.swift:85:55:85:79 | call to NSString.init(string:) | semmle.label | call to NSString.init(string:) |
 | UncontrolledFormatString.swift:85:72:85:72 | tainted | semmle.label | tainted |
 | UncontrolledFormatString.swift:88:11:88:11 | tainted | semmle.label | tainted |
+| UncontrolledFormatString.swift:89:11:89:11 | tainted | semmle.label | tainted |
 | UncontrolledFormatString.swift:91:61:91:61 | tainted | semmle.label | tainted |
 subpaths
 #select
+| UncontrolledFormatString.swift:59:16:59:16 | format | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:59:16:59:16 | format | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
 | UncontrolledFormatString.swift:70:28:70:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:70:28:70:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
 | UncontrolledFormatString.swift:73:28:73:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:73:28:73:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
 | UncontrolledFormatString.swift:74:28:74:28 | tainted | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:74:28:74:28 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:64:24:64:77 | call to String.init(contentsOf:) | this user-provided value |
diff --git a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift
index e5665eedeac7..05fc1cb25648 100644
--- a/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift
+++ b/swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift
@@ -56,7 +56,7 @@ func getVaList(_ args: [CVarArg]) -> CVaListPointer { return (nil as CVaListPoin
 
 func MyLog(_ format: String, _ args: CVarArg...) {
     withVaList(args) { arglist in
-        NSLogv(format, arglist) // BAD [NOT DETECTED]
+        NSLogv(format, arglist) // BAD
     }
 }
 

From 65e13aa5edf5a8ff3446e41c0aab3f4ed2ce3266 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Fri, 27 Oct 2023 10:27:35 +0100
Subject: [PATCH 28/32] Swift: Add simple version of the 'captureList' test
 that works.

---
 .../ql/test/library-tests/dataflow/capture/closures.swift  | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index 0634ec2e6b75..3b11356432a1 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -15,6 +15,13 @@ func captureList() {
   }()
 }
 
+func withoutCaptureList() {
+  let y: Int = source("withoutCaptureList", 124);
+  { [] () in
+     sink(y) // $ hasValueFlow=withoutCaptureList
+  }()
+}
+
 func setAndCallEscape() {
   let x = source("setAndCallEscape", 0)
 

From b41ec37993f46f24fafef409dc07a3db194c075d Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Fri, 27 Oct 2023 11:05:48 +0100
Subject: [PATCH 29/32] Swift: Remove the code related to constructor capture
 (and the related TODO). This cannot happen in Swift.

---
 .../dataflow/internal/DataFlowPrivate.qll     | 27 ++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index c8a373b99e41..b97dbb2669ad 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -942,7 +942,30 @@ private module CaptureInput implements VariableCapture::InputSig {
   }
 
   class Callable extends S::Callable {
-    predicate isConstructor() { this instanceof S::Initializer }
+    predicate isConstructor() {
+      // A class declaration cannot capture a variable in Swift. Consider this hypothetical example:
+      // ```
+      // protocol Interface { }
+      // func foo() -> Interface {
+      //   let y = 42
+      //   class Impl : Interface {
+      //     let x : Int
+      //     init() {
+      //         x = y
+      //     }
+      //   }
+      //   let object = Impl()
+      //   return object
+      // }
+      // ```
+      // The Swift compiler will reject this with an error message such as
+      // ```
+      // error: class declaration cannot close over value 'y' defined in outer scope
+      //          x = y
+      //              ^
+      // ```
+      none()
+    }
   }
 }
 
@@ -964,8 +987,6 @@ private CaptureFlow::ClosureNode asClosureNode(Node n) {
   or
   result.(CaptureFlow::ThisParameterNode).getCallable() = n.(ClosureSelfParameterNode).getClosure()
   or
-  result.(CaptureFlow::MallocNode).getClosureExpr() = n.getCfgNode().getNode().asAstNode() // TODO: figure out why the java version had PostUpdateNode logic here
-  or
   exists(CaptureInput::VariableWrite write |
     result.(CaptureFlow::VariableWriteSourceNode).getVariableWrite() = write and
     n.asExpr() = write.getSource()

From a5a7d27c4bd1f82a2647b0b997ad2b88ba1a26b1 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Fri, 27 Oct 2023 11:16:32 +0100
Subject: [PATCH 30/32] Swift: Add change note.

---
 swift/ql/lib/change-notes/2023-10-27-variable-capture.md | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 swift/ql/lib/change-notes/2023-10-27-variable-capture.md

diff --git a/swift/ql/lib/change-notes/2023-10-27-variable-capture.md b/swift/ql/lib/change-notes/2023-10-27-variable-capture.md
new file mode 100644
index 000000000000..94c7201c30ba
--- /dev/null
+++ b/swift/ql/lib/change-notes/2023-10-27-variable-capture.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Improved support for flow through captured variables that properly adheres to inter-procedural control flow.
\ No newline at end of file

From 9b150e4ea93790282c47e39be9bf9dd1602aae44 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Fri, 27 Oct 2023 11:22:56 +0100
Subject: [PATCH 31/32] Swift: Add failing test.

---
 .../ql/test/library-tests/dataflow/capture/closures.swift | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index 3b11356432a1..f4751d7a147a 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -181,3 +181,11 @@ class S {
     sink(self.bf2); // $ MISSING: hasValueFlow
   }
 }
+
+func multi() {
+  var x = 0
+  var y = source("multi", 1)
+  var f = { () in x = y }
+  f()
+  sink(x) // $ MISSING: hasValueFlow=multi
+}
\ No newline at end of file

From 68999f3cef4db3e3fbc6e6e8e6da7905c2ecd9cb Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Fri, 27 Oct 2023 11:25:19 +0100
Subject: [PATCH 32/32] Swift: Fix test by including the
 'allowParameterReturnInSelf' hook from the variable capture library.

---
 .../lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll | 7 ++++++-
 .../ql/test/library-tests/dataflow/capture/closures.swift  | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
index b97dbb2669ad..554869409182 100644
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -1387,7 +1387,12 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
  * One example would be to allow flow like `p.foo = p.bar;`, which is disallowed
  * by default as a heuristic.
  */
-predicate allowParameterReturnInSelf(ParameterNode p) { none() }
+predicate allowParameterReturnInSelf(ParameterNode p) {
+  exists(Callable c |
+    c = p.(ParameterNodeImpl).getEnclosingCallable().asSourceCallable() and
+    CaptureFlow::heuristicAllowInstanceParameterReturnInSelf(c)
+  )
+}
 
 /** An approximated `Content`. */
 class ContentApprox = Unit;
diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
index f4751d7a147a..948a46b9b989 100644
--- a/swift/ql/test/library-tests/dataflow/capture/closures.swift
+++ b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -187,5 +187,5 @@ func multi() {
   var y = source("multi", 1)
   var f = { () in x = y }
   f()
-  sink(x) // $ MISSING: hasValueFlow=multi
+  sink(x) // $ hasValueFlow=multi
 }
\ No newline at end of file