From 6a46da4c663d39a952d587f6487131049656b907 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 16:36:07 +0200 Subject: [PATCH 01/38] unified: Add tests with accessors Tests are split into three categories: local, static, and instance. They follow mostly the same structure. --- .../dataflow/accessors_instance.swift | 90 +++++++++++++++++++ .../dataflow/accessors_local.swift | 56 ++++++++++++ .../dataflow/accessors_static.swift | 67 ++++++++++++++ 3 files changed, 213 insertions(+) create mode 100644 unified/ql/test/library-tests/dataflow/accessors_instance.swift create mode 100644 unified/ql/test/library-tests/dataflow/accessors_local.swift create mode 100644 unified/ql/test/library-tests/dataflow/accessors_static.swift diff --git a/unified/ql/test/library-tests/dataflow/accessors_instance.swift b/unified/ql/test/library-tests/dataflow/accessors_instance.swift new file mode 100644 index 000000000000..9e2d0429deeb --- /dev/null +++ b/unified/ql/test/library-tests/dataflow/accessors_instance.swift @@ -0,0 +1,90 @@ +func source(_ s: String) -> String { return s } + +@discardableResult +func sink(_ s: Any) -> String { return "" } + +func t1() { + class C { + var x: String { source("t1.1") } + } + sink(C().x) // $ MISSING: hasValueFlow=t1.1 +} + +func t2() { + // Note: swift-format will try to remove the explicit 'get' below + // swift-format-ignore + class C { + var x: String { + get { source("t2.1") } + } + } + sink(C().x) // $ MISSING: hasValueFlow=t2.1 +} + +func t3() { + class C { + var x: String { + get { source("t3.1") } + set {} + } + } + sink(C().x) // $ MISSING: hasValueFlow=t3.1 +} + +func t4() { + class C { + var x: String { + get {} + set { sink(newValue) } // $ MISSING: hasValueFlow=t4.1 + } + } + C().x = source("t4.1") +} + +func t5() { + class C { + var x: String { + get { source("t5.1") } + set { sink(newValue) } // $ MISSING: hasValueFlow=t5.1 + } + } + let c = C() + c.x = c.x +} + +func t6() { + class C { + var x: String { source("t6.1") } + + func method() { + sink(self.x) // $ MISSING: hasValueFlow=t6.1 + sink(x) // $ MISSING: hasValueFlow=t6.1 + } + } +} + +func t7() { + class C { + var field: String = "safe" + var x: String { self.field } + } + var c = C() + sink(c.x) // no flow + c.field = source("t7.1") + sink(c.x) // $ MISSING: hasValueFlow=t7.1 +} + +func t8() { + class C { + var field: String = "safe" + var otherField: String = "safe" + var x: String { + get {} + set { otherField = field + newValue } + } + } + var c = C() + c.field = source("t8.1") + c.x = "safe" // trigger the setter + sink(c.otherField) // $ MISSING: hasTaintFlow=t8.1 +} diff --git a/unified/ql/test/library-tests/dataflow/accessors_local.swift b/unified/ql/test/library-tests/dataflow/accessors_local.swift new file mode 100644 index 000000000000..c66ebc30bbd0 --- /dev/null +++ b/unified/ql/test/library-tests/dataflow/accessors_local.swift @@ -0,0 +1,56 @@ +func source(_ s: String) -> String { return s } + +@discardableResult +func sink(_ s: Any) -> String { return "" } + +func t1() { + var x: String { source("t1.1") } + sink(x) // $ MISSING: hasValueFlow=t1.1 +} + +func t2() { + // Note: swift-format will try to remove the explicit 'get' below + // swift-format-ignore + var x: String { + get { source("t2.1") } + } + sink(x) // $ MISSING: hasValueFlow=t2.1 +} + +func t3() { + var x: String { + get { source("t3.1") } + set {} + } + sink(x) // $ MISSING: hasValueFlow=t3.1 +} + +func t4() { + var x: String { + get {} + set { sink(newValue) } // $ MISSING: hasValueFlow=t4.1 + } + x = source("t4.1") +} + +func t5() { + var x: String { + get { source("t5.1") } + set { sink(newValue) } // $ MISSING: hasValueFlow=t5.1 + } + x = x +} + +func t6() { + var y: String = source("t6.1") + var x: String { y } + sink(x) // $ MISSING: hasValueFlow=t6.1 +} + +func t7() { + var y: String = "safe" + var x: String { y } + sink(x) // no flow + y = source("t7.1") + sink(x) // $ MISSING: hasValueFlow=t7.1 +} diff --git a/unified/ql/test/library-tests/dataflow/accessors_static.swift b/unified/ql/test/library-tests/dataflow/accessors_static.swift new file mode 100644 index 000000000000..7df6858a4bda --- /dev/null +++ b/unified/ql/test/library-tests/dataflow/accessors_static.swift @@ -0,0 +1,67 @@ +func source(_ s: String) -> String { return s } + +@discardableResult +func sink(_ s: Any) -> String { return "" } + +func t1() { + class C { + static var x: String { source("t1.1") } + } + sink(C.x) // $ MISSING: hasValueFlow=t1.1 +} + +func t2() { + // Note: swift-format will try to remove the explicit 'get' below + // swift-format-ignore + class C { + static var x: String { + get { source("t2.1") } + } + } + sink(C.x) // $ MISSING: hasValueFlow=t2.1 +} + +func t3() { + class C { + static var x: String { + get { source("t3.1") } + set {} + } + } + sink(C.x) // $ MISSING: hasValueFlow=t3.1 +} + +func t4() { + class C { + static var x: String { + get {} + set { sink(newValue) } // $ MISSING: hasValueFlow=t4.1 + } + } + C.x = source("t4.1") +} + +func t5() { + class C { + static var x: String { + get { source("t5.1") } + set { sink(newValue) } // $ MISSING: hasValueFlow=t5.1 + } + } + C.x = C.x +} + +func t6() { + class C { + static var x: String { source("t6.1") } + + func method() { + sink(Self.x) // $ MISSING: hasValueFlow=t6.1 + } + + static func staticMethod() { + sink(Self.x) // $ MISSING: hasValueFlow=t6.1 + sink(x) // $ MISSING: hasValueFlow=t6.1 + } + } +} From 6270fad1d1f6e368418a60a1a264e3cb9c5ee0e7 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 16:48:32 +0200 Subject: [PATCH 02/38] unified: Allow accessors to be seen as instance members --- unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll | 1 + 1 file changed, 1 insertion(+) diff --git a/unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll b/unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll index f6b6cc25f7f5..2b01d98d28dc 100644 --- a/unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll +++ b/unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll @@ -81,6 +81,7 @@ predicate isInstanceMember(Member member) { ( member instanceof VariableDeclaration or member instanceof FunctionDeclaration or + member instanceof AccessorDeclaration or member instanceof ConstructorDeclaration ) and exists(ClassLikeDeclaration cls | cls.getAMember() = member | From 3fcfb68a94514c1299c449cdee7b1d0180a8f09c Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 13:07:04 +0200 Subject: [PATCH 03/38] unified: Add DataFlowCalls for getter and setter calls --- .../internal/dataflow/DataFlowCall.qll | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll index b6ceba637ef5..b6e18bc960ad 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll @@ -1,11 +1,20 @@ private import unified private import AllDataFlow private import codeql.unified.internal.ExprPositions +private import codeql.unified.internal.NameBinding as N private newtype TDataFlowCall = TExplicitCall(CallExpr call) { not isInBindingContext(call, _) // ignore constructor patterns } or + TGetterCall(Expr expr) { + hasResultValue(expr) and + N::getStaticBindingTargetFromRef(expr) = any(GetterDeclaration g).getNameNode() + } or + TSetterCall(Expr expr) { + isInAssignmentContext(expr, _) and + N::getStaticBindingTargetFromRef(expr) = any(SetterDeclaration g).getNameNode() + } or TSummaryCall(SummarizedCallable callable, SummaryNode callee) { FlowSummaryOutput::summaryCallbackRange(callable, callee) } @@ -20,6 +29,12 @@ class DataFlowCall extends TDataFlowCall { /** Gets the `CallExpr` wrapped by this dataflow call, if any. */ CallExpr asExplicitCall() { this = TExplicitCall(result) } + /** Gets the member/variable access for which this represents the call to a getter. */ + Expr asGetterCall() { this = TGetterCall(result) } + + /** Gets the member/variable access for which this represents the call to a setter. */ + Expr asSetterCall() { this = TSetterCall(result) } + /** Holds if this is a synthesized call to `callee` in `callable`. */ predicate isSummaryCall(SummarizedCallable callable, SummaryNode callee) { this = TSummaryCall(callable, callee) @@ -29,6 +44,10 @@ class DataFlowCall extends TDataFlowCall { string toString() { result = this.asExplicitCall().toString() or + result = "[getter call] " + this.asGetterCall().toString() + or + result = "[setter call] " + this.asSetterCall().toString() + or exists(SummarizedCallable callable, SummaryNode callee | this.isSummaryCall(callable, callee) and result = "[summary] call to " + callee.toString() + " in " + callable.toString() @@ -39,6 +58,10 @@ class DataFlowCall extends TDataFlowCall { Location getLocation() { result = this.asExplicitCall().getLocation() or + result = this.asGetterCall().getLocation() + or + result = this.asSetterCall().getLocation() + or this.isSummaryCall(_, _) and result instanceof EmptyLocation } @@ -46,6 +69,10 @@ class DataFlowCall extends TDataFlowCall { DataFlowCallable getEnclosingCallable() { result.asSourceCallable() = this.asExplicitCall().getEnclosingCallable() or + result.asSourceCallable() = this.asGetterCall().getEnclosingCallable() + or + result.asSourceCallable() = this.asSetterCall().getEnclosingCallable() + or this.isSummaryCall(result.asSummarizedCallable(), _) } } From 61a102c108a336da5421158e66034819013fa71c Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 21:07:18 +0200 Subject: [PATCH 04/38] unified: Resolve calls statically --- .../lib/codeql/unified/internal/dataflow/CallGraph.qll | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll index b539ca7e4240..38226b6e497c 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll @@ -1,9 +1,19 @@ private import unified private import AllDataFlow private import codeql.unified.internal.typeinference.TypeInference as T +private import codeql.unified.internal.NameBinding as N +private import codeql.unified.internal.ExprPositions DataFlowCallable viableCallable(DataFlowCall c) { result.asSourceCallable() = T::resolveCallTarget(c.asExplicitCall()) or + result.asSourceCallable() = resolveAccessor(c.asGetterCall()).(GetterDeclaration) + or + result.asSourceCallable() = resolveAccessor(c.asSetterCall()).(SetterDeclaration) + or result.asSummarizedCallable().matchesCall(c.asExplicitCall()) } + +private AccessorDeclaration resolveAccessor(Expr access) { + result.getNameNode() = N::getStaticBindingTargetFromRef(access) +} From f1f4d8400ee0a270331173d53a9d114af826d829 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 21:07:31 +0200 Subject: [PATCH 05/38] unified: Add out-node from getter calls --- .../dataflow/DataFlowInstantiation.qll | 6 ++++++ .../dataflow/accessors_instance.swift | 2 +- .../dataflow/accessors_local.swift | 6 +++--- .../dataflow/accessors_static.swift | 2 +- .../test/library-tests/dataflow/test.expected | 20 +++++++++++++++++++ 5 files changed, 31 insertions(+), 5 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll index b84f051047e6..08779aafffbd 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll @@ -41,6 +41,12 @@ module DataFlowInput implements InputSig { FlowSummaryOutput::summaryOutNode(callee, getSummaryNode(n), kind) and call.isSummaryCall(_, callee) ) + or + exists(Expr access | + call.asGetterCall() = access and + n.isResultValue(access) and + kind.isStandardReturnValue() + ) } class OutNode extends Node { diff --git a/unified/ql/test/library-tests/dataflow/accessors_instance.swift b/unified/ql/test/library-tests/dataflow/accessors_instance.swift index 9e2d0429deeb..97a0adee4ead 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_instance.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_instance.swift @@ -58,7 +58,7 @@ func t6() { func method() { sink(self.x) // $ MISSING: hasValueFlow=t6.1 - sink(x) // $ MISSING: hasValueFlow=t6.1 + sink(x) // $ hasValueFlow=t6.1 } } } diff --git a/unified/ql/test/library-tests/dataflow/accessors_local.swift b/unified/ql/test/library-tests/dataflow/accessors_local.swift index c66ebc30bbd0..e15ef561b0cd 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_local.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_local.swift @@ -5,7 +5,7 @@ func sink(_ s: Any) -> String { return "" } func t1() { var x: String { source("t1.1") } - sink(x) // $ MISSING: hasValueFlow=t1.1 + sink(x) // $ hasValueFlow=t1.1 } func t2() { @@ -14,7 +14,7 @@ func t2() { var x: String { get { source("t2.1") } } - sink(x) // $ MISSING: hasValueFlow=t2.1 + sink(x) // $ hasValueFlow=t2.1 } func t3() { @@ -22,7 +22,7 @@ func t3() { get { source("t3.1") } set {} } - sink(x) // $ MISSING: hasValueFlow=t3.1 + sink(x) // $ hasValueFlow=t3.1 } func t4() { diff --git a/unified/ql/test/library-tests/dataflow/accessors_static.swift b/unified/ql/test/library-tests/dataflow/accessors_static.swift index 7df6858a4bda..7bd37efa7611 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_static.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_static.swift @@ -61,7 +61,7 @@ func t6() { static func staticMethod() { sink(Self.x) // $ MISSING: hasValueFlow=t6.1 - sink(x) // $ MISSING: hasValueFlow=t6.1 + sink(x) // $ hasValueFlow=t6.1 } } } diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index 5ec726ff99be..32e420134d31 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -1,5 +1,10 @@ models edges +| accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | provenance | | +| accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | | +| accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | | +| accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | provenance | | +| accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:64:18:64:18 | x | provenance | | | calls.swift:7:19:7:19 | x | calls.swift:8:14:8:14 | x | provenance | | | calls.swift:10:12:10:25 | source(...) | calls.swift:7:19:7:19 | x | provenance | | | calls.swift:15:16:15:29 | source(...) | calls.swift:17:10:17:17 | target(...) | provenance | | @@ -459,6 +464,16 @@ edges | test.swift:271:20:271:35 | source(...) | test.swift:271:9:271:9 | x | provenance | | | test.swift:273:30:273:45 | source(...) | test.swift:274:10:274:12 | f(...) | provenance | | nodes +| accessors_instance.swift:57:25:57:38 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:61:18:61:18 | x | semmle.label | x | +| accessors_local.swift:7:21:7:34 | source(...) | semmle.label | source(...) | +| accessors_local.swift:8:10:8:10 | x | semmle.label | x | +| accessors_local.swift:15:15:15:28 | source(...) | semmle.label | source(...) | +| accessors_local.swift:17:10:17:10 | x | semmle.label | x | +| accessors_local.swift:22:15:22:28 | source(...) | semmle.label | source(...) | +| accessors_local.swift:25:10:25:10 | x | semmle.label | x | +| accessors_static.swift:56:32:56:45 | source(...) | semmle.label | source(...) | +| accessors_static.swift:64:18:64:18 | x | semmle.label | x | | calls.swift:7:19:7:19 | x | semmle.label | x | | calls.swift:8:14:8:14 | x | semmle.label | x | | calls.swift:10:12:10:25 | source(...) | semmle.label | source(...) | @@ -1036,6 +1051,11 @@ subpaths | test.swift:214:43:214:57 | source(...) | test.swift:193:30:193:30 | s | test.swift:193:73:193:73 | s | test.swift:214:21:214:58 | asyncThrowingIdentity(...) | testFailures #select +| accessors_instance.swift:61:18:61:18 | x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) | +| accessors_local.swift:8:10:8:10 | x | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | $@ | accessors_local.swift:7:21:7:34 | source(...) | source(...) | +| accessors_local.swift:17:10:17:10 | x | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | $@ | accessors_local.swift:15:15:15:28 | source(...) | source(...) | +| accessors_local.swift:25:10:25:10 | x | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | $@ | accessors_local.swift:22:15:22:28 | source(...) | source(...) | +| accessors_static.swift:64:18:64:18 | x | accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:64:18:64:18 | x | $@ | accessors_static.swift:56:32:56:45 | source(...) | source(...) | | calls.swift:8:14:8:14 | x | calls.swift:10:12:10:25 | source(...) | calls.swift:8:14:8:14 | x | $@ | calls.swift:10:12:10:25 | source(...) | source(...) | | calls.swift:17:10:17:17 | target(...) | calls.swift:15:16:15:29 | source(...) | calls.swift:17:10:17:17 | target(...) | $@ | calls.swift:15:16:15:29 | source(...) | source(...) | | calls.swift:22:14:22:14 | x | calls.swift:24:15:24:28 | source(...) | calls.swift:22:14:22:14 | x | $@ | calls.swift:24:15:24:28 | source(...) | source(...) | From 82487f807281b19d91aa60e5f7d4967869d64d03 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 21:35:20 +0200 Subject: [PATCH 06/38] unified: Synthesize setter parameters and return type in accessors Accessors previously had a 'type' field. Now, it has a return_type instead, which is only set for getters. For set, didSet, and willSet, we insert a synthetic newValue/oldValue parameter, which also carries the type previously stored in accessor_declaration.type. --- unified/extractor/ast_types.yml | 2 +- .../extractor/src/languages/swift/swift.rs | 105 ++++++++++++------ ...er-does-not-leak-into-accessor-body.output | 2 +- .../swift/types/computed-property.output | 4 +- .../property-with-getter-and-setter.output | 10 +- ...nd-read-write-property-requirements.output | 14 ++- ...y-with-willset-and-didset-observers.output | 14 ++- .../ql/lib/codeql/unified/internal/Ast.qll | 10 +- .../codeql/unified/internal/ExprPositions.qll | 2 +- unified/ql/lib/unified.dbscheme | 4 +- 10 files changed, 114 insertions(+), 53 deletions(-) diff --git a/unified/extractor/ast_types.yml b/unified/extractor/ast_types.yml index 0eaf1e8f02f1..fb9d88333087 100644 --- a/unified/extractor/ast_types.yml +++ b/unified/extractor/ast_types.yml @@ -534,7 +534,7 @@ named: name_node: identifier accessor_kind: accessor_kind parameter*: parameter - type?: expr + return_type?: expr body?: block # "get", "set", or a language-specific kind like "didSet" diff --git a/unified/extractor/src/languages/swift/swift.rs b/unified/extractor/src/languages/swift/swift.rs index 75b7c4e9c5c5..3e7f3cef408a 100644 --- a/unified/extractor/src/languages/swift/swift.rs +++ b/unified/extractor/src/languages/swift/swift.rs @@ -12,10 +12,10 @@ struct SwiftContext { /// `variableDecl` rule before translating the accessor block; read by the /// inner `accessorDecl` rules to name each `accessor_declaration`. property_name_node: Option, - /// Translated type node for the property type. Set (for computed - /// properties) by the accessor-bearing `variableDecl` rule; read by the - /// inner `accessorDecl` rules. Left `None` for stored properties with - /// observers, so their `willSet`/`didSet` accessors carry no type. + /// Translated type node for the property type. Set by the + /// accessor-bearing `variableDecl` rule; read by the inner `accessorDecl` + /// rules for both the getter return type and its `newValue`/`oldValue` + /// parameter type, when available. property_type: Option, /// Translated outer modifiers to attach to each child of a flattening /// outer rule — e.g. the `let`/`var` binding modifier on each @@ -334,7 +334,7 @@ fn translation_rules() -> Vec> { (accessor_declaration modifier: (modifier #{spec}) name_node: (identifier #{name}) - type: {ty} + return_type: {ty} accessor_kind: {ctx.literal_at_start_of("accessor_kind", "get", body)} body: {body}) ), @@ -345,12 +345,12 @@ fn translation_rules() -> Vec> { // * With an initializer (`var x: T = e { willSet {…} didSet {…} }`) it is // a *stored* property with observers: emit the backing // `variable_declaration` first, then one `accessor_declaration` per - // observer (observers carry no type). + // observer. // * Without an initializer (`var v: T { get set }`, incl. protocol - // requirements) it is a *computed* property: no backing variable; the - // type is published so the get/set accessors carry it. + // requirements) it is a *computed* property: no backing variable. // - // In both cases the first emitted declaration is unchained and every + // In both cases the type is published so every accessor carries it, and + // the first emitted declaration is unchained and every // subsequent one is tagged `chained_declaration` (the `!result.is_empty()` // test). Must precede the plain `variableDecl` rules. rule!( @@ -365,6 +365,7 @@ fn translation_rules() -> Vec> { member* { ctx.outer_modifiers = vec![tree!((modifier #{spec}))]; ctx.property_name_node = Some(tree!((identifier #{name}))); + ctx.property_type = Some(ty); let mut result = Vec::new(); if let Some(val) = val { // Stored property with observers: the initializer is not part @@ -380,9 +381,6 @@ fn translation_rules() -> Vec> { type: {ty} value: {val}) )); - } else { - // Computed property: the accessors carry the type. - ctx.property_type = Some(ty); } for acc in accessors.into_iter() { ctx.is_chained = !result.is_empty(); @@ -400,27 +398,70 @@ fn translation_rules() -> Vec> { // one (a protocol requirement) carries neither. The property context is // read out *before* translating the body, which resets `ctx` so the // accessor's context does not leak into the body subtree. - rule!( - (accessorDecl accessorSpecifier: @@spec body: _? @@body) - => - (accessor_declaration - modifier: { - if body.is_some() { - ctx.outer_modifiers.clone() - } else { - Vec::new() + // + // `set` and `willSet` accessors get a parameter holding the incoming + // value, and `didSet` gets one holding the previous value. It is named + // explicitly (`set(v)`) or else implicitly `newValue`/`oldValue`. + rule!( + (accessorDecl + accessorSpecifier: @@spec + parameters: (accessorParameters name: @@param_name)? + body: _? @@body) + => + accessor_declaration { + let spec_text = ctx.source_text(spec); + let implicit_param_name = match spec_text.as_str() { + "set" | "willSet" => Some("newValue"), + "didSet" => Some("oldValue"), + _ => None, + }; + let param_ty = ctx.property_type; + let param = match (param_name, implicit_param_name) { + (Some(n), _) => { + Some(tree!((parameter pattern: (identifier #{n}) type: {param_ty}))) } - } - modifier: {chained_modifier(&mut ctx)} - name_node: { - ctx.property_name_node - .ok_or("accessor outside property context")? - } - type: {ctx.property_type} - accessor_kind: (accessor_kind #{spec}) - body: {ctx.reset(); body} - location: {spec} - location: {body}) + (None, Some(n)) => { + let n = n.to_string(); + Some(tree!((parameter + modifier: (modifier "generated") + pattern: (identifier #{n} location: {spec}) + type: {param_ty} + location: {spec}))) + } + (None, None) => None, + }; + let binding = if body.is_some() { + ctx.outer_modifiers.clone() + } else { + Vec::new() + }; + let chained = chained_modifier(&mut ctx); + let name = ctx + .property_name_node + .ok_or("accessor outside property context")?; + let return_ty = if spec_text == "get" { + ctx.property_type + } else { + None + }; + let body = match body { + Some(block) => { + ctx.reset(); + ctx.translate(block)?.into_iter().next() + } + None => None, + }; + tree!((accessor_declaration + modifier: {binding} + modifier: {chained} + name_node: {name} + return_type: {return_ty} + accessor_kind: (accessor_kind #{spec}) + parameter: {param} + body: {body} + location: {spec} + location: {body})) + } ), // ---- Variables ---- // The individual bindings of a `variableDecl`. The binding modifier and diff --git a/unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output b/unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output index 88674d76efbd..f3ce25b05608 100644 --- a/unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output +++ b/unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output @@ -100,7 +100,7 @@ top_level source="var p: Int {\n ⟨body⟩\n}" modifier: modifier "var" source="var" (external) name_node: identifier "p" source="p" (external) accessor_kind: accessor_kind "get" source="get" - type: identifier "Int" source="Int" (external) + return_type: identifier "Int" source="Int" (external) body: block source="{\n ⟨stmt⟩\n }" stmt: diff --git a/unified/extractor/tests/corpus/swift/types/computed-property.output b/unified/extractor/tests/corpus/swift/types/computed-property.output index 4ed7a0e5d096..83e7c84a1a3a 100644 --- a/unified/extractor/tests/corpus/swift/types/computed-property.output +++ b/unified/extractor/tests/corpus/swift/types/computed-property.output @@ -113,11 +113,11 @@ top_level source="⟨body⟩" modifier: modifier "var" source="var" pattern: identifier "h" source="h" type: identifier "Double" source="Double" - accessor_declaration source="⟨modifier⟩ ⟨name_node⟩: ⟨type⟩ ⟨body⟩" + accessor_declaration source="⟨modifier⟩ ⟨name_node⟩: ⟨return_type⟩ ⟨body⟩" modifier: modifier "var" source="var" name_node: identifier "area" source="area" accessor_kind: accessor_kind "get" source="" - type: identifier "Double" source="Double" + return_type: identifier "Double" source="Double" body: block source="{\n ⟨stmt⟩\n }" stmt: diff --git a/unified/extractor/tests/corpus/swift/types/property-with-getter-and-setter.output b/unified/extractor/tests/corpus/swift/types/property-with-getter-and-setter.output index ce8d1d6f9bd2..b315af33e2b5 100644 --- a/unified/extractor/tests/corpus/swift/types/property-with-getter-and-setter.output +++ b/unified/extractor/tests/corpus/swift/types/property-with-getter-and-setter.output @@ -120,19 +120,23 @@ top_level source="⟨body⟩" modifier: modifier "var" source="var" (external) name_node: identifier "v" source="v" (external) accessor_kind: accessor_kind "get" source="get" - type: identifier "Int" source="Int" (external) + return_type: identifier "Int" source="Int" (external) body: block source="{ ⟨stmt⟩ }" stmt: return_expr source="return ⟨value⟩" value: identifier "_v" source="_v" - accessor_declaration source="⟨accessor_kind⟩ ⟨body⟩" + accessor_declaration source="⟨accessor_kind⟩⟨parameter⟩ ⟨body⟩" modifier: modifier "var" source="var" (external) modifier "chained_declaration" source="" name_node: identifier "v" source="v" (external) accessor_kind: accessor_kind "set" source="set" - type: identifier "Int" source="Int" (external) + parameter: + parameter source="⟨pattern⟩" + modifier: modifier "generated" source="" + type: identifier "Int" source="Int" (external) + pattern: identifier "newValue" source="set" body: block source="{ ⟨stmt⟩ }" stmt: diff --git a/unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output b/unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output index 8a43a3c70588..515496c79a33 100644 --- a/unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output +++ b/unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output @@ -113,17 +113,21 @@ top_level source="⟨body⟩" accessor_declaration source="⟨accessor_kind⟩" name_node: identifier "foo" source="foo" (external) accessor_kind: accessor_kind "get" source="get" - type: identifier "Int" source="Int" (external) + return_type: identifier "Int" source="Int" (external) accessor_declaration source="⟨accessor_kind⟩" name_node: identifier "bar" source="bar" (external) accessor_kind: accessor_kind "get" source="get" - type: identifier "String" source="String" (external) - accessor_declaration source="⟨accessor_kind⟩" + return_type: identifier "String" source="String" (external) + accessor_declaration source="⟨accessor_kind⟩⟨parameter⟩" modifier: modifier "chained_declaration" source="" name_node: identifier "bar" source="bar" (external) accessor_kind: accessor_kind "set" source="set" - type: identifier "String" source="String" (external) + parameter: + parameter source="⟨pattern⟩" + modifier: modifier "generated" source="" + type: identifier "String" source="String" (external) + pattern: identifier "newValue" source="set" accessor_declaration source="⟨accessor_kind⟩" name_node: identifier "count" source="count" (external) accessor_kind: accessor_kind "get" source="get" - type: identifier "Int" source="Int" (external) + return_type: identifier "Int" source="Int" (external) diff --git a/unified/extractor/tests/corpus/swift/variables/property-with-willset-and-didset-observers.output b/unified/extractor/tests/corpus/swift/variables/property-with-willset-and-didset-observers.output index d95dfd51bc74..6b9a2fad278a 100644 --- a/unified/extractor/tests/corpus/swift/variables/property-with-willset-and-didset-observers.output +++ b/unified/extractor/tests/corpus/swift/variables/property-with-willset-and-didset-observers.output @@ -110,12 +110,17 @@ top_level source="⟨body⟩" pattern: identifier "x" source="x" type: identifier "Int" source="Int" value: int_literal "0" source="0" - accessor_declaration source="⟨accessor_kind⟩ ⟨body⟩" + accessor_declaration source="⟨accessor_kind⟩⟨parameter⟩ ⟨body⟩" modifier: modifier "var" source="var" (external) modifier "chained_declaration" source="" name_node: identifier "x" source="x" (external) accessor_kind: accessor_kind "willSet" source="willSet" + parameter: + parameter source="⟨pattern⟩" + modifier: modifier "generated" source="" + type: identifier "Int" source="Int" (external) + pattern: identifier "newValue" source="willSet" body: block source="{ ⟨stmt⟩ }" stmt: @@ -124,12 +129,17 @@ top_level source="⟨body⟩" argument: argument source="⟨value⟩" value: identifier "newValue" source="newValue" - accessor_declaration source="⟨accessor_kind⟩ ⟨body⟩" + accessor_declaration source="⟨accessor_kind⟩⟨parameter⟩ ⟨body⟩" modifier: modifier "var" source="var" (external) modifier "chained_declaration" source="" name_node: identifier "x" source="x" (external) accessor_kind: accessor_kind "didSet" source="didSet" + parameter: + parameter source="⟨pattern⟩" + modifier: modifier "generated" source="" + type: identifier "Int" source="Int" (external) + pattern: identifier "oldValue" source="didSet" body: block source="{ ⟨stmt⟩ }" stmt: diff --git a/unified/ql/lib/codeql/unified/internal/Ast.qll b/unified/ql/lib/codeql/unified/internal/Ast.qll index 17c06cf1fd66..5f410460536e 100644 --- a/unified/ql/lib/codeql/unified/internal/Ast.qll +++ b/unified/ql/lib/codeql/unified/internal/Ast.qll @@ -123,8 +123,10 @@ module Unified { /** Gets the node corresponding to the field `parameter`. */ final override F::Parameter getAParameter() { result = this.getParameter(_) } - /** Gets the node corresponding to the field `type`. */ - final F::Expr getType() { unified_accessor_declaration_type(this, result) } + /** Gets the node corresponding to the field `return_type`. */ + final override F::Expr getReturnType() { + unified_accessor_declaration_return_type(this, result) + } /** Gets a field or child node of this node. */ final override F::AstNode getAFieldOrChild() { @@ -133,7 +135,7 @@ module Unified { unified_accessor_declaration_modifier(this, _, result) or unified_accessor_declaration_def(this, _, result) or unified_accessor_declaration_parameter(this, _, result) or - unified_accessor_declaration_type(this, result) + unified_accessor_declaration_return_type(this, result) } } @@ -1624,7 +1626,7 @@ module Unified { or result = node.(AccessorDeclaration).getParameter(i) and name = "getParameter" or - result = node.(AccessorDeclaration).getType() and i = -1 and name = "getType" + result = node.(AccessorDeclaration).getReturnType() and i = -1 and name = "getReturnType" or result = node.(Argument).getModifier(i) and name = "getModifier" or diff --git a/unified/ql/lib/codeql/unified/internal/ExprPositions.qll b/unified/ql/lib/codeql/unified/internal/ExprPositions.qll index 3edc36cc1eb2..d93205b371a1 100644 --- a/unified/ql/lib/codeql/unified/internal/ExprPositions.qll +++ b/unified/ql/lib/codeql/unified/internal/ExprPositions.qll @@ -16,7 +16,7 @@ predicate isInTypeContext(Expr expr) { or expr = any(FunctionExpr n).getReturnType() or - expr = any(AccessorDeclaration n).getType() + expr = any(AccessorDeclaration n).getReturnType() or expr = any(Parameter n).getType() or diff --git a/unified/ql/lib/unified.dbscheme b/unified/ql/lib/unified.dbscheme index 5c93d547be59..54ede4d9d45c 100644 --- a/unified/ql/lib/unified.dbscheme +++ b/unified/ql/lib/unified.dbscheme @@ -155,9 +155,9 @@ unified_accessor_declaration_parameter( unique int parameter: @unified_parameter ref ); -unified_accessor_declaration_type( +unified_accessor_declaration_return_type( unique int unified_accessor_declaration: @unified_accessor_declaration ref, - unique int type__: @unified_expr ref + unique int return_type: @unified_expr ref ); unified_accessor_declaration_def( From 5dec1dc7bd3e9b4967884bb6b533de0899b64249 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 21:38:09 +0200 Subject: [PATCH 07/38] unified: Pass the incoming value into setter calls --- .../dataflow/DataFlowInstantiation.qll | 6 ++++++ .../dataflow/accessors_local.swift | 4 ++-- .../test/library-tests/dataflow/test.expected | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll index 08779aafffbd..8f9477b4749a 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll @@ -107,6 +107,12 @@ module DataFlowInput implements InputSig { or n.(BuilderNode).isImplicitArgument(call, pos, false) or + exists(Expr access | + call.asSetterCall() = access and + n.isIncomingValue(access) and + pos.asPositional() = 0 + ) + or exists(FlowSummaryOutput::SummaryNode callee | FlowSummaryOutput::summaryArgumentNode(callee, getSummaryNode(n), pos) and call.isSummaryCall(_, callee) diff --git a/unified/ql/test/library-tests/dataflow/accessors_local.swift b/unified/ql/test/library-tests/dataflow/accessors_local.swift index e15ef561b0cd..a54e8f178b26 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_local.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_local.swift @@ -28,7 +28,7 @@ func t3() { func t4() { var x: String { get {} - set { sink(newValue) } // $ MISSING: hasValueFlow=t4.1 + set { sink(newValue) } // $ hasValueFlow=t4.1 } x = source("t4.1") } @@ -36,7 +36,7 @@ func t4() { func t5() { var x: String { get { source("t5.1") } - set { sink(newValue) } // $ MISSING: hasValueFlow=t5.1 + set { sink(newValue) } // $ hasValueFlow=t5.1 } x = x } diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index 32e420134d31..c1fdc513c4eb 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -4,6 +4,13 @@ edges | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | | | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | | | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | provenance | | +| accessors_local.swift:31:9:31:11 | newValue | accessors_local.swift:31:20:31:27 | newValue | provenance | | +| accessors_local.swift:33:5:33:5 | x | accessors_local.swift:31:9:31:11 | newValue | provenance | | +| accessors_local.swift:33:9:33:22 | source(...) | accessors_local.swift:33:5:33:5 | x | provenance | | +| accessors_local.swift:38:15:38:28 | source(...) | accessors_local.swift:41:9:41:9 | x | provenance | | +| accessors_local.swift:39:9:39:11 | newValue | accessors_local.swift:39:20:39:27 | newValue | provenance | | +| accessors_local.swift:41:5:41:5 | x | accessors_local.swift:39:9:39:11 | newValue | provenance | | +| accessors_local.swift:41:9:41:9 | x | accessors_local.swift:41:5:41:5 | x | provenance | | | accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:64:18:64:18 | x | provenance | | | calls.swift:7:19:7:19 | x | calls.swift:8:14:8:14 | x | provenance | | | calls.swift:10:12:10:25 | source(...) | calls.swift:7:19:7:19 | x | provenance | | @@ -472,6 +479,15 @@ nodes | accessors_local.swift:17:10:17:10 | x | semmle.label | x | | accessors_local.swift:22:15:22:28 | source(...) | semmle.label | source(...) | | accessors_local.swift:25:10:25:10 | x | semmle.label | x | +| accessors_local.swift:31:9:31:11 | newValue | semmle.label | newValue | +| accessors_local.swift:31:20:31:27 | newValue | semmle.label | newValue | +| accessors_local.swift:33:5:33:5 | x | semmle.label | x | +| accessors_local.swift:33:9:33:22 | source(...) | semmle.label | source(...) | +| accessors_local.swift:38:15:38:28 | source(...) | semmle.label | source(...) | +| accessors_local.swift:39:9:39:11 | newValue | semmle.label | newValue | +| accessors_local.swift:39:20:39:27 | newValue | semmle.label | newValue | +| accessors_local.swift:41:5:41:5 | x | semmle.label | x | +| accessors_local.swift:41:9:41:9 | x | semmle.label | x | | accessors_static.swift:56:32:56:45 | source(...) | semmle.label | source(...) | | accessors_static.swift:64:18:64:18 | x | semmle.label | x | | calls.swift:7:19:7:19 | x | semmle.label | x | @@ -1055,6 +1071,8 @@ testFailures | accessors_local.swift:8:10:8:10 | x | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | $@ | accessors_local.swift:7:21:7:34 | source(...) | source(...) | | accessors_local.swift:17:10:17:10 | x | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | $@ | accessors_local.swift:15:15:15:28 | source(...) | source(...) | | accessors_local.swift:25:10:25:10 | x | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | $@ | accessors_local.swift:22:15:22:28 | source(...) | source(...) | +| accessors_local.swift:31:20:31:27 | newValue | accessors_local.swift:33:9:33:22 | source(...) | accessors_local.swift:31:20:31:27 | newValue | $@ | accessors_local.swift:33:9:33:22 | source(...) | source(...) | +| accessors_local.swift:39:20:39:27 | newValue | accessors_local.swift:38:15:38:28 | source(...) | accessors_local.swift:39:20:39:27 | newValue | $@ | accessors_local.swift:38:15:38:28 | source(...) | source(...) | | accessors_static.swift:64:18:64:18 | x | accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:64:18:64:18 | x | $@ | accessors_static.swift:56:32:56:45 | source(...) | source(...) | | calls.swift:8:14:8:14 | x | calls.swift:10:12:10:25 | source(...) | calls.swift:8:14:8:14 | x | $@ | calls.swift:10:12:10:25 | source(...) | source(...) | | calls.swift:17:10:17:17 | target(...) | calls.swift:15:16:15:29 | source(...) | calls.swift:17:10:17:17 | target(...) | $@ | calls.swift:15:16:15:29 | source(...) | source(...) | From c7c80a76a5b4b97221e7f5673bf9b4585b034862 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 21:55:22 +0200 Subject: [PATCH 08/38] unified: Ensure 'static' modifier is preserved for accessors Modifiers like 'static' did not get preserved when extracting accessors, making most of the tests fail for static getters/setters. All those tests now pass. --- .../extractor/src/languages/swift/swift.rs | 33 ++++++++++------- ...nd-read-write-property-requirements.output | 7 +++- .../dataflow/accessors_static.swift | 14 ++++---- .../test/library-tests/dataflow/test.expected | 36 +++++++++++++++++++ 4 files changed, 69 insertions(+), 21 deletions(-) diff --git a/unified/extractor/src/languages/swift/swift.rs b/unified/extractor/src/languages/swift/swift.rs index 3e7f3cef408a..a1dab86b626e 100644 --- a/unified/extractor/src/languages/swift/swift.rs +++ b/unified/extractor/src/languages/swift/swift.rs @@ -18,9 +18,9 @@ struct SwiftContext { /// parameter type, when available. property_type: Option, /// Translated outer modifiers to attach to each child of a flattening - /// outer rule — e.g. the `let`/`var` binding modifier on each - /// `patternBinding` of a `variableDecl`, or the binding modifier on each - /// accessor of a property. + /// outer rule — e.g. the `let`/`var` binding modifier, attributes and + /// modifiers (`static`, `public`, …) on each `patternBinding` of a + /// `variableDecl`, or on each accessor of a property. outer_modifiers: Vec, /// True when the current child of a flattening outer rule is not /// the first one — its inner rule should emit a @@ -325,6 +325,8 @@ fn translation_rules() -> Vec> { // block. rule!( (variableDecl + attributes: _* @attrs + modifiers: _* @mods bindingSpecifier: @@spec bindings: (patternBinding pattern: (identifierPattern identifier: @@name) @@ -333,6 +335,8 @@ fn translation_rules() -> Vec> { => (accessor_declaration modifier: (modifier #{spec}) + modifier: {attrs} + modifier: {mods} name_node: (identifier #{name}) return_type: {ty} accessor_kind: {ctx.literal_at_start_of("accessor_kind", "get", body)} @@ -355,6 +359,8 @@ fn translation_rules() -> Vec> { // test). Must precede the plain `variableDecl` rules. rule!( (variableDecl + attributes: _* @attrs + modifiers: _* @mods bindingSpecifier: @@spec bindings: (patternBinding pattern: (identifierPattern identifier: @@name) @@ -363,7 +369,12 @@ fn translation_rules() -> Vec> { accessorBlock: (accessorBlock accessors: (accessorDecl)+ @@accessors))) => member* { - ctx.outer_modifiers = vec![tree!((modifier #{spec}))]; + // The binding (`let`/`var`) leads, then attributes then modifiers, + // matching the plain `variableDecl` rule. + ctx.outer_modifiers = std::iter::once(tree!((modifier #{spec}))) + .chain(attrs) + .chain(mods) + .collect(); ctx.property_name_node = Some(tree!((identifier #{name}))); ctx.property_type = Some(ty); let mut result = Vec::new(); @@ -390,12 +401,12 @@ fn translation_rules() -> Vec> { } ), // Each `accessorDecl` becomes an `accessor_declaration`, reading the - // property name/type and the binding/chained modifiers from `ctx`. The + // property name/type and the modifiers from `ctx`. The // accessor kind comes straight from the specifier keyword // (`get`/`set`/`willSet`/`didSet`). The body is optional: a body-bearing // accessor (a computed getter/setter, or a `willSet`/`didSet` observer) - // carries the binding modifier and a translated body, whereas a bodyless - // one (a protocol requirement) carries neither. The property context is + // has a translated body, whereas a bodyless one (a protocol requirement) + // has none. The property context is // read out *before* translating the body, which resets `ctx` so the // accessor's context does not leak into the body subtree. // @@ -430,11 +441,7 @@ fn translation_rules() -> Vec> { } (None, None) => None, }; - let binding = if body.is_some() { - ctx.outer_modifiers.clone() - } else { - Vec::new() - }; + let modifiers = ctx.outer_modifiers.clone(); let chained = chained_modifier(&mut ctx); let name = ctx .property_name_node @@ -452,7 +459,7 @@ fn translation_rules() -> Vec> { None => None, }; tree!((accessor_declaration - modifier: {binding} + modifier: {modifiers} modifier: {chained} name_node: {name} return_type: {return_ty} diff --git a/unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output b/unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output index 515496c79a33..07fbf97a94fe 100644 --- a/unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output +++ b/unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output @@ -111,15 +111,19 @@ top_level source="⟨body⟩" name_node: identifier "P" source="P" member: accessor_declaration source="⟨accessor_kind⟩" + modifier: modifier "var" source="var" (external) name_node: identifier "foo" source="foo" (external) accessor_kind: accessor_kind "get" source="get" return_type: identifier "Int" source="Int" (external) accessor_declaration source="⟨accessor_kind⟩" + modifier: modifier "var" source="var" (external) name_node: identifier "bar" source="bar" (external) accessor_kind: accessor_kind "get" source="get" return_type: identifier "String" source="String" (external) accessor_declaration source="⟨accessor_kind⟩⟨parameter⟩" - modifier: modifier "chained_declaration" source="" + modifier: + modifier "var" source="var" (external) + modifier "chained_declaration" source="" name_node: identifier "bar" source="bar" (external) accessor_kind: accessor_kind "set" source="set" parameter: @@ -128,6 +132,7 @@ top_level source="⟨body⟩" type: identifier "String" source="String" (external) pattern: identifier "newValue" source="set" accessor_declaration source="⟨accessor_kind⟩" + modifier: modifier "var" source="var" (external) name_node: identifier "count" source="count" (external) accessor_kind: accessor_kind "get" source="get" return_type: identifier "Int" source="Int" (external) diff --git a/unified/ql/test/library-tests/dataflow/accessors_static.swift b/unified/ql/test/library-tests/dataflow/accessors_static.swift index 7bd37efa7611..42a9299164b8 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_static.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_static.swift @@ -7,7 +7,7 @@ func t1() { class C { static var x: String { source("t1.1") } } - sink(C.x) // $ MISSING: hasValueFlow=t1.1 + sink(C.x) // $ hasValueFlow=t1.1 } func t2() { @@ -18,7 +18,7 @@ func t2() { get { source("t2.1") } } } - sink(C.x) // $ MISSING: hasValueFlow=t2.1 + sink(C.x) // $ hasValueFlow=t2.1 } func t3() { @@ -28,14 +28,14 @@ func t3() { set {} } } - sink(C.x) // $ MISSING: hasValueFlow=t3.1 + sink(C.x) // $ hasValueFlow=t3.1 } func t4() { class C { static var x: String { get {} - set { sink(newValue) } // $ MISSING: hasValueFlow=t4.1 + set { sink(newValue) } // $ hasValueFlow=t4.1 } } C.x = source("t4.1") @@ -45,7 +45,7 @@ func t5() { class C { static var x: String { get { source("t5.1") } - set { sink(newValue) } // $ MISSING: hasValueFlow=t5.1 + set { sink(newValue) } // $ hasValueFlow=t5.1 } } C.x = C.x @@ -56,11 +56,11 @@ func t6() { static var x: String { source("t6.1") } func method() { - sink(Self.x) // $ MISSING: hasValueFlow=t6.1 + sink(Self.x) // $ hasValueFlow=t6.1 } static func staticMethod() { - sink(Self.x) // $ MISSING: hasValueFlow=t6.1 + sink(Self.x) // $ hasValueFlow=t6.1 sink(x) // $ hasValueFlow=t6.1 } } diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index c1fdc513c4eb..6ddf924dc507 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -11,6 +11,18 @@ edges | accessors_local.swift:39:9:39:11 | newValue | accessors_local.swift:39:20:39:27 | newValue | provenance | | | accessors_local.swift:41:5:41:5 | x | accessors_local.swift:39:9:39:11 | newValue | provenance | | | accessors_local.swift:41:9:41:9 | x | accessors_local.swift:41:5:41:5 | x | provenance | | +| accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | provenance | | +| accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | provenance | | +| accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | provenance | | +| accessors_static.swift:38:13:38:15 | newValue | accessors_static.swift:38:24:38:31 | newValue | provenance | | +| accessors_static.swift:41:5:41:7 | ... .x | accessors_static.swift:38:13:38:15 | newValue | provenance | | +| accessors_static.swift:41:11:41:24 | source(...) | accessors_static.swift:41:5:41:7 | ... .x | provenance | | +| accessors_static.swift:47:19:47:32 | source(...) | accessors_static.swift:51:11:51:13 | ... .x | provenance | | +| accessors_static.swift:48:13:48:15 | newValue | accessors_static.swift:48:24:48:31 | newValue | provenance | | +| accessors_static.swift:51:5:51:7 | ... .x | accessors_static.swift:48:13:48:15 | newValue | provenance | | +| accessors_static.swift:51:11:51:13 | ... .x | accessors_static.swift:51:5:51:7 | ... .x | provenance | | +| accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:59:18:59:23 | ... .x | provenance | | +| accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:63:18:63:23 | ... .x | provenance | | | accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:64:18:64:18 | x | provenance | | | calls.swift:7:19:7:19 | x | calls.swift:8:14:8:14 | x | provenance | | | calls.swift:10:12:10:25 | source(...) | calls.swift:7:19:7:19 | x | provenance | | @@ -488,7 +500,24 @@ nodes | accessors_local.swift:39:20:39:27 | newValue | semmle.label | newValue | | accessors_local.swift:41:5:41:5 | x | semmle.label | x | | accessors_local.swift:41:9:41:9 | x | semmle.label | x | +| accessors_static.swift:8:32:8:45 | source(...) | semmle.label | source(...) | +| accessors_static.swift:10:10:10:12 | ... .x | semmle.label | ... .x | +| accessors_static.swift:18:19:18:32 | source(...) | semmle.label | source(...) | +| accessors_static.swift:21:10:21:12 | ... .x | semmle.label | ... .x | +| accessors_static.swift:27:19:27:32 | source(...) | semmle.label | source(...) | +| accessors_static.swift:31:10:31:12 | ... .x | semmle.label | ... .x | +| accessors_static.swift:38:13:38:15 | newValue | semmle.label | newValue | +| accessors_static.swift:38:24:38:31 | newValue | semmle.label | newValue | +| accessors_static.swift:41:5:41:7 | ... .x | semmle.label | ... .x | +| accessors_static.swift:41:11:41:24 | source(...) | semmle.label | source(...) | +| accessors_static.swift:47:19:47:32 | source(...) | semmle.label | source(...) | +| accessors_static.swift:48:13:48:15 | newValue | semmle.label | newValue | +| accessors_static.swift:48:24:48:31 | newValue | semmle.label | newValue | +| accessors_static.swift:51:5:51:7 | ... .x | semmle.label | ... .x | +| accessors_static.swift:51:11:51:13 | ... .x | semmle.label | ... .x | | accessors_static.swift:56:32:56:45 | source(...) | semmle.label | source(...) | +| accessors_static.swift:59:18:59:23 | ... .x | semmle.label | ... .x | +| accessors_static.swift:63:18:63:23 | ... .x | semmle.label | ... .x | | accessors_static.swift:64:18:64:18 | x | semmle.label | x | | calls.swift:7:19:7:19 | x | semmle.label | x | | calls.swift:8:14:8:14 | x | semmle.label | x | @@ -1073,6 +1102,13 @@ testFailures | accessors_local.swift:25:10:25:10 | x | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | $@ | accessors_local.swift:22:15:22:28 | source(...) | source(...) | | accessors_local.swift:31:20:31:27 | newValue | accessors_local.swift:33:9:33:22 | source(...) | accessors_local.swift:31:20:31:27 | newValue | $@ | accessors_local.swift:33:9:33:22 | source(...) | source(...) | | accessors_local.swift:39:20:39:27 | newValue | accessors_local.swift:38:15:38:28 | source(...) | accessors_local.swift:39:20:39:27 | newValue | $@ | accessors_local.swift:38:15:38:28 | source(...) | source(...) | +| accessors_static.swift:10:10:10:12 | ... .x | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | $@ | accessors_static.swift:8:32:8:45 | source(...) | source(...) | +| accessors_static.swift:21:10:21:12 | ... .x | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | $@ | accessors_static.swift:18:19:18:32 | source(...) | source(...) | +| accessors_static.swift:31:10:31:12 | ... .x | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | $@ | accessors_static.swift:27:19:27:32 | source(...) | source(...) | +| accessors_static.swift:38:24:38:31 | newValue | accessors_static.swift:41:11:41:24 | source(...) | accessors_static.swift:38:24:38:31 | newValue | $@ | accessors_static.swift:41:11:41:24 | source(...) | source(...) | +| accessors_static.swift:48:24:48:31 | newValue | accessors_static.swift:47:19:47:32 | source(...) | accessors_static.swift:48:24:48:31 | newValue | $@ | accessors_static.swift:47:19:47:32 | source(...) | source(...) | +| accessors_static.swift:59:18:59:23 | ... .x | accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:59:18:59:23 | ... .x | $@ | accessors_static.swift:56:32:56:45 | source(...) | source(...) | +| accessors_static.swift:63:18:63:23 | ... .x | accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:63:18:63:23 | ... .x | $@ | accessors_static.swift:56:32:56:45 | source(...) | source(...) | | accessors_static.swift:64:18:64:18 | x | accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:64:18:64:18 | x | $@ | accessors_static.swift:56:32:56:45 | source(...) | source(...) | | calls.swift:8:14:8:14 | x | calls.swift:10:12:10:25 | source(...) | calls.swift:8:14:8:14 | x | $@ | calls.swift:10:12:10:25 | source(...) | source(...) | | calls.swift:17:10:17:17 | target(...) | calls.swift:15:16:15:29 | source(...) | calls.swift:17:10:17:17 | target(...) | $@ | calls.swift:15:16:15:29 | source(...) | source(...) | From 32979b2c100565e41a03817c2db474719b5708b7 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 13:02:05 +0200 Subject: [PATCH 09/38] unified: Add MemberAccess as union of MemberAccessExpr and UnqualifiedMemberAccess --- .../unified/internal/StaticNameBinding.qll | 19 +++++++++++++++++-- .../internal/dataflow/DataFlowGraph.qll | 4 ++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll b/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll index 58fd36fc5512..199155913d8e 100644 --- a/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll +++ b/unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll @@ -755,10 +755,25 @@ predicate unqualifiedMemberAccess( } module Public { + /** + * An expression that accesses a named member, either with an explicit base (`MemberAccessExpr`) + * or with an implicit base (`UnqualifiedMemberAccess`). + */ + abstract private class MemberAccessImpl extends Expr { + /** Gets the name of the member being accessed. */ + abstract string getMemberName(); + } + + final class MemberAccess = MemberAccessImpl; + + private class MemberAccessExprImpl extends MemberAccessImpl instanceof MemberAccessExpr { + override string getMemberName() { result = MemberAccessExpr.super.getMemberName() } + } + /** * A name node appearing in an unqualified position, referring to a member of an enclosing class. */ - class UnqualifiedMemberAccess extends Identifier { + class UnqualifiedMemberAccess extends Identifier, MemberAccessImpl { private boolean instanceAccess; private NameBinding target; private ClassLikeDeclaration accessingClass; @@ -783,7 +798,7 @@ module Public { } /** Gets the simple name of this identifier, that is, the name of the member being accessed. */ - string getName() { result = this.getValue() } + override string getMemberName() { result = this.getValue() } } } diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll index a4f45f3277d5..d96ffb4e8a95 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll @@ -92,11 +92,11 @@ predicate step(BuilderNode node1, Step step, BuilderNode node2) { or exists(UnqualifiedMemberAccess access | access.isInstanceAccess() | node1.isLocalVariableRead(access, access.getImplicitQualifierVariable()) and - step.readName(access.getName()) and + step.readName(access.getMemberName()) and node2.isResultValue(access) or (node1.isIncomingValue(access) or node1.isPostUpdate(access)) and - step.storeName(access.getName()) and + step.storeName(access.getMemberName()) and node2.isLocalVariablePostUpdate(access, access.getImplicitQualifierVariable()) ) or From 364e98219e17a03aa57d836672f0bf489f4924d5 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 22:04:43 +0200 Subject: [PATCH 10/38] unified: Also resolve accessors on instances --- .../unified/internal/dataflow/CallGraph.qll | 8 +++++ .../internal/dataflow/DataFlowCall.qll | 12 +++++-- .../unified/internal/typeinference/Type.qll | 3 ++ .../dataflow/accessors_instance.swift | 12 +++---- .../test/library-tests/dataflow/test.expected | 33 +++++++++++++++++++ 5 files changed, 60 insertions(+), 8 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll index 38226b6e497c..04672886c699 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll @@ -16,4 +16,12 @@ DataFlowCallable viableCallable(DataFlowCall c) { private AccessorDeclaration resolveAccessor(Expr access) { result.getNameNode() = N::getStaticBindingTargetFromRef(access) + or + not exists(N::getStaticBindingTargetFromRef(access)) and + exists(MemberAccessExpr m, N::NamespaceNode namespace | + access = m and + // TODO: Make type inference own accessor-resolution. It also needs it internally. + namespace.isInstanceMemberNamespace(T::inferType(m.getBase()).asClass()) and + namespace.getMember(m.getMemberName()).isMember(result) + ) } diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll index b6e18bc960ad..9a12befe8b27 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll @@ -9,11 +9,19 @@ private newtype TDataFlowCall = } or TGetterCall(Expr expr) { hasResultValue(expr) and - N::getStaticBindingTargetFromRef(expr) = any(GetterDeclaration g).getNameNode() + ( + N::getStaticBindingTargetFromRef(expr) = any(GetterDeclaration g).getNameNode() + or + expr instanceof MemberAccess + ) } or TSetterCall(Expr expr) { isInAssignmentContext(expr, _) and - N::getStaticBindingTargetFromRef(expr) = any(SetterDeclaration g).getNameNode() + ( + N::getStaticBindingTargetFromRef(expr) = any(SetterDeclaration g).getNameNode() + or + expr instanceof MemberAccess + ) } or TSummaryCall(SummarizedCallable callable, SummaryNode callee) { FlowSummaryOutput::summaryCallbackRange(callable, callee) diff --git a/unified/ql/lib/codeql/unified/internal/typeinference/Type.qll b/unified/ql/lib/codeql/unified/internal/typeinference/Type.qll index 181e129fc955..0395b24b028a 100644 --- a/unified/ql/lib/codeql/unified/internal/typeinference/Type.qll +++ b/unified/ql/lib/codeql/unified/internal/typeinference/Type.qll @@ -88,6 +88,9 @@ abstract private class TypeImpl extends TType { /** Gets the location of this type. */ abstract Location getLocation(); + + /** Gets the class whose instances are described by this type. */ + ClassLikeDeclaration asClass() { this = TClassLikeDeclarationType(result) } } /** diff --git a/unified/ql/test/library-tests/dataflow/accessors_instance.swift b/unified/ql/test/library-tests/dataflow/accessors_instance.swift index 97a0adee4ead..59e7ec448df3 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_instance.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_instance.swift @@ -7,7 +7,7 @@ func t1() { class C { var x: String { source("t1.1") } } - sink(C().x) // $ MISSING: hasValueFlow=t1.1 + sink(C().x) // $ hasValueFlow=t1.1 } func t2() { @@ -18,7 +18,7 @@ func t2() { get { source("t2.1") } } } - sink(C().x) // $ MISSING: hasValueFlow=t2.1 + sink(C().x) // $ hasValueFlow=t2.1 } func t3() { @@ -28,14 +28,14 @@ func t3() { set {} } } - sink(C().x) // $ MISSING: hasValueFlow=t3.1 + sink(C().x) // $ hasValueFlow=t3.1 } func t4() { class C { var x: String { get {} - set { sink(newValue) } // $ MISSING: hasValueFlow=t4.1 + set { sink(newValue) } // $ hasValueFlow=t4.1 } } C().x = source("t4.1") @@ -45,7 +45,7 @@ func t5() { class C { var x: String { get { source("t5.1") } - set { sink(newValue) } // $ MISSING: hasValueFlow=t5.1 + set { sink(newValue) } // $ hasValueFlow=t5.1 } } let c = C() @@ -57,7 +57,7 @@ func t6() { var x: String { source("t6.1") } func method() { - sink(self.x) // $ MISSING: hasValueFlow=t6.1 + sink(self.x) // $ hasValueFlow=t6.1 sink(x) // $ hasValueFlow=t6.1 } } diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index 6ddf924dc507..68280d55973a 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -1,5 +1,16 @@ models edges +| accessors_instance.swift:8:25:8:38 | source(...) | accessors_instance.swift:10:10:10:14 | ... .x | provenance | | +| accessors_instance.swift:18:19:18:32 | source(...) | accessors_instance.swift:21:10:21:14 | ... .x | provenance | | +| accessors_instance.swift:27:19:27:32 | source(...) | accessors_instance.swift:31:10:31:14 | ... .x | provenance | | +| accessors_instance.swift:38:13:38:15 | newValue | accessors_instance.swift:38:24:38:31 | newValue | provenance | | +| accessors_instance.swift:41:5:41:9 | ... .x | accessors_instance.swift:38:13:38:15 | newValue | provenance | | +| accessors_instance.swift:41:13:41:26 | source(...) | accessors_instance.swift:41:5:41:9 | ... .x | provenance | | +| accessors_instance.swift:47:19:47:32 | source(...) | accessors_instance.swift:52:11:52:13 | ... .x | provenance | | +| accessors_instance.swift:48:13:48:15 | newValue | accessors_instance.swift:48:24:48:31 | newValue | provenance | | +| accessors_instance.swift:52:5:52:7 | ... .x | accessors_instance.swift:48:13:48:15 | newValue | provenance | | +| accessors_instance.swift:52:11:52:13 | ... .x | accessors_instance.swift:52:5:52:7 | ... .x | provenance | | +| accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:60:18:60:23 | ... .x | provenance | | | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | provenance | | | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | | | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | | @@ -483,7 +494,23 @@ edges | test.swift:271:20:271:35 | source(...) | test.swift:271:9:271:9 | x | provenance | | | test.swift:273:30:273:45 | source(...) | test.swift:274:10:274:12 | f(...) | provenance | | nodes +| accessors_instance.swift:8:25:8:38 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:10:10:10:14 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:18:19:18:32 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:21:10:21:14 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:27:19:27:32 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:31:10:31:14 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:38:13:38:15 | newValue | semmle.label | newValue | +| accessors_instance.swift:38:24:38:31 | newValue | semmle.label | newValue | +| accessors_instance.swift:41:5:41:9 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:41:13:41:26 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:47:19:47:32 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:48:13:48:15 | newValue | semmle.label | newValue | +| accessors_instance.swift:48:24:48:31 | newValue | semmle.label | newValue | +| accessors_instance.swift:52:5:52:7 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:52:11:52:13 | ... .x | semmle.label | ... .x | | accessors_instance.swift:57:25:57:38 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:60:18:60:23 | ... .x | semmle.label | ... .x | | accessors_instance.swift:61:18:61:18 | x | semmle.label | x | | accessors_local.swift:7:21:7:34 | source(...) | semmle.label | source(...) | | accessors_local.swift:8:10:8:10 | x | semmle.label | x | @@ -1096,6 +1123,12 @@ subpaths | test.swift:214:43:214:57 | source(...) | test.swift:193:30:193:30 | s | test.swift:193:73:193:73 | s | test.swift:214:21:214:58 | asyncThrowingIdentity(...) | testFailures #select +| accessors_instance.swift:10:10:10:14 | ... .x | accessors_instance.swift:8:25:8:38 | source(...) | accessors_instance.swift:10:10:10:14 | ... .x | $@ | accessors_instance.swift:8:25:8:38 | source(...) | source(...) | +| accessors_instance.swift:21:10:21:14 | ... .x | accessors_instance.swift:18:19:18:32 | source(...) | accessors_instance.swift:21:10:21:14 | ... .x | $@ | accessors_instance.swift:18:19:18:32 | source(...) | source(...) | +| accessors_instance.swift:31:10:31:14 | ... .x | accessors_instance.swift:27:19:27:32 | source(...) | accessors_instance.swift:31:10:31:14 | ... .x | $@ | accessors_instance.swift:27:19:27:32 | source(...) | source(...) | +| accessors_instance.swift:38:24:38:31 | newValue | accessors_instance.swift:41:13:41:26 | source(...) | accessors_instance.swift:38:24:38:31 | newValue | $@ | accessors_instance.swift:41:13:41:26 | source(...) | source(...) | +| accessors_instance.swift:48:24:48:31 | newValue | accessors_instance.swift:47:19:47:32 | source(...) | accessors_instance.swift:48:24:48:31 | newValue | $@ | accessors_instance.swift:47:19:47:32 | source(...) | source(...) | +| accessors_instance.swift:60:18:60:23 | ... .x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:60:18:60:23 | ... .x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) | | accessors_instance.swift:61:18:61:18 | x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) | | accessors_local.swift:8:10:8:10 | x | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | $@ | accessors_local.swift:7:21:7:34 | source(...) | source(...) | | accessors_local.swift:17:10:17:10 | x | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | $@ | accessors_local.swift:15:15:15:28 | source(...) | source(...) | From 9da580f41c401f9c3b6d7b5195e7d7a8a412b75f Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 23 Oct 2026 22:38:49 +0200 Subject: [PATCH 11/38] unified: Make accessors introduce LocalVariables too --- .../unified/internal/LocalNameBinding.qll | 20 +++++++++++++++++++ .../unified/internal/dataflow/CaptureSsa.qll | 2 +- .../unified/internal/dataflow/LocalSsa.qll | 2 +- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll b/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll index e4a179741207..9db28d702497 100644 --- a/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll +++ b/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll @@ -409,6 +409,7 @@ module Public { | decl instanceof VariableDeclaration or decl instanceof FunctionDeclaration or // treat functions as values + decl instanceof AccessorDeclaration or decl instanceof Parameter or decl instanceof ForEachStmt or decl instanceof PatternGuardExpr or @@ -446,6 +447,25 @@ module Public { } } + private AccessorDeclaration getAnAccessor(LocalVariable v) { + v.getABinding() = result.getNameNode() and + (result instanceof GetterDeclaration or result instanceof SetterDeclaration) + } + + /** A local variable defined by a getter/setter pair, which does not store a value on its own. */ + final class ComputedLocalVariable extends LocalVariable { + ComputedLocalVariable() { exists(getAnAccessor(this)) } + + /** Gets the getter or setter for this computed variable. */ + AccessorDeclaration getAnAccessor() { result = getAnAccessor(this) } + + /** Gets the getter for this computed variable. */ + GetterDeclaration getGetterDeclaration() { result = getAnAccessor(this) } + + /** Gets the setter for this computed variable. */ + SetterDeclaration getSetterDeclaration() { result = getAnAccessor(this) } + } + /** An access to a locally-declared name. */ class LocalNameAccess extends PotentialLocalNameAccess { LocalNameAccess() { not this instanceof UnqualifiedMemberAccess } diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll index cd74ad53eafd..5d71027df905 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll @@ -21,7 +21,7 @@ module CaptureSsaInput implements InputSig { Callable basicBlockGetEnclosingCallable(BasicBlock bb) { result = bb.getEnclosingCallable() } class CapturedVariable extends LocalVariable { - CapturedVariable() { this.isCaptured() } + CapturedVariable() { this.isCaptured() and not this instanceof ComputedLocalVariable } Callable getCallable() { result = super.getDeclaringCallable() } } diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/LocalSsa.qll b/unified/ql/lib/codeql/unified/internal/dataflow/LocalSsa.qll index 2a92691a6347..55ae2926b438 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/LocalSsa.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/LocalSsa.qll @@ -10,7 +10,7 @@ private import codeql.util.Void module LocalSsaInput implements InputSig { class SourceVariable extends LocalVariable { - SourceVariable() { not this.isCaptured() } + SourceVariable() { not this.isCaptured() and not this instanceof ComputedLocalVariable } } predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) { From 34de9101a92bf066f4d53242cce10518e32251aa Mon Sep 17 00:00:00 2001 From: Asger F Date: Sat, 24 Oct 2026 10:47:36 +0200 Subject: [PATCH 12/38] unified: Handle capturing local getters/setters This does the minimal change needed to get things working, but it's not pretty. A later commit will refactor this to have less ad-hoc logic here. --- .../unified/internal/dataflow/CaptureSsa.qll | 23 ++++++++++++++++++- .../dataflow/accessors_local.swift | 4 ++-- .../test/library-tests/dataflow/test.expected | 18 +++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll index 5d71027df905..0ec03e5f7fd9 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll @@ -35,7 +35,7 @@ module CaptureSsaInput implements InputSig { CapturedParameter() { none() } } - private class TExpr = TCallableNode or TLocalVariableRefNode; + private class TExpr = TCallableNode or TLocalVariableRefNode or TImplicitArgumentNode; private predicate isTopLevelCallable(BuilderNode callable) { callable.isCallable(any(TopLevel t)) @@ -55,6 +55,14 @@ module CaptureSsaInput implements InputSig { this = TCallableNode(callable) and bb.getNode(i).injects(callable.asSourceCallable()) ) + or + // We need to associate a CFG node with the callee argument passed to a capturing getter/setter, + // so it can carry captured data in and out the getter/setter. + exists(DataFlowCall call | this = TImplicitArgumentNode(call, _, false) | + performsVariableAccess(call.asGetterCall(), _, TRead(), bb.getNode(i)) + or + performsVariableAccess(call.asSetterCall(), _, TWrite(), bb.getNode(i)) + ) } } @@ -114,6 +122,19 @@ module CaptureSsaInput implements InputSig { predicate hasAliasedAccess(Expr f) { // TODO: Restrict to places where the function is called or escapes into a call where it might get called. callableHasLocalAlias(callable, f) and not f.(LocalVariableRefNode).getRefKind().isWrite() + or + // Record the callee argument passed into a local capturing getter/setter as an alias for that getter/setter + // so it can carry capture data in and out of the getter/setter call. + exists(ComputedLocalVariable local, DataFlowCall call | + callable = local.getAnAccessor() and + f = TImplicitArgumentNode(call, any(ArgumentPosition p | p.isCallee()), false) + | + callable instanceof GetterDeclaration and + call.asGetterCall() = local.getAnAccess() + or + callable instanceof SetterDeclaration and + call.asSetterCall() = local.getAnAccess() + ) } } } diff --git a/unified/ql/test/library-tests/dataflow/accessors_local.swift b/unified/ql/test/library-tests/dataflow/accessors_local.swift index a54e8f178b26..1ec43b376c00 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_local.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_local.swift @@ -44,7 +44,7 @@ func t5() { func t6() { var y: String = source("t6.1") var x: String { y } - sink(x) // $ MISSING: hasValueFlow=t6.1 + sink(x) // $ hasValueFlow=t6.1 } func t7() { @@ -52,5 +52,5 @@ func t7() { var x: String { y } sink(x) // no flow y = source("t7.1") - sink(x) // $ MISSING: hasValueFlow=t7.1 + sink(x) // $ hasValueFlow=t7.1 } diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index 68280d55973a..d4508afb8b8e 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -22,6 +22,12 @@ edges | accessors_local.swift:39:9:39:11 | newValue | accessors_local.swift:39:20:39:27 | newValue | provenance | | | accessors_local.swift:41:5:41:5 | x | accessors_local.swift:39:9:39:11 | newValue | provenance | | | accessors_local.swift:41:9:41:9 | x | accessors_local.swift:41:5:41:5 | x | provenance | | +| accessors_local.swift:45:9:45:9 | y | accessors_local.swift:46:21:46:21 | y | provenance | | +| accessors_local.swift:45:9:45:9 | y | accessors_local.swift:47:10:47:10 | x | provenance | | +| accessors_local.swift:45:21:45:34 | source(...) | accessors_local.swift:45:9:45:9 | y | provenance | | +| accessors_local.swift:54:5:54:5 | y | accessors_local.swift:52:21:52:21 | y | provenance | | +| accessors_local.swift:54:5:54:5 | y | accessors_local.swift:55:10:55:10 | x | provenance | | +| accessors_local.swift:54:9:54:22 | source(...) | accessors_local.swift:54:5:54:5 | y | provenance | | | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | provenance | | | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | provenance | | | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | provenance | | @@ -527,6 +533,14 @@ nodes | accessors_local.swift:39:20:39:27 | newValue | semmle.label | newValue | | accessors_local.swift:41:5:41:5 | x | semmle.label | x | | accessors_local.swift:41:9:41:9 | x | semmle.label | x | +| accessors_local.swift:45:9:45:9 | y | semmle.label | y | +| accessors_local.swift:45:21:45:34 | source(...) | semmle.label | source(...) | +| accessors_local.swift:46:21:46:21 | y | semmle.label | y | +| accessors_local.swift:47:10:47:10 | x | semmle.label | x | +| accessors_local.swift:52:21:52:21 | y | semmle.label | y | +| accessors_local.swift:54:5:54:5 | y | semmle.label | y | +| accessors_local.swift:54:9:54:22 | source(...) | semmle.label | source(...) | +| accessors_local.swift:55:10:55:10 | x | semmle.label | x | | accessors_static.swift:8:32:8:45 | source(...) | semmle.label | source(...) | | accessors_static.swift:10:10:10:12 | ... .x | semmle.label | ... .x | | accessors_static.swift:18:19:18:32 | source(...) | semmle.label | source(...) | @@ -1105,6 +1119,8 @@ nodes | test.swift:273:30:273:45 | source(...) | semmle.label | source(...) | | test.swift:274:10:274:12 | f(...) | semmle.label | f(...) | subpaths +| accessors_local.swift:45:9:45:9 | y | accessors_local.swift:46:21:46:21 | y | accessors_local.swift:46:21:46:21 | y | accessors_local.swift:47:10:47:10 | x | +| accessors_local.swift:54:5:54:5 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:55:10:55:10 | x | | calls.swift:31:17:31:30 | source(...) | calls.swift:28:19:28:19 | x | calls.swift:29:16:29:24 | ... + ... | calls.swift:31:10:31:31 | target(...) | | calls.swift:32:17:32:30 | source(...) | calls.swift:28:19:28:19 | x | calls.swift:29:16:29:24 | ... + ... | calls.swift:32:10:32:31 | target(...) | | capture.swift:37:5:37:11 | closure [x] | capture.swift:32:14:32:14 | x | capture.swift:32:9:32:10 | y1 | capture.swift:38:10:38:11 | y1 | @@ -1135,6 +1151,8 @@ testFailures | accessors_local.swift:25:10:25:10 | x | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | $@ | accessors_local.swift:22:15:22:28 | source(...) | source(...) | | accessors_local.swift:31:20:31:27 | newValue | accessors_local.swift:33:9:33:22 | source(...) | accessors_local.swift:31:20:31:27 | newValue | $@ | accessors_local.swift:33:9:33:22 | source(...) | source(...) | | accessors_local.swift:39:20:39:27 | newValue | accessors_local.swift:38:15:38:28 | source(...) | accessors_local.swift:39:20:39:27 | newValue | $@ | accessors_local.swift:38:15:38:28 | source(...) | source(...) | +| accessors_local.swift:47:10:47:10 | x | accessors_local.swift:45:21:45:34 | source(...) | accessors_local.swift:47:10:47:10 | x | $@ | accessors_local.swift:45:21:45:34 | source(...) | source(...) | +| accessors_local.swift:55:10:55:10 | x | accessors_local.swift:54:9:54:22 | source(...) | accessors_local.swift:55:10:55:10 | x | $@ | accessors_local.swift:54:9:54:22 | source(...) | source(...) | | accessors_static.swift:10:10:10:12 | ... .x | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | $@ | accessors_static.swift:8:32:8:45 | source(...) | source(...) | | accessors_static.swift:21:10:21:12 | ... .x | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | $@ | accessors_static.swift:18:19:18:32 | source(...) | source(...) | | accessors_static.swift:31:10:31:12 | ... .x | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | $@ | accessors_static.swift:27:19:27:32 | source(...) | source(...) | From 5c9942f3379a712496b104ae588a028b45a97028 Mon Sep 17 00:00:00 2001 From: Asger F Date: Sat, 24 Oct 2026 15:16:18 +0200 Subject: [PATCH 13/38] unified: Propagate receiver argument into accessor calls --- .../internal/dataflow/DataFlowGraph.qll | 22 +++++++++++++++++++ .../internal/dataflow/DataFlowNode.qll | 14 +++++++++--- .../dataflow/accessors_instance.swift | 2 +- .../test/library-tests/dataflow/test.expected | 15 +++++++++++++ 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll index d96ffb4e8a95..098d444a99d8 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll @@ -98,6 +98,17 @@ predicate step(BuilderNode node1, Step step, BuilderNode node2) { (node1.isIncomingValue(access) or node1.isPostUpdate(access)) and step.storeName(access.getMemberName()) and node2.isLocalVariablePostUpdate(access, access.getImplicitQualifierVariable()) + or + // Pass the receiver along to the getter/setter invoked by this expression + exists(DataFlowCall call | access = [call.asGetterCall(), call.asSetterCall()] | + node1.isLocalVariableRead(access, access.getImplicitQualifierVariable()) and + step.value() and + node2.isReceiverArgumentEx(call) + or + node1.isReceiverPostUpdateEx(call) and + step.value() and + node2.isLocalVariablePostUpdate(access, access.getImplicitQualifierVariable()) + ) ) or exists(StringInterpolationExpr expr | @@ -124,6 +135,17 @@ predicate step(BuilderNode node1, Step step, BuilderNode node2) { (node1.isIncomingValue(expr) or node1.isPostUpdate(expr)) and step.storeName(expr.getMemberName()) and node2.isPostUpdate(expr.getBase()) + or + // Pass the receiver along to the getter/setter invoked by this expression + exists(DataFlowCall call | expr = [call.asGetterCall(), call.asSetterCall()] | + node1.isResultValue(expr.getBase()) and + step.value() and + node2.isReceiverArgumentEx(call) + or + node1.isReceiverPostUpdateEx(call) and + step.value() and + node2.isPostUpdate(expr.getBase()) + ) ) or // Calls and constructor-patterns targeting an enum-case constructor. diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll index b4cf63992ece..309e1bb26e4a 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll @@ -186,13 +186,21 @@ class BuilderNode extends TDataFlowNodeStage1 { } /** Holds if this node represents the receiver argument passed to `call`. */ - predicate isReceiverArgument(CallExpr call) { - this.isImplicitArgument(toDataFlowCall(call), any(ArgumentPosition p | p.isReceiver()), false) + predicate isReceiverArgumentEx(DataFlowCall call) { + this.isImplicitArgument(call, any(ArgumentPosition p | p.isReceiver()), false) } + /** Holds if this node represents the receiver argument passed to `call`. */ + predicate isReceiverArgument(CallExpr call) { this.isReceiverArgumentEx(toDataFlowCall(call)) } + /** Holds if this node represents the updated state of the receiver of `call` after the call returns. */ predicate isReceiverPostUpdate(CallExpr call) { - this.isImplicitArgument(toDataFlowCall(call), any(ArgumentPosition p | p.isReceiver()), true) + this.isReceiverPostUpdateEx(toDataFlowCall(call)) + } + + /** Holds if this node represents the updated state of the receiver of `call` after the call returns. */ + predicate isReceiverPostUpdateEx(DataFlowCall call) { + this.isImplicitArgument(call, any(ArgumentPosition p | p.isReceiver()), true) } /** Holds if this is the canonical representative for the given `callable`. */ diff --git a/unified/ql/test/library-tests/dataflow/accessors_instance.swift b/unified/ql/test/library-tests/dataflow/accessors_instance.swift index 59e7ec448df3..dbdafd719ae7 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_instance.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_instance.swift @@ -71,7 +71,7 @@ func t7() { var c = C() sink(c.x) // no flow c.field = source("t7.1") - sink(c.x) // $ MISSING: hasValueFlow=t7.1 + sink(c.x) // $ hasValueFlow=t7.1 } func t8() { diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index d4508afb8b8e..6004725ef153 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -12,6 +12,12 @@ edges | accessors_instance.swift:52:11:52:13 | ... .x | accessors_instance.swift:52:5:52:7 | ... .x | provenance | | | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:60:18:60:23 | ... .x | provenance | | | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | provenance | | +| accessors_instance.swift:69:25:69:28 | self [field] | accessors_instance.swift:69:25:69:34 | ... .field | provenance | | +| accessors_instance.swift:73:5:73:5 | [post] c [field] | accessors_instance.swift:74:10:74:10 | c [field] | provenance | | +| accessors_instance.swift:73:5:73:11 | ... .field | accessors_instance.swift:73:5:73:5 | [post] c [field] | provenance | | +| accessors_instance.swift:73:15:73:28 | source(...) | accessors_instance.swift:73:5:73:11 | ... .field | provenance | | +| accessors_instance.swift:74:10:74:10 | c [field] | accessors_instance.swift:69:25:69:28 | self [field] | provenance | | +| accessors_instance.swift:74:10:74:10 | c [field] | accessors_instance.swift:74:10:74:12 | ... .x | provenance | | | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | | | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | | | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | provenance | | @@ -518,6 +524,13 @@ nodes | accessors_instance.swift:57:25:57:38 | source(...) | semmle.label | source(...) | | accessors_instance.swift:60:18:60:23 | ... .x | semmle.label | ... .x | | accessors_instance.swift:61:18:61:18 | x | semmle.label | x | +| accessors_instance.swift:69:25:69:28 | self [field] | semmle.label | self [field] | +| accessors_instance.swift:69:25:69:34 | ... .field | semmle.label | ... .field | +| accessors_instance.swift:73:5:73:5 | [post] c [field] | semmle.label | [post] c [field] | +| accessors_instance.swift:73:5:73:11 | ... .field | semmle.label | ... .field | +| accessors_instance.swift:73:15:73:28 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:74:10:74:10 | c [field] | semmle.label | c [field] | +| accessors_instance.swift:74:10:74:12 | ... .x | semmle.label | ... .x | | accessors_local.swift:7:21:7:34 | source(...) | semmle.label | source(...) | | accessors_local.swift:8:10:8:10 | x | semmle.label | x | | accessors_local.swift:15:15:15:28 | source(...) | semmle.label | source(...) | @@ -1119,6 +1132,7 @@ nodes | test.swift:273:30:273:45 | source(...) | semmle.label | source(...) | | test.swift:274:10:274:12 | f(...) | semmle.label | f(...) | subpaths +| accessors_instance.swift:74:10:74:10 | c [field] | accessors_instance.swift:69:25:69:28 | self [field] | accessors_instance.swift:69:25:69:34 | ... .field | accessors_instance.swift:74:10:74:12 | ... .x | | accessors_local.swift:45:9:45:9 | y | accessors_local.swift:46:21:46:21 | y | accessors_local.swift:46:21:46:21 | y | accessors_local.swift:47:10:47:10 | x | | accessors_local.swift:54:5:54:5 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:55:10:55:10 | x | | calls.swift:31:17:31:30 | source(...) | calls.swift:28:19:28:19 | x | calls.swift:29:16:29:24 | ... + ... | calls.swift:31:10:31:31 | target(...) | @@ -1146,6 +1160,7 @@ testFailures | accessors_instance.swift:48:24:48:31 | newValue | accessors_instance.swift:47:19:47:32 | source(...) | accessors_instance.swift:48:24:48:31 | newValue | $@ | accessors_instance.swift:47:19:47:32 | source(...) | source(...) | | accessors_instance.swift:60:18:60:23 | ... .x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:60:18:60:23 | ... .x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) | | accessors_instance.swift:61:18:61:18 | x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) | +| accessors_instance.swift:74:10:74:12 | ... .x | accessors_instance.swift:73:15:73:28 | source(...) | accessors_instance.swift:74:10:74:12 | ... .x | $@ | accessors_instance.swift:73:15:73:28 | source(...) | source(...) | | accessors_local.swift:8:10:8:10 | x | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | $@ | accessors_local.swift:7:21:7:34 | source(...) | source(...) | | accessors_local.swift:17:10:17:10 | x | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | $@ | accessors_local.swift:15:15:15:28 | source(...) | source(...) | | accessors_local.swift:25:10:25:10 | x | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | $@ | accessors_local.swift:22:15:22:28 | source(...) | source(...) | From b3488034f0d0a4662ad168630b96caa2f48fa8cf Mon Sep 17 00:00:00 2001 From: Asger F Date: Sat, 24 Oct 2026 15:17:16 +0200 Subject: [PATCH 14/38] unified: Allow self-return via receiver parameter --- .../internal/dataflow/DataFlowInstantiation.qll | 2 ++ .../dataflow/accessors_instance.swift | 2 +- .../test/library-tests/dataflow/test.expected | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll index 8f9477b4749a..03b92a35e80e 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll @@ -272,6 +272,8 @@ module DataFlowInput implements InputSig { predicate isUnreachableInCall(NodeRegion nr, DataFlowCall call) { none() } // TODO predicate allowParameterReturnInSelf(ParameterNode p) { + p.isReceiverParameter(_) + or captureSsaAllowParameterReturnInSelf(p) or exists(DataFlowCallable c, ParameterPosition pos | diff --git a/unified/ql/test/library-tests/dataflow/accessors_instance.swift b/unified/ql/test/library-tests/dataflow/accessors_instance.swift index dbdafd719ae7..9a3a2bfb9e5e 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_instance.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_instance.swift @@ -86,5 +86,5 @@ func t8() { var c = C() c.field = source("t8.1") c.x = "safe" // trigger the setter - sink(c.otherField) // $ MISSING: hasTaintFlow=t8.1 + sink(c.otherField) // $ hasTaintFlow=t8.1 } diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index 6004725ef153..1612261b4ac5 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -18,6 +18,13 @@ edges | accessors_instance.swift:73:15:73:28 | source(...) | accessors_instance.swift:73:5:73:11 | ... .field | provenance | | | accessors_instance.swift:74:10:74:10 | c [field] | accessors_instance.swift:69:25:69:28 | self [field] | provenance | | | accessors_instance.swift:74:10:74:10 | c [field] | accessors_instance.swift:74:10:74:12 | ... .x | provenance | | +| accessors_instance.swift:83:32:83:36 | field | accessors_instance.swift:83:19:83:28 | otherField | provenance | | +| accessors_instance.swift:87:5:87:5 | [post] c [field] | accessors_instance.swift:88:5:88:5 | c [field] | provenance | | +| accessors_instance.swift:87:5:87:11 | ... .field | accessors_instance.swift:87:5:87:5 | [post] c [field] | provenance | | +| accessors_instance.swift:87:15:87:28 | source(...) | accessors_instance.swift:87:5:87:11 | ... .field | provenance | | +| accessors_instance.swift:88:5:88:5 | c [field] | accessors_instance.swift:83:32:83:36 | field | provenance | | +| accessors_instance.swift:88:5:88:5 | c [field] | accessors_instance.swift:89:10:89:10 | c [otherField] | provenance | | +| accessors_instance.swift:89:10:89:10 | c [otherField] | accessors_instance.swift:89:10:89:21 | ... .otherField | provenance | | | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | | | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | | | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | provenance | | @@ -531,6 +538,14 @@ nodes | accessors_instance.swift:73:15:73:28 | source(...) | semmle.label | source(...) | | accessors_instance.swift:74:10:74:10 | c [field] | semmle.label | c [field] | | accessors_instance.swift:74:10:74:12 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:83:19:83:28 | otherField | semmle.label | otherField | +| accessors_instance.swift:83:32:83:36 | field | semmle.label | field | +| accessors_instance.swift:87:5:87:5 | [post] c [field] | semmle.label | [post] c [field] | +| accessors_instance.swift:87:5:87:11 | ... .field | semmle.label | ... .field | +| accessors_instance.swift:87:15:87:28 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:88:5:88:5 | c [field] | semmle.label | c [field] | +| accessors_instance.swift:89:10:89:10 | c [otherField] | semmle.label | c [otherField] | +| accessors_instance.swift:89:10:89:21 | ... .otherField | semmle.label | ... .otherField | | accessors_local.swift:7:21:7:34 | source(...) | semmle.label | source(...) | | accessors_local.swift:8:10:8:10 | x | semmle.label | x | | accessors_local.swift:15:15:15:28 | source(...) | semmle.label | source(...) | @@ -1133,6 +1148,7 @@ nodes | test.swift:274:10:274:12 | f(...) | semmle.label | f(...) | subpaths | accessors_instance.swift:74:10:74:10 | c [field] | accessors_instance.swift:69:25:69:28 | self [field] | accessors_instance.swift:69:25:69:34 | ... .field | accessors_instance.swift:74:10:74:12 | ... .x | +| accessors_instance.swift:88:5:88:5 | c [field] | accessors_instance.swift:83:32:83:36 | field | accessors_instance.swift:83:19:83:28 | otherField | accessors_instance.swift:89:10:89:10 | c [otherField] | | accessors_local.swift:45:9:45:9 | y | accessors_local.swift:46:21:46:21 | y | accessors_local.swift:46:21:46:21 | y | accessors_local.swift:47:10:47:10 | x | | accessors_local.swift:54:5:54:5 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:55:10:55:10 | x | | calls.swift:31:17:31:30 | source(...) | calls.swift:28:19:28:19 | x | calls.swift:29:16:29:24 | ... + ... | calls.swift:31:10:31:31 | target(...) | @@ -1161,6 +1177,7 @@ testFailures | accessors_instance.swift:60:18:60:23 | ... .x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:60:18:60:23 | ... .x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) | | accessors_instance.swift:61:18:61:18 | x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) | | accessors_instance.swift:74:10:74:12 | ... .x | accessors_instance.swift:73:15:73:28 | source(...) | accessors_instance.swift:74:10:74:12 | ... .x | $@ | accessors_instance.swift:73:15:73:28 | source(...) | source(...) | +| accessors_instance.swift:89:10:89:21 | ... .otherField | accessors_instance.swift:87:15:87:28 | source(...) | accessors_instance.swift:89:10:89:21 | ... .otherField | $@ | accessors_instance.swift:87:15:87:28 | source(...) | source(...) | | accessors_local.swift:8:10:8:10 | x | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | $@ | accessors_local.swift:7:21:7:34 | source(...) | source(...) | | accessors_local.swift:17:10:17:10 | x | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | $@ | accessors_local.swift:15:15:15:28 | source(...) | source(...) | | accessors_local.swift:25:10:25:10 | x | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | $@ | accessors_local.swift:22:15:22:28 | source(...) | source(...) | From edc465e372ab8cdca41545d275d22463eae0aa9f Mon Sep 17 00:00:00 2001 From: Asger F Date: Sat, 24 Oct 2026 15:20:02 +0200 Subject: [PATCH 15/38] unified: Add more capture tests --- .../dataflow/accessors_local.swift | 18 ++++++++++++++++++ .../test/library-tests/dataflow/test.expected | 12 ++++++++++++ 2 files changed, 30 insertions(+) diff --git a/unified/ql/test/library-tests/dataflow/accessors_local.swift b/unified/ql/test/library-tests/dataflow/accessors_local.swift index 1ec43b376c00..e283f8bffe75 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_local.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_local.swift @@ -54,3 +54,21 @@ func t7() { y = source("t7.1") sink(x) // $ hasValueFlow=t7.1 } + +func t8() { + var x: String { source("t8.1") } + let closure = { + sink(x) // $ hasValueFlow=t8.1 + } + closure() +} + +func t9() { + var y: String = "safe" + var x: String { y } + let closure = { + sink(x) // $ hasValueFlow=t9.1 + } + y = source("t9.1") + closure() +} diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index 1612261b4ac5..b6567bf507ae 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -41,6 +41,10 @@ edges | accessors_local.swift:54:5:54:5 | y | accessors_local.swift:52:21:52:21 | y | provenance | | | accessors_local.swift:54:5:54:5 | y | accessors_local.swift:55:10:55:10 | x | provenance | | | accessors_local.swift:54:9:54:22 | source(...) | accessors_local.swift:54:5:54:5 | y | provenance | | +| accessors_local.swift:59:21:59:34 | source(...) | accessors_local.swift:61:14:61:14 | x | provenance | | +| accessors_local.swift:72:5:72:5 | y | accessors_local.swift:73:5:73:11 | closure [y] | provenance | | +| accessors_local.swift:72:9:72:22 | source(...) | accessors_local.swift:72:5:72:5 | y | provenance | | +| accessors_local.swift:73:5:73:11 | closure [y] | accessors_local.swift:70:14:70:14 | x | provenance | | | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | provenance | | | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | provenance | | | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | provenance | | @@ -569,6 +573,12 @@ nodes | accessors_local.swift:54:5:54:5 | y | semmle.label | y | | accessors_local.swift:54:9:54:22 | source(...) | semmle.label | source(...) | | accessors_local.swift:55:10:55:10 | x | semmle.label | x | +| accessors_local.swift:59:21:59:34 | source(...) | semmle.label | source(...) | +| accessors_local.swift:61:14:61:14 | x | semmle.label | x | +| accessors_local.swift:70:14:70:14 | x | semmle.label | x | +| accessors_local.swift:72:5:72:5 | y | semmle.label | y | +| accessors_local.swift:72:9:72:22 | source(...) | semmle.label | source(...) | +| accessors_local.swift:73:5:73:11 | closure [y] | semmle.label | closure [y] | | accessors_static.swift:8:32:8:45 | source(...) | semmle.label | source(...) | | accessors_static.swift:10:10:10:12 | ... .x | semmle.label | ... .x | | accessors_static.swift:18:19:18:32 | source(...) | semmle.label | source(...) | @@ -1185,6 +1195,8 @@ testFailures | accessors_local.swift:39:20:39:27 | newValue | accessors_local.swift:38:15:38:28 | source(...) | accessors_local.swift:39:20:39:27 | newValue | $@ | accessors_local.swift:38:15:38:28 | source(...) | source(...) | | accessors_local.swift:47:10:47:10 | x | accessors_local.swift:45:21:45:34 | source(...) | accessors_local.swift:47:10:47:10 | x | $@ | accessors_local.swift:45:21:45:34 | source(...) | source(...) | | accessors_local.swift:55:10:55:10 | x | accessors_local.swift:54:9:54:22 | source(...) | accessors_local.swift:55:10:55:10 | x | $@ | accessors_local.swift:54:9:54:22 | source(...) | source(...) | +| accessors_local.swift:61:14:61:14 | x | accessors_local.swift:59:21:59:34 | source(...) | accessors_local.swift:61:14:61:14 | x | $@ | accessors_local.swift:59:21:59:34 | source(...) | source(...) | +| accessors_local.swift:70:14:70:14 | x | accessors_local.swift:72:9:72:22 | source(...) | accessors_local.swift:70:14:70:14 | x | $@ | accessors_local.swift:72:9:72:22 | source(...) | source(...) | | accessors_static.swift:10:10:10:12 | ... .x | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | $@ | accessors_static.swift:8:32:8:45 | source(...) | source(...) | | accessors_static.swift:21:10:21:12 | ... .x | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | $@ | accessors_static.swift:18:19:18:32 | source(...) | source(...) | | accessors_static.swift:31:10:31:12 | ... .x | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | $@ | accessors_static.swift:27:19:27:32 | source(...) | source(...) | From 6444d0bf7535795761b3ea7aaedb714ac9d171c3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Sun, 25 Oct 2026 09:16:11 +0100 Subject: [PATCH 16/38] unified: Add test with chain of accessors --- .../library-tests/dataflow/accessors_local.swift | 12 ++++++++++++ unified/ql/test/library-tests/dataflow/test.expected | 8 ++++++++ 2 files changed, 20 insertions(+) diff --git a/unified/ql/test/library-tests/dataflow/accessors_local.swift b/unified/ql/test/library-tests/dataflow/accessors_local.swift index e283f8bffe75..c5b19297e630 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_local.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_local.swift @@ -72,3 +72,15 @@ func t9() { y = source("t9.1") closure() } + +func t10() { + var y: String = "safe" + var x1: String { y } + var x2: String { x1 } + var x3: String { x2 } + let closure = { + sink(x3) // $ hasValueFlow=t9.1 + } + y = source("t9.1") + closure() +} diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index b6567bf507ae..c01df4925f6a 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -45,6 +45,9 @@ edges | accessors_local.swift:72:5:72:5 | y | accessors_local.swift:73:5:73:11 | closure [y] | provenance | | | accessors_local.swift:72:9:72:22 | source(...) | accessors_local.swift:72:5:72:5 | y | provenance | | | accessors_local.swift:73:5:73:11 | closure [y] | accessors_local.swift:70:14:70:14 | x | provenance | | +| accessors_local.swift:84:5:84:5 | y | accessors_local.swift:85:5:85:11 | closure [y] | provenance | | +| accessors_local.swift:84:9:84:22 | source(...) | accessors_local.swift:84:5:84:5 | y | provenance | | +| accessors_local.swift:85:5:85:11 | closure [y] | accessors_local.swift:82:14:82:15 | x3 | provenance | | | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | provenance | | | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | provenance | | | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | provenance | | @@ -579,6 +582,10 @@ nodes | accessors_local.swift:72:5:72:5 | y | semmle.label | y | | accessors_local.swift:72:9:72:22 | source(...) | semmle.label | source(...) | | accessors_local.swift:73:5:73:11 | closure [y] | semmle.label | closure [y] | +| accessors_local.swift:82:14:82:15 | x3 | semmle.label | x3 | +| accessors_local.swift:84:5:84:5 | y | semmle.label | y | +| accessors_local.swift:84:9:84:22 | source(...) | semmle.label | source(...) | +| accessors_local.swift:85:5:85:11 | closure [y] | semmle.label | closure [y] | | accessors_static.swift:8:32:8:45 | source(...) | semmle.label | source(...) | | accessors_static.swift:10:10:10:12 | ... .x | semmle.label | ... .x | | accessors_static.swift:18:19:18:32 | source(...) | semmle.label | source(...) | @@ -1197,6 +1204,7 @@ testFailures | accessors_local.swift:55:10:55:10 | x | accessors_local.swift:54:9:54:22 | source(...) | accessors_local.swift:55:10:55:10 | x | $@ | accessors_local.swift:54:9:54:22 | source(...) | source(...) | | accessors_local.swift:61:14:61:14 | x | accessors_local.swift:59:21:59:34 | source(...) | accessors_local.swift:61:14:61:14 | x | $@ | accessors_local.swift:59:21:59:34 | source(...) | source(...) | | accessors_local.swift:70:14:70:14 | x | accessors_local.swift:72:9:72:22 | source(...) | accessors_local.swift:70:14:70:14 | x | $@ | accessors_local.swift:72:9:72:22 | source(...) | source(...) | +| accessors_local.swift:82:14:82:15 | x3 | accessors_local.swift:84:9:84:22 | source(...) | accessors_local.swift:82:14:82:15 | x3 | $@ | accessors_local.swift:84:9:84:22 | source(...) | source(...) | | accessors_static.swift:10:10:10:12 | ... .x | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | $@ | accessors_static.swift:8:32:8:45 | source(...) | source(...) | | accessors_static.swift:21:10:21:12 | ... .x | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | $@ | accessors_static.swift:18:19:18:32 | source(...) | source(...) | | accessors_static.swift:31:10:31:12 | ... .x | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | $@ | accessors_static.swift:27:19:27:32 | source(...) | source(...) | From 4bd9473d91a8e9539cfffce508ca60c5a48b4261 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 26 Oct 2026 12:46:39 +0100 Subject: [PATCH 17/38] unified: Split up hasControlFlowPosition() --- .../internal/dataflow/DataFlowNode.qll | 93 +++++++++++-------- 1 file changed, 55 insertions(+), 38 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll index 309e1bb26e4a..04b16cf21f15 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll @@ -320,6 +320,50 @@ class BuilderNode extends TDataFlowNodeStage1 { result = call.getLocation() ) } + + /** + * Holds if this data flow node is associated with the `i`'th index in the given basic block. + * + * Note that some data flow nodes may have an index appearing before the first or after the + * last `ControlFlowNode` node in the basic block. Multiple data flow nodes may share the same control flow position. + * + * Also note that some data flow nodes have no associated control flow position, either because they are + * in unreachable code, or belong to a synthesized callable that has no control flow graph. + */ + predicate hasControlFlowPosition(BasicBlock bb, int i) { + exists(ControlFlowNode cfgNode | cfgNode = bb.getNode(i) | + exists(Expr expr | + this.isResultValue(expr) and cfgNode.asExpr() = expr + or + this.isIncomingValue(expr) and hasIncomingValueAtCfgNode(expr, cfgNode) + or + this.isPostUpdate(expr) and hasPostUpdate(expr, cfgNode) + ) + or + exists(AstNode repr, LocalVariable var, VariableRefKind kind | + this.isLocalVariableRef(repr, var, kind) and + performsVariableAccess(repr, var, kind, cfgNode) + ) + or + exists(DataFlowCallable callable | + this.isImplicitParameter(callable, _) and + cfgNode.(ControlFlow::EntryNode).getEnclosingCallable() = callable.asSourceCallable() + or + this.isCallableEx(callable) and + cfgNode.injects(callable.asSourceCallable()) + ) + or + exists(DataFlowCall call, CallExpr sourceCall, boolean isPost | + call.asExplicitCall() = sourceCall and + this.isImplicitArgument(call, _, isPost) and + ( + isPost = false and cfgNode.injects(sourceCall) + or + isPost = true and cfgNode.isAfter(sourceCall) + ) + ) + ) + } } /** A node in stage 2, which includes stage 1 and local SSA nodes. */ @@ -343,6 +387,16 @@ class Stage2Node extends TDataFlowNodeStage2 { result = node.getLocation() ) } + + predicate hasControlFlowPosition(BasicBlock bb, int i) { + this.(BuilderNode).hasControlFlowPosition(bb, i) + or + exists(LocalSsaDataFlowOutput::SsaNode node | + this = TLocalSsaNode(node) and + bb = node.getBasicBlock() and + i = node.getIndex() // TODO: why is this marked as internal in the SSA library? + ) + } } class Node extends TDataFlowNode { @@ -491,44 +545,7 @@ class Node extends TDataFlowNode { * in unreachable code, or belong to a synthesized callable that has no control flow graph. */ predicate hasControlFlowPosition(BasicBlock bb, int i) { - exists(ControlFlowNode cfgNode | cfgNode = bb.getNode(i) | - exists(Expr expr | - this.isResultValue(expr) and cfgNode.asExpr() = expr - or - this.isIncomingValue(expr) and hasIncomingValueAtCfgNode(expr, cfgNode) - or - this.isPostUpdate(expr) and hasPostUpdate(expr, cfgNode) - ) - or - exists(AstNode repr, LocalVariable var, VariableRefKind kind | - this.isLocalVariableRef(repr, var, kind) and - performsVariableAccess(repr, var, kind, cfgNode) - ) - or - exists(DataFlowCallable callable | - this.(BuilderNode).isImplicitParameter(callable, _) and - cfgNode.(ControlFlow::EntryNode).getEnclosingCallable() = callable.asSourceCallable() - or - this.isCallableEx(callable) and - cfgNode.injects(callable.asSourceCallable()) - ) - or - exists(DataFlowCall call, CallExpr sourceCall, boolean isPost | - call.asExplicitCall() = sourceCall and - this.(BuilderNode).isImplicitArgument(call, _, isPost) and - ( - isPost = false and cfgNode.injects(sourceCall) - or - isPost = true and cfgNode.isAfter(sourceCall) - ) - ) - ) - or - exists(LocalSsaDataFlowOutput::SsaNode node | - this = TLocalSsaNode(node) and - bb = node.getBasicBlock() and - i = node.getIndex() // TODO: why is this marked as internal in the SSA library? - ) + this.(Stage2Node).hasControlFlowPosition(bb, i) or exists(CaptureSsaOutput::SynthesizedCaptureNode node | this = TCaptureSsaNode(node) and From a808fdeb21e6bea6884d2d90a91ea7ec47925262 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 26 Oct 2026 13:13:39 +0100 Subject: [PATCH 18/38] unified: Associate CFG nodes with implicit getter/setter arguments --- .../unified/internal/dataflow/DataFlowNode.qll | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll index 04b16cf21f15..84b91da3e924 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll @@ -353,13 +353,20 @@ class BuilderNode extends TDataFlowNodeStage1 { cfgNode.injects(callable.asSourceCallable()) ) or - exists(DataFlowCall call, CallExpr sourceCall, boolean isPost | - call.asExplicitCall() = sourceCall and - this.isImplicitArgument(call, _, isPost) and + exists(DataFlowCall call, Expr access, boolean isPost | + this.isImplicitArgument(call, _, isPost) + | ( - isPost = false and cfgNode.injects(sourceCall) + access = call.asExplicitCall() or - isPost = true and cfgNode.isAfter(sourceCall) + access = call.asGetterCall() + or + access = call.asSetterCall() + ) and + ( + isPost = false and cfgNode.injects(access) + or + isPost = true and cfgNode.isAfter(access) ) ) ) From 3f7735fa074018279b16e60a2a9aa90aa2ff8f28 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 26 Oct 2026 13:14:26 +0100 Subject: [PATCH 19/38] unified: Use hasControlFlowPosition in CaptureSsa --- .../unified/internal/dataflow/CaptureSsa.qll | 25 ++----------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll index 0ec03e5f7fd9..b0626e60f247 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll @@ -35,35 +35,14 @@ module CaptureSsaInput implements InputSig { CapturedParameter() { none() } } - private class TExpr = TCallableNode or TLocalVariableRefNode or TImplicitArgumentNode; - private predicate isTopLevelCallable(BuilderNode callable) { callable.isCallable(any(TopLevel t)) } - class Expr extends TExpr { + class Expr extends Stage2Node { Expr() { not isTopLevelCallable(this) } - string toString() { result = this.(BuilderNode).toString() } - - Location getLocation() { result = this.(BuilderNode).getLocation() } - - predicate hasCfgNode(BasicBlock bb, int i) { - this.(LocalVariableRefNode).hasCfgNode(bb, i) - or - exists(DataFlowCallable callable | - this = TCallableNode(callable) and - bb.getNode(i).injects(callable.asSourceCallable()) - ) - or - // We need to associate a CFG node with the callee argument passed to a capturing getter/setter, - // so it can carry captured data in and out the getter/setter. - exists(DataFlowCall call | this = TImplicitArgumentNode(call, _, false) | - performsVariableAccess(call.asGetterCall(), _, TRead(), bb.getNode(i)) - or - performsVariableAccess(call.asSetterCall(), _, TWrite(), bb.getNode(i)) - ) - } + predicate hasCfgNode(BasicBlock bb, int i) { this.hasControlFlowPosition(bb, i) } } final private class FinalLocalVariableRefNode = LocalVariableRefNode; From d0d1363691e668ecaf117a4e62ce3633bd6679fd Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 26 Oct 2026 13:12:47 +0100 Subject: [PATCH 20/38] unified: Move ad-hoc aliasing logic into a step --- .../unified/internal/dataflow/CaptureSsa.qll | 15 +-------------- .../unified/internal/dataflow/DataFlowGraph.qll | 14 ++++++++++++++ .../lib/codeql/unified/internal/dataflow/Step.qll | 8 +++++++- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll index b0626e60f247..a615f3b857e9 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll @@ -71,7 +71,7 @@ module CaptureSsaInput implements InputSig { node.(BuilderNode).isCallable(callable) or exists(Stage2Node prev | callableHasLocalAlias(callable, prev) | - step(prev, any(Step s | s.value()), node) + step(prev, any(Step s | s.value() or s.callGraph()), node) or localSsaStep(prev, node, _) ) @@ -101,19 +101,6 @@ module CaptureSsaInput implements InputSig { predicate hasAliasedAccess(Expr f) { // TODO: Restrict to places where the function is called or escapes into a call where it might get called. callableHasLocalAlias(callable, f) and not f.(LocalVariableRefNode).getRefKind().isWrite() - or - // Record the callee argument passed into a local capturing getter/setter as an alias for that getter/setter - // so it can carry capture data in and out of the getter/setter call. - exists(ComputedLocalVariable local, DataFlowCall call | - callable = local.getAnAccessor() and - f = TImplicitArgumentNode(call, any(ArgumentPosition p | p.isCallee()), false) - | - callable instanceof GetterDeclaration and - call.asGetterCall() = local.getAnAccess() - or - callable instanceof SetterDeclaration and - call.asSetterCall() = local.getAnAccess() - ) } } } diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll index 098d444a99d8..d804e790b7a7 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll @@ -161,6 +161,20 @@ predicate step(BuilderNode node1, Step step, BuilderNode node2) { node2.isIncomingValue(arg.getValue()) ) or + // For local getters/setters, pass the callable into the callee argument. + // Capture SSA depends on this. + exists(ComputedLocalVariable local, AccessorDeclaration accessor, DataFlowCall call | + accessor = local.getAnAccessor() and + node1.isCallable(accessor) and + step.callGraph() and + ( + call.asGetterCall() = local.getAnAccess() and accessor instanceof GetterDeclaration + or + call.asSetterCall() = local.getAnAccess() and accessor instanceof SetterDeclaration + ) and + node2.isImplicitArgument(call, any(ArgumentPosition pos | pos.isCallee()), false) + ) + or exists(SwitchExpr expr | node1.isResultValue(expr.getValue()) and step.value() and diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/Step.qll b/unified/ql/lib/codeql/unified/internal/dataflow/Step.qll index 4f54fe36fd65..c4f3fc10e24d 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/Step.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/Step.qll @@ -7,7 +7,8 @@ private newtype TStep = TJumpStep() or TTaintStep() or TReadStep(ContentSet contents) or - TStoreStep(ContentSet contents) + TStoreStep(ContentSet contents) or + TCallGraphStep() /** * A type of data flow step, used during construction of the data flow graph. @@ -47,6 +48,9 @@ class Step extends TStep { pragma[nomagic] predicate storeArrayElement() { this.store(ContentSet::arrayElement()) } + /** Holds if this step can only be used for call graph construction. */ + predicate callGraph() { this = TCallGraphStep() } + string toString() { this.value() and result = "value" or @@ -61,5 +65,7 @@ class Step extends TStep { or this.store(contents) and result = "store[" + contents + "]" ) + or + this.callGraph() and result = "call-graph" } } From e3883da2e1d0806b13b6bfc94bd31ebdc437686f Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 26 Oct 2026 13:15:08 +0100 Subject: [PATCH 21/38] unified: Add basic escape Now that all data flow nodes are in Expr, we get more noise if we don't restrict this --- .../unified/internal/dataflow/CaptureSsa.qll | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll index a615f3b857e9..481d534d0fa4 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll @@ -90,6 +90,19 @@ module CaptureSsaInput implements InputSig { ) } + private predicate isEscape(BuilderNode node) { + node.isResultValue(any(Argument arg).getValue()) + or + node.isImplicitArgument(_, _, _) + or + exists(U::Expr e | + isReturnedExpr(_, e) and + node.isResultValue(e) + ) + or + step(node, any(Step s | s.store(_)), _) + } + class ClosureExpr extends Expr instanceof TCallableNode { // TODO: In order to support classes with captures, this needs to cover constructor calls to such classes as well. private Callable callable; @@ -99,8 +112,8 @@ module CaptureSsaInput implements InputSig { predicate hasBody(Callable body) { callable = body } predicate hasAliasedAccess(Expr f) { - // TODO: Restrict to places where the function is called or escapes into a call where it might get called. - callableHasLocalAlias(callable, f) and not f.(LocalVariableRefNode).getRefKind().isWrite() + callableHasLocalAlias(callable, f) and + isEscape(f) } } } From 09fb8f12ff37a91149f968975969de081f310237 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 26 Oct 2026 13:15:44 +0100 Subject: [PATCH 22/38] unified: Update nodes/edges in test output --- .../test/library-tests/dataflow/test.expected | 84 ++++++------------- 1 file changed, 26 insertions(+), 58 deletions(-) diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index c01df4925f6a..87168c53fde9 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -42,12 +42,10 @@ edges | accessors_local.swift:54:5:54:5 | y | accessors_local.swift:55:10:55:10 | x | provenance | | | accessors_local.swift:54:9:54:22 | source(...) | accessors_local.swift:54:5:54:5 | y | provenance | | | accessors_local.swift:59:21:59:34 | source(...) | accessors_local.swift:61:14:61:14 | x | provenance | | -| accessors_local.swift:72:5:72:5 | y | accessors_local.swift:73:5:73:11 | closure [y] | provenance | | +| accessors_local.swift:72:5:72:5 | y | accessors_local.swift:70:14:70:14 | x | provenance | | | accessors_local.swift:72:9:72:22 | source(...) | accessors_local.swift:72:5:72:5 | y | provenance | | -| accessors_local.swift:73:5:73:11 | closure [y] | accessors_local.swift:70:14:70:14 | x | provenance | | -| accessors_local.swift:84:5:84:5 | y | accessors_local.swift:85:5:85:11 | closure [y] | provenance | | +| accessors_local.swift:84:5:84:5 | y | accessors_local.swift:82:14:82:15 | x3 | provenance | | | accessors_local.swift:84:9:84:22 | source(...) | accessors_local.swift:84:5:84:5 | y | provenance | | -| accessors_local.swift:85:5:85:11 | closure [y] | accessors_local.swift:82:14:82:15 | x3 | provenance | | | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | provenance | | | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | provenance | | | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | provenance | | @@ -99,19 +97,17 @@ edges | calls.swift:99:18:99:21 | self [field] | calls.swift:99:18:99:27 | ... .field | provenance | | | calls.swift:111:18:111:21 | self [field] | calls.swift:111:18:111:27 | ... .field | provenance | | | calls.swift:123:18:123:21 | self [field] | calls.swift:123:18:123:27 | ... .field | provenance | | -| capture.swift:10:9:10:9 | x | capture.swift:14:5:14:11 | closure [x] | provenance | | +| capture.swift:10:9:10:9 | x | capture.swift:12:14:12:14 | x | provenance | | | capture.swift:10:13:10:26 | source(...) | capture.swift:10:9:10:9 | x | provenance | | -| capture.swift:14:5:14:11 | closure [x] | capture.swift:12:14:12:14 | x | provenance | | | capture.swift:20:9:20:9 | x | capture.swift:24:10:24:10 | x | provenance | | | capture.swift:20:13:20:26 | source(...) | capture.swift:20:9:20:9 | x | provenance | | -| capture.swift:28:9:28:9 | x | capture.swift:37:5:37:11 | closure [x] | provenance | | +| capture.swift:28:9:28:9 | x | capture.swift:32:14:32:14 | x | provenance | | +| capture.swift:28:9:28:9 | x | capture.swift:33:14:33:14 | x | provenance | | +| capture.swift:28:9:28:9 | x | capture.swift:38:10:38:11 | y1 | provenance | | +| capture.swift:28:9:28:9 | x | capture.swift:39:10:39:11 | y2 | provenance | | | capture.swift:28:13:28:26 | source(...) | capture.swift:28:9:28:9 | x | provenance | | | capture.swift:32:14:32:14 | x | capture.swift:32:9:32:10 | y1 | provenance | | | capture.swift:33:14:33:14 | x | capture.swift:33:9:33:10 | y2 | provenance | | -| capture.swift:37:5:37:11 | closure [x] | capture.swift:32:14:32:14 | x | provenance | | -| capture.swift:37:5:37:11 | closure [x] | capture.swift:33:14:33:14 | x | provenance | | -| capture.swift:37:5:37:11 | closure [x] | capture.swift:38:10:38:11 | y1 | provenance | | -| capture.swift:37:5:37:11 | closure [x] | capture.swift:39:10:39:11 | y2 | provenance | | | capture.swift:43:9:43:9 | x | capture.swift:49:13:49:13 | x | provenance | | | capture.swift:43:13:43:26 | source(...) | capture.swift:43:9:43:9 | x | provenance | | | capture.swift:45:22:45:24 | arg | capture.swift:46:13:46:15 | arg | provenance | | @@ -126,70 +122,58 @@ edges | capture.swift:60:13:60:13 | x | capture.swift:56:22:56:24 | arg | provenance | | | capture.swift:60:13:60:13 | x | capture.swift:61:10:61:10 | y [0] | provenance | | | capture.swift:61:10:61:10 | y [0] | capture.swift:61:10:61:12 | ... .0 | provenance | | -| capture.swift:69:5:69:5 | x | capture.swift:70:5:70:11 | closure [x] | provenance | | +| capture.swift:69:5:69:5 | x | capture.swift:67:14:67:14 | x | provenance | | | capture.swift:69:9:69:22 | source(...) | capture.swift:69:5:69:5 | x | provenance | | -| capture.swift:70:5:70:11 | closure [x] | capture.swift:67:14:67:14 | x | provenance | | -| capture.swift:74:9:74:9 | x | capture.swift:78:5:78:9 | local [x] | provenance | | +| capture.swift:74:9:74:9 | x | capture.swift:76:14:76:14 | x | provenance | | | capture.swift:74:13:74:26 | source(...) | capture.swift:74:9:74:9 | x | provenance | | -| capture.swift:78:5:78:9 | local [x] | capture.swift:76:14:76:14 | x | provenance | | | capture.swift:84:9:84:9 | x | capture.swift:88:10:88:10 | x | provenance | | | capture.swift:84:13:84:26 | source(...) | capture.swift:84:9:84:9 | x | provenance | | -| capture.swift:93:9:93:9 | x | capture.swift:94:13:94:17 | local [x] | provenance | | +| capture.swift:93:9:93:9 | x | capture.swift:94:13:94:19 | local(...) | provenance | | +| capture.swift:93:9:93:9 | x | capture.swift:98:14:98:14 | x | provenance | | +| capture.swift:93:9:93:9 | x | capture.swift:99:16:99:16 | x | provenance | | | capture.swift:93:13:93:26 | source(...) | capture.swift:93:9:93:9 | x | provenance | | | capture.swift:94:9:94:9 | y | capture.swift:95:10:95:10 | y | provenance | | -| capture.swift:94:13:94:17 | local [x] | capture.swift:94:13:94:19 | local(...) | provenance | | -| capture.swift:94:13:94:17 | local [x] | capture.swift:98:14:98:14 | x | provenance | | -| capture.swift:94:13:94:17 | local [x] | capture.swift:99:16:99:16 | x | provenance | | | capture.swift:94:13:94:19 | local(...) | capture.swift:94:9:94:9 | y | provenance | | | capture.swift:104:9:104:9 | x | capture.swift:105:28:105:28 | x | provenance | | | capture.swift:104:9:104:9 | x | capture.swift:108:23:108:23 | x | provenance | | | capture.swift:104:13:104:27 | source(...) | capture.swift:104:9:104:9 | x | provenance | | -| capture.swift:105:23:105:24 | x1 | capture.swift:111:5:111:12 | closure1 [x1] | provenance | | +| capture.swift:105:23:105:24 | x1 | capture.swift:106:14:106:15 | x1 | provenance | | | capture.swift:105:28:105:28 | x | capture.swift:105:23:105:24 | x1 | provenance | | | capture.swift:108:23:108:23 | x | capture.swift:108:23:108:23 | x | provenance | | -| capture.swift:108:23:108:23 | x | capture.swift:112:5:112:12 | closure2 [x] | provenance | | -| capture.swift:111:5:111:12 | closure1 [x1] | capture.swift:106:14:106:15 | x1 | provenance | | -| capture.swift:112:5:112:12 | closure2 [x] | capture.swift:109:14:109:14 | x | provenance | | +| capture.swift:108:23:108:23 | x | capture.swift:109:14:109:14 | x | provenance | | | capture.swift:116:9:116:9 | x | capture.swift:117:26:117:26 | x | provenance | | | capture.swift:116:13:116:27 | source(...) | capture.swift:116:9:116:9 | x | provenance | | | capture.swift:117:22:117:22 | x | capture.swift:117:36:117:36 | x | provenance | | -| capture.swift:117:22:117:22 | x | capture.swift:121:5:121:11 | closure [x] | provenance | | +| capture.swift:117:22:117:22 | x | capture.swift:118:14:118:14 | x | provenance | | | capture.swift:117:26:117:26 | x | capture.swift:117:22:117:22 | x | provenance | | -| capture.swift:117:29:117:32 | blah | capture.swift:121:5:121:11 | closure [blah] | provenance | | +| capture.swift:117:29:117:32 | blah | capture.swift:119:14:119:17 | blah | provenance | | | capture.swift:117:36:117:36 | x | capture.swift:117:29:117:32 | blah | provenance | | -| capture.swift:121:5:121:11 | closure [blah] | capture.swift:119:14:119:17 | blah | provenance | | -| capture.swift:121:5:121:11 | closure [x] | capture.swift:118:14:118:14 | x | provenance | | -| capture.swift:147:5:147:5 | x | capture.swift:149:5:149:12 | closure2 [x] | provenance | | +| capture.swift:147:5:147:5 | x | capture.swift:143:14:143:14 | x | provenance | | | capture.swift:147:9:147:23 | source(...) | capture.swift:147:5:147:5 | x | provenance | | -| capture.swift:149:5:149:12 | closure2 [x] | capture.swift:143:14:143:14 | x | provenance | | -| capture.swift:156:9:156:9 | x | capture.swift:160:9:160:15 | closure [self, x] | provenance | | +| capture.swift:156:9:156:9 | x | capture.swift:158:18:158:21 | self [x] | provenance | | | capture.swift:156:13:156:25 | source(...) | capture.swift:156:9:156:9 | x | provenance | | | capture.swift:158:18:158:21 | self [x] | capture.swift:158:18:158:23 | ... .x | provenance | | -| capture.swift:160:9:160:15 | closure [self, x] | capture.swift:158:18:158:21 | self [x] | provenance | | | capture.swift:164:9:164:9 | x | capture.swift:165:26:165:29 | self [x] | provenance | | | capture.swift:164:13:164:25 | source(...) | capture.swift:164:9:164:9 | x | provenance | | | capture.swift:165:26:165:29 | self [x] | capture.swift:165:26:165:29 | self [x] | provenance | | -| capture.swift:165:26:165:29 | self [x] | capture.swift:169:9:169:15 | closure [self, x] | provenance | | +| capture.swift:165:26:165:29 | self [x] | capture.swift:166:18:166:21 | self [x] | provenance | | +| capture.swift:165:26:165:29 | self [x] | capture.swift:167:18:167:18 | x | provenance | | | capture.swift:166:18:166:21 | self [x] | capture.swift:166:18:166:23 | ... .x | provenance | | -| capture.swift:169:9:169:15 | closure [self, x] | capture.swift:166:18:166:21 | self [x] | provenance | | -| capture.swift:169:9:169:15 | closure [self, x] | capture.swift:167:18:167:18 | x | provenance | | | capture.swift:173:9:173:9 | x | capture.swift:174:31:174:34 | self [x] | provenance | | | capture.swift:173:13:173:25 | source(...) | capture.swift:173:9:173:9 | x | provenance | | | capture.swift:174:31:174:34 | self [x] | capture.swift:174:31:174:34 | self [x] | provenance | | -| capture.swift:174:31:174:34 | self [x] | capture.swift:179:9:179:15 | closure [self, x] | provenance | | +| capture.swift:174:31:174:34 | self [x] | capture.swift:175:23:175:26 | self [x] | provenance | | | capture.swift:175:19:175:26 | ... .some(...) [x] | capture.swift:175:23:175:26 | self [x] | provenance | | | capture.swift:175:23:175:26 | self [x] | capture.swift:175:19:175:26 | ... .some(...) [x] | provenance | | | capture.swift:175:23:175:26 | self [x] | capture.swift:176:18:176:21 | self [x] | provenance | | | capture.swift:175:23:175:26 | self [x] | capture.swift:177:18:177:18 | x | provenance | | | capture.swift:176:18:176:21 | self [x] | capture.swift:176:18:176:23 | ... .x | provenance | | -| capture.swift:179:9:179:15 | closure [self, x] | capture.swift:175:23:175:26 | self [x] | provenance | | | capture.swift:183:9:183:9 | x | capture.swift:184:34:184:37 | self [x] | provenance | | | capture.swift:183:13:183:25 | source(...) | capture.swift:183:9:183:9 | x | provenance | | | capture.swift:184:34:184:37 | self [x] | capture.swift:184:34:184:37 | self [x] | provenance | | -| capture.swift:184:34:184:37 | self [x] | capture.swift:188:9:188:15 | closure [self, x] | provenance | | +| capture.swift:184:34:184:37 | self [x] | capture.swift:185:18:185:21 | self [x] | provenance | | +| capture.swift:184:34:184:37 | self [x] | capture.swift:186:18:186:18 | x | provenance | | | capture.swift:185:18:185:21 | self [x] | capture.swift:185:18:185:23 | ... .x | provenance | | -| capture.swift:188:9:188:15 | closure [self, x] | capture.swift:185:18:185:21 | self [x] | provenance | | -| capture.swift:188:9:188:15 | closure [self, x] | capture.swift:186:18:186:18 | x | provenance | | | enums.swift:12:9:12:9 | e [case1.0] | enums.swift:14:12:14:12 | e [case1.0] | provenance | | | enums.swift:12:13:12:35 | ... .case1(...) [case1.0] | enums.swift:12:9:12:9 | e [case1.0] | provenance | | | enums.swift:12:21:12:34 | source(...) | enums.swift:12:13:12:35 | ... .case1(...) [case1.0] | provenance | | @@ -581,11 +565,9 @@ nodes | accessors_local.swift:70:14:70:14 | x | semmle.label | x | | accessors_local.swift:72:5:72:5 | y | semmle.label | y | | accessors_local.swift:72:9:72:22 | source(...) | semmle.label | source(...) | -| accessors_local.swift:73:5:73:11 | closure [y] | semmle.label | closure [y] | | accessors_local.swift:82:14:82:15 | x3 | semmle.label | x3 | | accessors_local.swift:84:5:84:5 | y | semmle.label | y | | accessors_local.swift:84:9:84:22 | source(...) | semmle.label | source(...) | -| accessors_local.swift:85:5:85:11 | closure [y] | semmle.label | closure [y] | | accessors_static.swift:8:32:8:45 | source(...) | semmle.label | source(...) | | accessors_static.swift:10:10:10:12 | ... .x | semmle.label | ... .x | | accessors_static.swift:18:19:18:32 | source(...) | semmle.label | source(...) | @@ -656,7 +638,6 @@ nodes | capture.swift:10:9:10:9 | x | semmle.label | x | | capture.swift:10:13:10:26 | source(...) | semmle.label | source(...) | | capture.swift:12:14:12:14 | x | semmle.label | x | -| capture.swift:14:5:14:11 | closure [x] | semmle.label | closure [x] | | capture.swift:20:9:20:9 | x | semmle.label | x | | capture.swift:20:13:20:26 | source(...) | semmle.label | source(...) | | capture.swift:24:10:24:10 | x | semmle.label | x | @@ -666,7 +647,6 @@ nodes | capture.swift:32:14:32:14 | x | semmle.label | x | | capture.swift:33:9:33:10 | y2 | semmle.label | y2 | | capture.swift:33:14:33:14 | x | semmle.label | x | -| capture.swift:37:5:37:11 | closure [x] | semmle.label | closure [x] | | capture.swift:38:10:38:11 | y1 | semmle.label | y1 | | capture.swift:39:10:39:11 | y2 | semmle.label | y2 | | capture.swift:43:9:43:9 | x | semmle.label | x | @@ -688,18 +668,15 @@ nodes | capture.swift:67:14:67:14 | x | semmle.label | x | | capture.swift:69:5:69:5 | x | semmle.label | x | | capture.swift:69:9:69:22 | source(...) | semmle.label | source(...) | -| capture.swift:70:5:70:11 | closure [x] | semmle.label | closure [x] | | capture.swift:74:9:74:9 | x | semmle.label | x | | capture.swift:74:13:74:26 | source(...) | semmle.label | source(...) | | capture.swift:76:14:76:14 | x | semmle.label | x | -| capture.swift:78:5:78:9 | local [x] | semmle.label | local [x] | | capture.swift:84:9:84:9 | x | semmle.label | x | | capture.swift:84:13:84:26 | source(...) | semmle.label | source(...) | | capture.swift:88:10:88:10 | x | semmle.label | x | | capture.swift:93:9:93:9 | x | semmle.label | x | | capture.swift:93:13:93:26 | source(...) | semmle.label | source(...) | | capture.swift:94:9:94:9 | y | semmle.label | y | -| capture.swift:94:13:94:17 | local [x] | semmle.label | local [x] | | capture.swift:94:13:94:19 | local(...) | semmle.label | local(...) | | capture.swift:95:10:95:10 | y | semmle.label | y | | capture.swift:98:14:98:14 | x | semmle.label | x | @@ -712,8 +689,6 @@ nodes | capture.swift:108:23:108:23 | x | semmle.label | x | | capture.swift:108:23:108:23 | x | semmle.label | x | | capture.swift:109:14:109:14 | x | semmle.label | x | -| capture.swift:111:5:111:12 | closure1 [x1] | semmle.label | closure1 [x1] | -| capture.swift:112:5:112:12 | closure2 [x] | semmle.label | closure2 [x] | | capture.swift:116:9:116:9 | x | semmle.label | x | | capture.swift:116:13:116:27 | source(...) | semmle.label | source(...) | | capture.swift:117:22:117:22 | x | semmle.label | x | @@ -722,17 +697,13 @@ nodes | capture.swift:117:36:117:36 | x | semmle.label | x | | capture.swift:118:14:118:14 | x | semmle.label | x | | capture.swift:119:14:119:17 | blah | semmle.label | blah | -| capture.swift:121:5:121:11 | closure [blah] | semmle.label | closure [blah] | -| capture.swift:121:5:121:11 | closure [x] | semmle.label | closure [x] | | capture.swift:143:14:143:14 | x | semmle.label | x | | capture.swift:147:5:147:5 | x | semmle.label | x | | capture.swift:147:9:147:23 | source(...) | semmle.label | source(...) | -| capture.swift:149:5:149:12 | closure2 [x] | semmle.label | closure2 [x] | | capture.swift:156:9:156:9 | x | semmle.label | x | | capture.swift:156:13:156:25 | source(...) | semmle.label | source(...) | | capture.swift:158:18:158:21 | self [x] | semmle.label | self [x] | | capture.swift:158:18:158:23 | ... .x | semmle.label | ... .x | -| capture.swift:160:9:160:15 | closure [self, x] | semmle.label | closure [self, x] | | capture.swift:164:9:164:9 | x | semmle.label | x | | capture.swift:164:13:164:25 | source(...) | semmle.label | source(...) | | capture.swift:165:26:165:29 | self [x] | semmle.label | self [x] | @@ -740,7 +711,6 @@ nodes | capture.swift:166:18:166:21 | self [x] | semmle.label | self [x] | | capture.swift:166:18:166:23 | ... .x | semmle.label | ... .x | | capture.swift:167:18:167:18 | x | semmle.label | x | -| capture.swift:169:9:169:15 | closure [self, x] | semmle.label | closure [self, x] | | capture.swift:173:9:173:9 | x | semmle.label | x | | capture.swift:173:13:173:25 | source(...) | semmle.label | source(...) | | capture.swift:174:31:174:34 | self [x] | semmle.label | self [x] | @@ -751,7 +721,6 @@ nodes | capture.swift:176:18:176:21 | self [x] | semmle.label | self [x] | | capture.swift:176:18:176:23 | ... .x | semmle.label | ... .x | | capture.swift:177:18:177:18 | x | semmle.label | x | -| capture.swift:179:9:179:15 | closure [self, x] | semmle.label | closure [self, x] | | capture.swift:183:9:183:9 | x | semmle.label | x | | capture.swift:183:13:183:25 | source(...) | semmle.label | source(...) | | capture.swift:184:34:184:37 | self [x] | semmle.label | self [x] | @@ -759,7 +728,6 @@ nodes | capture.swift:185:18:185:21 | self [x] | semmle.label | self [x] | | capture.swift:185:18:185:23 | ... .x | semmle.label | ... .x | | capture.swift:186:18:186:18 | x | semmle.label | x | -| capture.swift:188:9:188:15 | closure [self, x] | semmle.label | closure [self, x] | | enums.swift:12:9:12:9 | e [case1.0] | semmle.label | e [case1.0] | | enums.swift:12:13:12:35 | ... .case1(...) [case1.0] | semmle.label | ... .case1(...) [case1.0] | | enums.swift:12:21:12:34 | source(...) | semmle.label | source(...) | @@ -1170,11 +1138,11 @@ subpaths | accessors_local.swift:54:5:54:5 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:55:10:55:10 | x | | calls.swift:31:17:31:30 | source(...) | calls.swift:28:19:28:19 | x | calls.swift:29:16:29:24 | ... + ... | calls.swift:31:10:31:31 | target(...) | | calls.swift:32:17:32:30 | source(...) | calls.swift:28:19:28:19 | x | calls.swift:29:16:29:24 | ... + ... | calls.swift:32:10:32:31 | target(...) | -| capture.swift:37:5:37:11 | closure [x] | capture.swift:32:14:32:14 | x | capture.swift:32:9:32:10 | y1 | capture.swift:38:10:38:11 | y1 | -| capture.swift:37:5:37:11 | closure [x] | capture.swift:33:14:33:14 | x | capture.swift:33:9:33:10 | y2 | capture.swift:39:10:39:11 | y2 | +| capture.swift:28:9:28:9 | x | capture.swift:32:14:32:14 | x | capture.swift:32:9:32:10 | y1 | capture.swift:38:10:38:11 | y1 | +| capture.swift:28:9:28:9 | x | capture.swift:33:14:33:14 | x | capture.swift:33:9:33:10 | y2 | capture.swift:39:10:39:11 | y2 | | capture.swift:49:13:49:13 | x | capture.swift:45:22:45:24 | arg | capture.swift:46:9:46:9 | y | capture.swift:50:10:50:10 | y | | capture.swift:60:13:60:13 | x | capture.swift:56:22:56:24 | arg | capture.swift:57:9:57:9 | [post] y [0] | capture.swift:61:10:61:10 | y [0] | -| capture.swift:94:13:94:17 | local [x] | capture.swift:99:16:99:16 | x | capture.swift:99:16:99:16 | x | capture.swift:94:13:94:19 | local(...) | +| capture.swift:93:9:93:9 | x | capture.swift:99:16:99:16 | x | capture.swift:99:16:99:16 | x | capture.swift:94:13:94:19 | local(...) | | flow-summaries.swift:54:35:54:48 | source(...) | flow-summaries.swift:54:53:54:53 | y | flow-summaries.swift:54:65:54:65 | y | flow-summaries.swift:54:10:54:67 | ... .throughCallback(...) | | flow-summaries.swift:55:35:55:48 | source(...) | flow-summaries.swift:55:53:55:53 | y | flow-summaries.swift:55:65:55:71 | ... + ... | flow-summaries.swift:55:10:55:73 | ... .throughCallback(...) | | test.swift:196:30:196:44 | source(...) | test.swift:189:22:189:22 | s | test.swift:189:58:189:58 | s | test.swift:196:16:196:45 | asyncIdentity(...) | From b084467976cac5d898151a21eaa254db7531c4b0 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 26 Oct 2026 13:17:16 +0100 Subject: [PATCH 23/38] unified: Tolerate missing post-update for RHS --- unified/ql/consistency-queries/DataFlowConsistency.ql | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/unified/ql/consistency-queries/DataFlowConsistency.ql b/unified/ql/consistency-queries/DataFlowConsistency.ql index 8cbec3a23f8e..eaaeec834ef7 100644 --- a/unified/ql/consistency-queries/DataFlowConsistency.ql +++ b/unified/ql/consistency-queries/DataFlowConsistency.ql @@ -5,6 +5,10 @@ private import codeql.dataflow.internal.DataFlowImplConsistency module ConsistencyInput implements InputSig { predicate argHasPostUpdateExclude(DataFlowInput::ArgumentNode n) { not exists(n.getBasicBlock()) // ignore unreachable data flow nodes + or + // The RHS value passed into a setter call technically needs a post-update node + // because it's an argument to a call, but this rarely matters in practice. + n.isIncomingValue(any(DataFlowCall c).asSetterCall()) } predicate reverseReadExclude(DataFlow::Node n) { From 1cc99d502d35195c158e04b4591798c9eeab061c Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 26 Oct 2026 13:35:30 +0100 Subject: [PATCH 24/38] unified: Add test for explicitly-named setter parameter This was already handled in the AST mapping, but no QL test was added initially --- .../dataflow/accessors_instance.swift | 10 ++ .../test/library-tests/dataflow/test.expected | 106 ++++++++++-------- 2 files changed, 67 insertions(+), 49 deletions(-) diff --git a/unified/ql/test/library-tests/dataflow/accessors_instance.swift b/unified/ql/test/library-tests/dataflow/accessors_instance.swift index 9a3a2bfb9e5e..a03f324eee0e 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_instance.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_instance.swift @@ -41,6 +41,16 @@ func t4() { C().x = source("t4.1") } +func t4a() { + class C { + var x: String { + get {} + set(v) { sink(v) } // $ hasValueFlow=t4a.1 + } + } + C().x = source("t4a.1") +} + func t5() { class C { var x: String { diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index 87168c53fde9..37b29ef36f0d 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -6,25 +6,28 @@ edges | accessors_instance.swift:38:13:38:15 | newValue | accessors_instance.swift:38:24:38:31 | newValue | provenance | | | accessors_instance.swift:41:5:41:9 | ... .x | accessors_instance.swift:38:13:38:15 | newValue | provenance | | | accessors_instance.swift:41:13:41:26 | source(...) | accessors_instance.swift:41:5:41:9 | ... .x | provenance | | -| accessors_instance.swift:47:19:47:32 | source(...) | accessors_instance.swift:52:11:52:13 | ... .x | provenance | | -| accessors_instance.swift:48:13:48:15 | newValue | accessors_instance.swift:48:24:48:31 | newValue | provenance | | -| accessors_instance.swift:52:5:52:7 | ... .x | accessors_instance.swift:48:13:48:15 | newValue | provenance | | -| accessors_instance.swift:52:11:52:13 | ... .x | accessors_instance.swift:52:5:52:7 | ... .x | provenance | | -| accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:60:18:60:23 | ... .x | provenance | | -| accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | provenance | | -| accessors_instance.swift:69:25:69:28 | self [field] | accessors_instance.swift:69:25:69:34 | ... .field | provenance | | -| accessors_instance.swift:73:5:73:5 | [post] c [field] | accessors_instance.swift:74:10:74:10 | c [field] | provenance | | -| accessors_instance.swift:73:5:73:11 | ... .field | accessors_instance.swift:73:5:73:5 | [post] c [field] | provenance | | -| accessors_instance.swift:73:15:73:28 | source(...) | accessors_instance.swift:73:5:73:11 | ... .field | provenance | | -| accessors_instance.swift:74:10:74:10 | c [field] | accessors_instance.swift:69:25:69:28 | self [field] | provenance | | -| accessors_instance.swift:74:10:74:10 | c [field] | accessors_instance.swift:74:10:74:12 | ... .x | provenance | | -| accessors_instance.swift:83:32:83:36 | field | accessors_instance.swift:83:19:83:28 | otherField | provenance | | -| accessors_instance.swift:87:5:87:5 | [post] c [field] | accessors_instance.swift:88:5:88:5 | c [field] | provenance | | -| accessors_instance.swift:87:5:87:11 | ... .field | accessors_instance.swift:87:5:87:5 | [post] c [field] | provenance | | -| accessors_instance.swift:87:15:87:28 | source(...) | accessors_instance.swift:87:5:87:11 | ... .field | provenance | | -| accessors_instance.swift:88:5:88:5 | c [field] | accessors_instance.swift:83:32:83:36 | field | provenance | | -| accessors_instance.swift:88:5:88:5 | c [field] | accessors_instance.swift:89:10:89:10 | c [otherField] | provenance | | -| accessors_instance.swift:89:10:89:10 | c [otherField] | accessors_instance.swift:89:10:89:21 | ... .otherField | provenance | | +| accessors_instance.swift:48:17:48:17 | v | accessors_instance.swift:48:27:48:27 | v | provenance | | +| accessors_instance.swift:51:5:51:9 | ... .x | accessors_instance.swift:48:17:48:17 | v | provenance | | +| accessors_instance.swift:51:13:51:27 | source(...) | accessors_instance.swift:51:5:51:9 | ... .x | provenance | | +| accessors_instance.swift:57:19:57:32 | source(...) | accessors_instance.swift:62:11:62:13 | ... .x | provenance | | +| accessors_instance.swift:58:13:58:15 | newValue | accessors_instance.swift:58:24:58:31 | newValue | provenance | | +| accessors_instance.swift:62:5:62:7 | ... .x | accessors_instance.swift:58:13:58:15 | newValue | provenance | | +| accessors_instance.swift:62:11:62:13 | ... .x | accessors_instance.swift:62:5:62:7 | ... .x | provenance | | +| accessors_instance.swift:67:25:67:38 | source(...) | accessors_instance.swift:70:18:70:23 | ... .x | provenance | | +| accessors_instance.swift:67:25:67:38 | source(...) | accessors_instance.swift:71:18:71:18 | x | provenance | | +| accessors_instance.swift:79:25:79:28 | self [field] | accessors_instance.swift:79:25:79:34 | ... .field | provenance | | +| accessors_instance.swift:83:5:83:5 | [post] c [field] | accessors_instance.swift:84:10:84:10 | c [field] | provenance | | +| accessors_instance.swift:83:5:83:11 | ... .field | accessors_instance.swift:83:5:83:5 | [post] c [field] | provenance | | +| accessors_instance.swift:83:15:83:28 | source(...) | accessors_instance.swift:83:5:83:11 | ... .field | provenance | | +| accessors_instance.swift:84:10:84:10 | c [field] | accessors_instance.swift:79:25:79:28 | self [field] | provenance | | +| accessors_instance.swift:84:10:84:10 | c [field] | accessors_instance.swift:84:10:84:12 | ... .x | provenance | | +| accessors_instance.swift:93:32:93:36 | field | accessors_instance.swift:93:19:93:28 | otherField | provenance | | +| accessors_instance.swift:97:5:97:5 | [post] c [field] | accessors_instance.swift:98:5:98:5 | c [field] | provenance | | +| accessors_instance.swift:97:5:97:11 | ... .field | accessors_instance.swift:97:5:97:5 | [post] c [field] | provenance | | +| accessors_instance.swift:97:15:97:28 | source(...) | accessors_instance.swift:97:5:97:11 | ... .field | provenance | | +| accessors_instance.swift:98:5:98:5 | c [field] | accessors_instance.swift:93:32:93:36 | field | provenance | | +| accessors_instance.swift:98:5:98:5 | c [field] | accessors_instance.swift:99:10:99:10 | c [otherField] | provenance | | +| accessors_instance.swift:99:10:99:10 | c [otherField] | accessors_instance.swift:99:10:99:21 | ... .otherField | provenance | | | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | | | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | | | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | provenance | | @@ -514,29 +517,33 @@ nodes | accessors_instance.swift:38:24:38:31 | newValue | semmle.label | newValue | | accessors_instance.swift:41:5:41:9 | ... .x | semmle.label | ... .x | | accessors_instance.swift:41:13:41:26 | source(...) | semmle.label | source(...) | -| accessors_instance.swift:47:19:47:32 | source(...) | semmle.label | source(...) | -| accessors_instance.swift:48:13:48:15 | newValue | semmle.label | newValue | -| accessors_instance.swift:48:24:48:31 | newValue | semmle.label | newValue | -| accessors_instance.swift:52:5:52:7 | ... .x | semmle.label | ... .x | -| accessors_instance.swift:52:11:52:13 | ... .x | semmle.label | ... .x | -| accessors_instance.swift:57:25:57:38 | source(...) | semmle.label | source(...) | -| accessors_instance.swift:60:18:60:23 | ... .x | semmle.label | ... .x | -| accessors_instance.swift:61:18:61:18 | x | semmle.label | x | -| accessors_instance.swift:69:25:69:28 | self [field] | semmle.label | self [field] | -| accessors_instance.swift:69:25:69:34 | ... .field | semmle.label | ... .field | -| accessors_instance.swift:73:5:73:5 | [post] c [field] | semmle.label | [post] c [field] | -| accessors_instance.swift:73:5:73:11 | ... .field | semmle.label | ... .field | -| accessors_instance.swift:73:15:73:28 | source(...) | semmle.label | source(...) | -| accessors_instance.swift:74:10:74:10 | c [field] | semmle.label | c [field] | -| accessors_instance.swift:74:10:74:12 | ... .x | semmle.label | ... .x | -| accessors_instance.swift:83:19:83:28 | otherField | semmle.label | otherField | -| accessors_instance.swift:83:32:83:36 | field | semmle.label | field | -| accessors_instance.swift:87:5:87:5 | [post] c [field] | semmle.label | [post] c [field] | -| accessors_instance.swift:87:5:87:11 | ... .field | semmle.label | ... .field | -| accessors_instance.swift:87:15:87:28 | source(...) | semmle.label | source(...) | -| accessors_instance.swift:88:5:88:5 | c [field] | semmle.label | c [field] | -| accessors_instance.swift:89:10:89:10 | c [otherField] | semmle.label | c [otherField] | -| accessors_instance.swift:89:10:89:21 | ... .otherField | semmle.label | ... .otherField | +| accessors_instance.swift:48:17:48:17 | v | semmle.label | v | +| accessors_instance.swift:48:27:48:27 | v | semmle.label | v | +| accessors_instance.swift:51:5:51:9 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:51:13:51:27 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:57:19:57:32 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:58:13:58:15 | newValue | semmle.label | newValue | +| accessors_instance.swift:58:24:58:31 | newValue | semmle.label | newValue | +| accessors_instance.swift:62:5:62:7 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:62:11:62:13 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:67:25:67:38 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:70:18:70:23 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:71:18:71:18 | x | semmle.label | x | +| accessors_instance.swift:79:25:79:28 | self [field] | semmle.label | self [field] | +| accessors_instance.swift:79:25:79:34 | ... .field | semmle.label | ... .field | +| accessors_instance.swift:83:5:83:5 | [post] c [field] | semmle.label | [post] c [field] | +| accessors_instance.swift:83:5:83:11 | ... .field | semmle.label | ... .field | +| accessors_instance.swift:83:15:83:28 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:84:10:84:10 | c [field] | semmle.label | c [field] | +| accessors_instance.swift:84:10:84:12 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:93:19:93:28 | otherField | semmle.label | otherField | +| accessors_instance.swift:93:32:93:36 | field | semmle.label | field | +| accessors_instance.swift:97:5:97:5 | [post] c [field] | semmle.label | [post] c [field] | +| accessors_instance.swift:97:5:97:11 | ... .field | semmle.label | ... .field | +| accessors_instance.swift:97:15:97:28 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:98:5:98:5 | c [field] | semmle.label | c [field] | +| accessors_instance.swift:99:10:99:10 | c [otherField] | semmle.label | c [otherField] | +| accessors_instance.swift:99:10:99:21 | ... .otherField | semmle.label | ... .otherField | | accessors_local.swift:7:21:7:34 | source(...) | semmle.label | source(...) | | accessors_local.swift:8:10:8:10 | x | semmle.label | x | | accessors_local.swift:15:15:15:28 | source(...) | semmle.label | source(...) | @@ -1132,8 +1139,8 @@ nodes | test.swift:273:30:273:45 | source(...) | semmle.label | source(...) | | test.swift:274:10:274:12 | f(...) | semmle.label | f(...) | subpaths -| accessors_instance.swift:74:10:74:10 | c [field] | accessors_instance.swift:69:25:69:28 | self [field] | accessors_instance.swift:69:25:69:34 | ... .field | accessors_instance.swift:74:10:74:12 | ... .x | -| accessors_instance.swift:88:5:88:5 | c [field] | accessors_instance.swift:83:32:83:36 | field | accessors_instance.swift:83:19:83:28 | otherField | accessors_instance.swift:89:10:89:10 | c [otherField] | +| accessors_instance.swift:84:10:84:10 | c [field] | accessors_instance.swift:79:25:79:28 | self [field] | accessors_instance.swift:79:25:79:34 | ... .field | accessors_instance.swift:84:10:84:12 | ... .x | +| accessors_instance.swift:98:5:98:5 | c [field] | accessors_instance.swift:93:32:93:36 | field | accessors_instance.swift:93:19:93:28 | otherField | accessors_instance.swift:99:10:99:10 | c [otherField] | | accessors_local.swift:45:9:45:9 | y | accessors_local.swift:46:21:46:21 | y | accessors_local.swift:46:21:46:21 | y | accessors_local.swift:47:10:47:10 | x | | accessors_local.swift:54:5:54:5 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:55:10:55:10 | x | | calls.swift:31:17:31:30 | source(...) | calls.swift:28:19:28:19 | x | calls.swift:29:16:29:24 | ... + ... | calls.swift:31:10:31:31 | target(...) | @@ -1158,11 +1165,12 @@ testFailures | accessors_instance.swift:21:10:21:14 | ... .x | accessors_instance.swift:18:19:18:32 | source(...) | accessors_instance.swift:21:10:21:14 | ... .x | $@ | accessors_instance.swift:18:19:18:32 | source(...) | source(...) | | accessors_instance.swift:31:10:31:14 | ... .x | accessors_instance.swift:27:19:27:32 | source(...) | accessors_instance.swift:31:10:31:14 | ... .x | $@ | accessors_instance.swift:27:19:27:32 | source(...) | source(...) | | accessors_instance.swift:38:24:38:31 | newValue | accessors_instance.swift:41:13:41:26 | source(...) | accessors_instance.swift:38:24:38:31 | newValue | $@ | accessors_instance.swift:41:13:41:26 | source(...) | source(...) | -| accessors_instance.swift:48:24:48:31 | newValue | accessors_instance.swift:47:19:47:32 | source(...) | accessors_instance.swift:48:24:48:31 | newValue | $@ | accessors_instance.swift:47:19:47:32 | source(...) | source(...) | -| accessors_instance.swift:60:18:60:23 | ... .x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:60:18:60:23 | ... .x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) | -| accessors_instance.swift:61:18:61:18 | x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) | -| accessors_instance.swift:74:10:74:12 | ... .x | accessors_instance.swift:73:15:73:28 | source(...) | accessors_instance.swift:74:10:74:12 | ... .x | $@ | accessors_instance.swift:73:15:73:28 | source(...) | source(...) | -| accessors_instance.swift:89:10:89:21 | ... .otherField | accessors_instance.swift:87:15:87:28 | source(...) | accessors_instance.swift:89:10:89:21 | ... .otherField | $@ | accessors_instance.swift:87:15:87:28 | source(...) | source(...) | +| accessors_instance.swift:48:27:48:27 | v | accessors_instance.swift:51:13:51:27 | source(...) | accessors_instance.swift:48:27:48:27 | v | $@ | accessors_instance.swift:51:13:51:27 | source(...) | source(...) | +| accessors_instance.swift:58:24:58:31 | newValue | accessors_instance.swift:57:19:57:32 | source(...) | accessors_instance.swift:58:24:58:31 | newValue | $@ | accessors_instance.swift:57:19:57:32 | source(...) | source(...) | +| accessors_instance.swift:70:18:70:23 | ... .x | accessors_instance.swift:67:25:67:38 | source(...) | accessors_instance.swift:70:18:70:23 | ... .x | $@ | accessors_instance.swift:67:25:67:38 | source(...) | source(...) | +| accessors_instance.swift:71:18:71:18 | x | accessors_instance.swift:67:25:67:38 | source(...) | accessors_instance.swift:71:18:71:18 | x | $@ | accessors_instance.swift:67:25:67:38 | source(...) | source(...) | +| accessors_instance.swift:84:10:84:12 | ... .x | accessors_instance.swift:83:15:83:28 | source(...) | accessors_instance.swift:84:10:84:12 | ... .x | $@ | accessors_instance.swift:83:15:83:28 | source(...) | source(...) | +| accessors_instance.swift:99:10:99:21 | ... .otherField | accessors_instance.swift:97:15:97:28 | source(...) | accessors_instance.swift:99:10:99:21 | ... .otherField | $@ | accessors_instance.swift:97:15:97:28 | source(...) | source(...) | | accessors_local.swift:8:10:8:10 | x | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | $@ | accessors_local.swift:7:21:7:34 | source(...) | source(...) | | accessors_local.swift:17:10:17:10 | x | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | $@ | accessors_local.swift:15:15:15:28 | source(...) | source(...) | | accessors_local.swift:25:10:25:10 | x | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | $@ | accessors_local.swift:22:15:22:28 | source(...) | source(...) | From 612c241238093200f74991f5797b2b65e018a50e Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 27 Oct 2026 09:35:43 +0100 Subject: [PATCH 25/38] Update isEscape predicate to include incoming values Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll index 481d534d0fa4..08902d55da69 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll @@ -93,6 +93,8 @@ module CaptureSsaInput implements InputSig { private predicate isEscape(BuilderNode node) { node.isResultValue(any(Argument arg).getValue()) or + node.isIncomingValue(any(DataFlowCall call).asSetterCall()) + or node.isImplicitArgument(_, _, _) or exists(U::Expr e | From ac8adeed8697533a5d6a6849228584ff4e71c77d Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 27 Oct 2026 09:39:34 +0100 Subject: [PATCH 26/38] unified: Add test for receiver updates from getter reads Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../dataflow/accessors_instance.swift | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/unified/ql/test/library-tests/dataflow/accessors_instance.swift b/unified/ql/test/library-tests/dataflow/accessors_instance.swift index a03f324eee0e..fa3ff41e0245 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_instance.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_instance.swift @@ -98,3 +98,31 @@ func t8() { c.x = "safe" // trigger the setter sink(c.otherField) // $ hasTaintFlow=t8.1 } + +func t9() { + class C { + var field: String = "safe" + var x: String { + field = source("t9.1") + return "" + } + } + let c = C() + let ignored = c.x + sink(c.field) // $ MISSING: hasValueFlow=t9.1 +} + +func t10() { + class C { + var field: String = "safe" + var x: String { + field = source("t10.1") + return "" + } + + func method() { + let ignored = x + sink(field) // $ MISSING: hasValueFlow=t10.1 + } + } +} From 674189608a1543a2682533c1f47bb256f7a8fdd3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 27 Oct 2026 10:26:59 +0100 Subject: [PATCH 27/38] unified: Simple fix for getter calls missing post update node --- .../internal/dataflow/DataFlowNode.qll | 29 +++++++++++++++++-- .../dataflow/accessors_instance.swift | 4 +-- .../test/library-tests/dataflow/test.expected | 14 +++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll index 84b91da3e924..06424fdc78de 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll @@ -27,9 +27,10 @@ private predicate hasIncomingValueAtCfgNode(Expr expr, ControlFlowNode cfgNode) ) } -private predicate hasPostUpdate(Expr expr, ControlFlowNode cfgNode) { +/** Holds if `expr` has a post-update at `cfgNode` resulting from a surrounding call or assignment. */ +private predicate hasPostUpdateAfterCallOrAssign(Expr expr, ControlFlowNode cfgNode) { exists(MemberAccessExpr member | - (hasIncomingValueAtCfgNode(member, cfgNode) or hasPostUpdate(member, cfgNode)) and + (hasIncomingValueAtCfgNode(member, cfgNode) or hasPostUpdateAfterCallOrAssign(member, cfgNode)) and expr = member.getBase() ) or @@ -40,11 +41,28 @@ private predicate hasPostUpdate(Expr expr, ControlFlowNode cfgNode) { ) or exists(Expr target | - hasPostUpdate(target, cfgNode) and + hasPostUpdateAfterCallOrAssign(target, cfgNode) and exprStep(expr, target, _) ) } +/** + * Holds if `expr` has a post-update at `cfgNode`. + */ +private predicate hasPostUpdate(Expr expr, ControlFlowNode cfgNode) { + hasPostUpdateAfterCallOrAssign(expr, cfgNode) + or + not hasPostUpdateAfterCallOrAssign(expr, _) and + exists(MemberAccessExpr member | + // If the member access is not updated by call or assignment, it might still be a call to a getter, + // and thus needs a post-update. Generate a post-update for this case. + // In case it's a call to a getter, while also being post-updated from a call or assignment, we accept + // the imprecision and use the post-update position from the call or assignment. + expr = member.getBase() and + cfgNode.isAfter(member) + ) +} + /** * Holds if `repr` performs an access to `var` of the given `kind` at `cfgNode`. * @@ -69,6 +87,11 @@ predicate performsVariableAccess( (hasIncomingValueAtCfgNode(access, cfgNode) or hasPostUpdate(access, cfgNode)) and kind.isPostUpdate() or + // Generate a post-update for the receiver in case this is a call to a getter that mutates 'self'. + not (hasIncomingValueAtCfgNode(access, _) or hasPostUpdate(access, _)) and + cfgNode.isAfter(access) and + kind.isPostUpdate() + or exists(CallExpr call | access = call.getCallee() and cfgNode.isAfter(call) and diff --git a/unified/ql/test/library-tests/dataflow/accessors_instance.swift b/unified/ql/test/library-tests/dataflow/accessors_instance.swift index fa3ff41e0245..ee702a4d1674 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_instance.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_instance.swift @@ -109,7 +109,7 @@ func t9() { } let c = C() let ignored = c.x - sink(c.field) // $ MISSING: hasValueFlow=t9.1 + sink(c.field) // $ hasValueFlow=t9.1 } func t10() { @@ -122,7 +122,7 @@ func t10() { func method() { let ignored = x - sink(field) // $ MISSING: hasValueFlow=t10.1 + sink(field) // $ hasValueFlow=t10.1 } } } diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index 37b29ef36f0d..dba5105d0e55 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -28,6 +28,11 @@ edges | accessors_instance.swift:98:5:98:5 | c [field] | accessors_instance.swift:93:32:93:36 | field | provenance | | | accessors_instance.swift:98:5:98:5 | c [field] | accessors_instance.swift:99:10:99:10 | c [otherField] | provenance | | | accessors_instance.swift:99:10:99:10 | c [otherField] | accessors_instance.swift:99:10:99:21 | ... .otherField | provenance | | +| accessors_instance.swift:106:13:106:17 | field | accessors_instance.swift:112:10:112:10 | c [field] | provenance | | +| accessors_instance.swift:106:21:106:34 | source(...) | accessors_instance.swift:106:13:106:17 | field | provenance | | +| accessors_instance.swift:112:10:112:10 | c [field] | accessors_instance.swift:112:10:112:16 | ... .field | provenance | | +| accessors_instance.swift:119:13:119:17 | field | accessors_instance.swift:125:18:125:22 | field | provenance | | +| accessors_instance.swift:119:21:119:35 | source(...) | accessors_instance.swift:119:13:119:17 | field | provenance | | | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | | | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | | | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | provenance | | @@ -544,6 +549,13 @@ nodes | accessors_instance.swift:98:5:98:5 | c [field] | semmle.label | c [field] | | accessors_instance.swift:99:10:99:10 | c [otherField] | semmle.label | c [otherField] | | accessors_instance.swift:99:10:99:21 | ... .otherField | semmle.label | ... .otherField | +| accessors_instance.swift:106:13:106:17 | field | semmle.label | field | +| accessors_instance.swift:106:21:106:34 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:112:10:112:10 | c [field] | semmle.label | c [field] | +| accessors_instance.swift:112:10:112:16 | ... .field | semmle.label | ... .field | +| accessors_instance.swift:119:13:119:17 | field | semmle.label | field | +| accessors_instance.swift:119:21:119:35 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:125:18:125:22 | field | semmle.label | field | | accessors_local.swift:7:21:7:34 | source(...) | semmle.label | source(...) | | accessors_local.swift:8:10:8:10 | x | semmle.label | x | | accessors_local.swift:15:15:15:28 | source(...) | semmle.label | source(...) | @@ -1171,6 +1183,8 @@ testFailures | accessors_instance.swift:71:18:71:18 | x | accessors_instance.swift:67:25:67:38 | source(...) | accessors_instance.swift:71:18:71:18 | x | $@ | accessors_instance.swift:67:25:67:38 | source(...) | source(...) | | accessors_instance.swift:84:10:84:12 | ... .x | accessors_instance.swift:83:15:83:28 | source(...) | accessors_instance.swift:84:10:84:12 | ... .x | $@ | accessors_instance.swift:83:15:83:28 | source(...) | source(...) | | accessors_instance.swift:99:10:99:21 | ... .otherField | accessors_instance.swift:97:15:97:28 | source(...) | accessors_instance.swift:99:10:99:21 | ... .otherField | $@ | accessors_instance.swift:97:15:97:28 | source(...) | source(...) | +| accessors_instance.swift:112:10:112:16 | ... .field | accessors_instance.swift:106:21:106:34 | source(...) | accessors_instance.swift:112:10:112:16 | ... .field | $@ | accessors_instance.swift:106:21:106:34 | source(...) | source(...) | +| accessors_instance.swift:125:18:125:22 | field | accessors_instance.swift:119:21:119:35 | source(...) | accessors_instance.swift:125:18:125:22 | field | $@ | accessors_instance.swift:119:21:119:35 | source(...) | source(...) | | accessors_local.swift:8:10:8:10 | x | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | $@ | accessors_local.swift:7:21:7:34 | source(...) | source(...) | | accessors_local.swift:17:10:17:10 | x | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | $@ | accessors_local.swift:15:15:15:28 | source(...) | source(...) | | accessors_local.swift:25:10:25:10 | x | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | $@ | accessors_local.swift:22:15:22:28 | source(...) | source(...) | From 4a510f7689e3cf44e6140a96795a47eb4e031f34 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 27 Oct 2026 12:58:46 +0100 Subject: [PATCH 28/38] unified: More precise CFG node for implicit setter arguments --- .../unified/internal/dataflow/DataFlowNode.qll | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll index 06424fdc78de..bd07fd7d1151 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll @@ -379,18 +379,19 @@ class BuilderNode extends TDataFlowNodeStage1 { exists(DataFlowCall call, Expr access, boolean isPost | this.isImplicitArgument(call, _, isPost) | - ( - access = call.asExplicitCall() - or - access = call.asGetterCall() - or - access = call.asSetterCall() - ) and + (access = call.asExplicitCall() or access = call.asGetterCall()) and ( isPost = false and cfgNode.injects(access) or isPost = true and cfgNode.isAfter(access) ) + or + access = call.asSetterCall() and + exists(Assignment assignment | isInAssignmentContext(access, assignment) | + isPost = false and cfgNode.injects(assignment) + or + isPost = true and cfgNode.isAfter(assignment) + ) ) ) } From ed70ef4bc6014fe46b49075d8bcacdce3a593d61 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 27 Oct 2026 13:40:23 +0100 Subject: [PATCH 29/38] unified: Support setter calls from writebacks If `p` is a getter/setter pair for a value type, then `p.f = x` ends with a call to p's setter with the updated value (called "writebacks" in Swift). --- .../DataFlowConsistency.ql | 4 +-- .../unified/internal/dataflow/CaptureSsa.qll | 2 +- .../internal/dataflow/DataFlowCall.qll | 7 +++- .../dataflow/DataFlowInstantiation.qll | 7 ++-- .../internal/dataflow/DataFlowNode.qll | 36 +++++++++++++++++-- .../dataflow/accessors_instance.swift | 26 ++++++++++++++ .../dataflow/accessors_local.swift | 8 +++++ .../test/library-tests/dataflow/test.expected | 24 +++++++++++++ 8 files changed, 102 insertions(+), 12 deletions(-) diff --git a/unified/ql/consistency-queries/DataFlowConsistency.ql b/unified/ql/consistency-queries/DataFlowConsistency.ql index eaaeec834ef7..71cd02531030 100644 --- a/unified/ql/consistency-queries/DataFlowConsistency.ql +++ b/unified/ql/consistency-queries/DataFlowConsistency.ql @@ -6,9 +6,9 @@ module ConsistencyInput implements InputSig { predicate argHasPostUpdateExclude(DataFlowInput::ArgumentNode n) { not exists(n.getBasicBlock()) // ignore unreachable data flow nodes or - // The RHS value passed into a setter call technically needs a post-update node + // The value passed into a setter call technically needs a post-update node // because it's an argument to a call, but this rarely matters in practice. - n.isIncomingValue(any(DataFlowCall c).asSetterCall()) + n.(BuilderNode).isSetterValueArgument(_) } predicate reverseReadExclude(DataFlow::Node n) { diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll index 08902d55da69..9a53e78af93b 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll @@ -93,7 +93,7 @@ module CaptureSsaInput implements InputSig { private predicate isEscape(BuilderNode node) { node.isResultValue(any(Argument arg).getValue()) or - node.isIncomingValue(any(DataFlowCall call).asSetterCall()) + node.isSetterValueArgument(_) or node.isImplicitArgument(_, _, _) or diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll index 9a12befe8b27..0328b25f32fc 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll @@ -16,7 +16,12 @@ private newtype TDataFlowCall = ) } or TSetterCall(Expr expr) { - isInAssignmentContext(expr, _) and + ( + isInAssignmentContext(expr, _) + or + // Write back a value that was mutated in-place, such as `x.p` in `x.p.f = y` + isMutatedByCallOrAssign(expr, _) + ) and ( N::getStaticBindingTargetFromRef(expr) = any(SetterDeclaration g).getNameNode() or diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll index 03b92a35e80e..d376be2cb84a 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll @@ -107,11 +107,8 @@ module DataFlowInput implements InputSig { or n.(BuilderNode).isImplicitArgument(call, pos, false) or - exists(Expr access | - call.asSetterCall() = access and - n.isIncomingValue(access) and - pos.asPositional() = 0 - ) + n.(BuilderNode).isSetterValueArgument(call) and + pos.asPositional() = 0 or exists(FlowSummaryOutput::SummaryNode callee | FlowSummaryOutput::summaryArgumentNode(callee, getSummaryNode(n), pos) and diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll index bd07fd7d1151..337ec1a5cc21 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll @@ -46,6 +46,14 @@ private predicate hasPostUpdateAfterCallOrAssign(Expr expr, ControlFlowNode cfgN ) } +/** + * Holds if the value of `expr` is mutated at `cfgNode` by a surrounding call or assignment, + * and thus needs to be written back if `expr` refers to a computed value. + */ +predicate isMutatedByCallOrAssign(Expr expr, ControlFlowNode cfgNode) { + hasPostUpdateAfterCallOrAssign(expr, cfgNode) +} + /** * Holds if `expr` has a post-update at `cfgNode`. */ @@ -178,6 +186,22 @@ class BuilderNode extends TDataFlowNodeStage1 { /** Holds if this represents the updated state of the value returned by `expr` after it has been mutated by the surrounding assignment or call. */ predicate isPostUpdate(Expr expr) { this = TExprPostUpdateNode(expr) } + /** + * Holds if this node represents the new value passed to the setter invoked by `call`. + * + * For a direct assignment, such as `x.p = y`, this is the incoming value of `x.p`. + * When the value of a computed property is mutated in place, such as `x.p.f = y`, + * this is the post-update node of `x.p`, as the mutated value is written back via the setter. + */ + predicate isSetterValueArgument(DataFlowCall call) { + exists(Expr access | access = call.asSetterCall() | + this.isIncomingValue(access) + or + not isInAssignmentContext(access, _) and + this.isPostUpdate(access) + ) + } + /** Holds if this node is the canonical representative for the return value of `callable`. */ predicate isReturnNode(Callable callable) { this.isReturnNodeEx(toDataFlowCallable(callable)) } @@ -387,10 +411,16 @@ class BuilderNode extends TDataFlowNodeStage1 { ) or access = call.asSetterCall() and - exists(Assignment assignment | isInAssignmentContext(access, assignment) | - isPost = false and cfgNode.injects(assignment) + ( + exists(Assignment assignment | isInAssignmentContext(access, assignment) | + isPost = false and cfgNode.injects(assignment) + or + isPost = true and cfgNode.isAfter(assignment) + ) or - isPost = true and cfgNode.isAfter(assignment) + // Setter invoked to write back a mutated value + not isInAssignmentContext(access, _) and + isMutatedByCallOrAssign(access, cfgNode) ) ) ) diff --git a/unified/ql/test/library-tests/dataflow/accessors_instance.swift b/unified/ql/test/library-tests/dataflow/accessors_instance.swift index ee702a4d1674..4771c48dc2a6 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_instance.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_instance.swift @@ -126,3 +126,29 @@ func t10() { } } } + +func t11() { + class C { + var p: (String, String) { + get { ("safe", "safe") } + set { sink(newValue.0) } // $ hasValueFlow=t11.1 + } + } + let c = C() + c.p.0 = source("t11.1") +} + +func t12() { + struct S { + var field: String = "safe" + mutating func update() { field = source("t12.1") } + } + class C { + var s: S { + get { S() } + set { sink(newValue.field) } // $ MISSING: hasValueFlow=t12.1 + } + } + let c = C() + c.s.update() // MISSING: the call to 'update' is not resolved +} diff --git a/unified/ql/test/library-tests/dataflow/accessors_local.swift b/unified/ql/test/library-tests/dataflow/accessors_local.swift index c5b19297e630..241862476cb2 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_local.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_local.swift @@ -84,3 +84,11 @@ func t10() { y = source("t9.1") closure() } + +func t11() { + var p: (String, String) { + get { ("safe", "safe") } + set { sink(newValue.0) } // $ hasValueFlow=t11.1 + } + p.0 = source("t11.1") +} diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index dba5105d0e55..5f5ceee231f9 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -33,6 +33,11 @@ edges | accessors_instance.swift:112:10:112:10 | c [field] | accessors_instance.swift:112:10:112:16 | ... .field | provenance | | | accessors_instance.swift:119:13:119:17 | field | accessors_instance.swift:125:18:125:22 | field | provenance | | | accessors_instance.swift:119:21:119:35 | source(...) | accessors_instance.swift:119:13:119:17 | field | provenance | | +| accessors_instance.swift:134:13:134:15 | newValue [0] | accessors_instance.swift:134:24:134:31 | newValue [0] | provenance | | +| accessors_instance.swift:134:24:134:31 | newValue [0] | accessors_instance.swift:134:24:134:33 | ... .0 | provenance | | +| accessors_instance.swift:138:5:138:7 | [post] ... .p [0] | accessors_instance.swift:134:13:134:15 | newValue [0] | provenance | | +| accessors_instance.swift:138:5:138:9 | ... .0 | accessors_instance.swift:138:5:138:7 | [post] ... .p [0] | provenance | | +| accessors_instance.swift:138:13:138:27 | source(...) | accessors_instance.swift:138:5:138:9 | ... .0 | provenance | | | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | | | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | | | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | provenance | | @@ -54,6 +59,11 @@ edges | accessors_local.swift:72:9:72:22 | source(...) | accessors_local.swift:72:5:72:5 | y | provenance | | | accessors_local.swift:84:5:84:5 | y | accessors_local.swift:82:14:82:15 | x3 | provenance | | | accessors_local.swift:84:9:84:22 | source(...) | accessors_local.swift:84:5:84:5 | y | provenance | | +| accessors_local.swift:91:9:91:11 | newValue [0] | accessors_local.swift:91:20:91:27 | newValue [0] | provenance | | +| accessors_local.swift:91:20:91:27 | newValue [0] | accessors_local.swift:91:20:91:29 | ... .0 | provenance | | +| accessors_local.swift:93:5:93:5 | [post] p [0] | accessors_local.swift:91:9:91:11 | newValue [0] | provenance | | +| accessors_local.swift:93:5:93:7 | ... .0 | accessors_local.swift:93:5:93:5 | [post] p [0] | provenance | | +| accessors_local.swift:93:11:93:25 | source(...) | accessors_local.swift:93:5:93:7 | ... .0 | provenance | | | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | provenance | | | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | provenance | | | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | provenance | | @@ -556,6 +566,12 @@ nodes | accessors_instance.swift:119:13:119:17 | field | semmle.label | field | | accessors_instance.swift:119:21:119:35 | source(...) | semmle.label | source(...) | | accessors_instance.swift:125:18:125:22 | field | semmle.label | field | +| accessors_instance.swift:134:13:134:15 | newValue [0] | semmle.label | newValue [0] | +| accessors_instance.swift:134:24:134:31 | newValue [0] | semmle.label | newValue [0] | +| accessors_instance.swift:134:24:134:33 | ... .0 | semmle.label | ... .0 | +| accessors_instance.swift:138:5:138:7 | [post] ... .p [0] | semmle.label | [post] ... .p [0] | +| accessors_instance.swift:138:5:138:9 | ... .0 | semmle.label | ... .0 | +| accessors_instance.swift:138:13:138:27 | source(...) | semmle.label | source(...) | | accessors_local.swift:7:21:7:34 | source(...) | semmle.label | source(...) | | accessors_local.swift:8:10:8:10 | x | semmle.label | x | | accessors_local.swift:15:15:15:28 | source(...) | semmle.label | source(...) | @@ -587,6 +603,12 @@ nodes | accessors_local.swift:82:14:82:15 | x3 | semmle.label | x3 | | accessors_local.swift:84:5:84:5 | y | semmle.label | y | | accessors_local.swift:84:9:84:22 | source(...) | semmle.label | source(...) | +| accessors_local.swift:91:9:91:11 | newValue [0] | semmle.label | newValue [0] | +| accessors_local.swift:91:20:91:27 | newValue [0] | semmle.label | newValue [0] | +| accessors_local.swift:91:20:91:29 | ... .0 | semmle.label | ... .0 | +| accessors_local.swift:93:5:93:5 | [post] p [0] | semmle.label | [post] p [0] | +| accessors_local.swift:93:5:93:7 | ... .0 | semmle.label | ... .0 | +| accessors_local.swift:93:11:93:25 | source(...) | semmle.label | source(...) | | accessors_static.swift:8:32:8:45 | source(...) | semmle.label | source(...) | | accessors_static.swift:10:10:10:12 | ... .x | semmle.label | ... .x | | accessors_static.swift:18:19:18:32 | source(...) | semmle.label | source(...) | @@ -1185,6 +1207,7 @@ testFailures | accessors_instance.swift:99:10:99:21 | ... .otherField | accessors_instance.swift:97:15:97:28 | source(...) | accessors_instance.swift:99:10:99:21 | ... .otherField | $@ | accessors_instance.swift:97:15:97:28 | source(...) | source(...) | | accessors_instance.swift:112:10:112:16 | ... .field | accessors_instance.swift:106:21:106:34 | source(...) | accessors_instance.swift:112:10:112:16 | ... .field | $@ | accessors_instance.swift:106:21:106:34 | source(...) | source(...) | | accessors_instance.swift:125:18:125:22 | field | accessors_instance.swift:119:21:119:35 | source(...) | accessors_instance.swift:125:18:125:22 | field | $@ | accessors_instance.swift:119:21:119:35 | source(...) | source(...) | +| accessors_instance.swift:134:24:134:33 | ... .0 | accessors_instance.swift:138:13:138:27 | source(...) | accessors_instance.swift:134:24:134:33 | ... .0 | $@ | accessors_instance.swift:138:13:138:27 | source(...) | source(...) | | accessors_local.swift:8:10:8:10 | x | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | $@ | accessors_local.swift:7:21:7:34 | source(...) | source(...) | | accessors_local.swift:17:10:17:10 | x | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | $@ | accessors_local.swift:15:15:15:28 | source(...) | source(...) | | accessors_local.swift:25:10:25:10 | x | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | $@ | accessors_local.swift:22:15:22:28 | source(...) | source(...) | @@ -1195,6 +1218,7 @@ testFailures | accessors_local.swift:61:14:61:14 | x | accessors_local.swift:59:21:59:34 | source(...) | accessors_local.swift:61:14:61:14 | x | $@ | accessors_local.swift:59:21:59:34 | source(...) | source(...) | | accessors_local.swift:70:14:70:14 | x | accessors_local.swift:72:9:72:22 | source(...) | accessors_local.swift:70:14:70:14 | x | $@ | accessors_local.swift:72:9:72:22 | source(...) | source(...) | | accessors_local.swift:82:14:82:15 | x3 | accessors_local.swift:84:9:84:22 | source(...) | accessors_local.swift:82:14:82:15 | x3 | $@ | accessors_local.swift:84:9:84:22 | source(...) | source(...) | +| accessors_local.swift:91:20:91:29 | ... .0 | accessors_local.swift:93:11:93:25 | source(...) | accessors_local.swift:91:20:91:29 | ... .0 | $@ | accessors_local.swift:93:11:93:25 | source(...) | source(...) | | accessors_static.swift:10:10:10:12 | ... .x | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | $@ | accessors_static.swift:8:32:8:45 | source(...) | source(...) | | accessors_static.swift:21:10:21:12 | ... .x | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | $@ | accessors_static.swift:18:19:18:32 | source(...) | source(...) | | accessors_static.swift:31:10:31:12 | ... .x | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | $@ | accessors_static.swift:27:19:27:32 | source(...) | source(...) | From fcf39ec04ab7ef1ccbf19a817e90d31dbd6480f6 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 27 Oct 2026 13:59:01 +0100 Subject: [PATCH 30/38] unified: Add a hook for writebacks Writebacks are a language-specific thing so let's add a hook for it --- .../unified/internal/dataflow/DataFlowCall.qll | 3 ++- .../internal/dataflow/DataFlowPlugin.qll | 18 ++++++++++++++++++ .../internal/dataflow/DataFlowPluginSwift.qll | 4 ++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll index 0328b25f32fc..95a20c70e4c4 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll @@ -20,7 +20,8 @@ private newtype TDataFlowCall = isInAssignmentContext(expr, _) or // Write back a value that was mutated in-place, such as `x.p` in `x.p.f = y` - isMutatedByCallOrAssign(expr, _) + isMutatedByCallOrAssign(expr, _) and + mayWriteBackViaSetterCall(expr) ) and ( N::getStaticBindingTargetFromRef(expr) = any(SetterDeclaration g).getNameNode() diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll index 05a26ad2d9b4..9b48861293e2 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll @@ -26,6 +26,15 @@ class DataFlowPlugin extends Unit { * This affects the steps generated by the language-agnostic code as well as steps from the plugin itself. */ predicate bypassContentSet(ContentSet contents) { none() } + + /** + * Holds if mutating the value of `expr` in-place, such as `expr.f = y` or `expr.mutate()`, + * may cause the mutated value to be written back via a setter call. + * + * `expr` is an expression in getter position, such as a member access. + */ + bindingset[expr] + predicate mayWriteBackViaSetterCall(Expr expr) { none() } } /** @@ -40,3 +49,12 @@ predicate bypassContentSet(ContentSet contents) { any(DataFlowPlugin p).bypassCo predicate exprStep(Expr expr1, Expr expr2, boolean mustFlow) { any(DataFlowPlugin p).exprStep(expr1, expr2, mustFlow) } + +/** + * Holds if mutating the value of `expr` in-place, such as `expr.f = y` or `expr.mutate()`, + * may cause the mutated value to be written back via a setter call. + */ +bindingset[expr] +predicate mayWriteBackViaSetterCall(Expr expr) { + any(DataFlowPlugin p).mayWriteBackViaSetterCall(expr) +} diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll index 95247afd8e43..7147a0a7101c 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll @@ -11,6 +11,10 @@ private class SwiftDataFlowPlugin extends DataFlowPlugin { contents.asSingleton().asNamedMember() = "some.0" } + // Swift writes back mutated values of computed properties via their setter + bindingset[expr] + override predicate mayWriteBackViaSetterCall(Expr expr) { any() } + override predicate exprStep(Expr expr1, Expr expr2, boolean mustFlow) { mustFlow = true and exists(UnaryExpr expr | From 3769c468af1fa8b85add52dc96370cd27a8a6471 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 27 Oct 2026 14:07:18 +0100 Subject: [PATCH 31/38] unified: Test that writebacks after a mutating call works --- .../library-tests/dataflow/accessors_local.swift | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/unified/ql/test/library-tests/dataflow/accessors_local.swift b/unified/ql/test/library-tests/dataflow/accessors_local.swift index 241862476cb2..51c5a62e2894 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_local.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_local.swift @@ -92,3 +92,17 @@ func t11() { } p.0 = source("t11.1") } + +func t12() { + struct S { + var field: String = "safe" + mutating func mutate() { field = source("t12.1") } + } + var p: S { + get { S() } + set { sink(newValue.field) } // $ MISSING: hasValueFlow=t12.1 + } + // TODO: test fails because the call to 'mutate' is not yet resolved + // At the time of writing, the test passes if the call graph is manually patched + p.mutate() +} From 0d1b4b957356356ae3de510d47778defa5c432ab Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 28 Oct 2026 11:20:17 +0100 Subject: [PATCH 32/38] unified: Be more precise about writebacks --- .../internal/dataflow/DataFlowPlugin.qll | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll index 9b48861293e2..b52a97816ddb 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll @@ -5,6 +5,8 @@ private import unified private import AllDataFlow private import codeql.util.Unit +private import codeql.unified.internal.ExprPositions +private import codeql.unified.internal.typeinference.TypeInference as T private module Plugins { private import DataFlowPluginSwift @@ -34,7 +36,37 @@ class DataFlowPlugin extends Unit { * `expr` is an expression in getter position, such as a member access. */ bindingset[expr] - predicate mayWriteBackViaSetterCall(Expr expr) { none() } + predicate mayWriteBackViaSetterCall(Expr expr) { + isInWriteBackPosition(expr) + // TODO: Check that 'expr' has a value type. Reference types don't cause writebacks. + } +} + +/** + * Holds if `expr` is target of an assignment or 'inout' argument, + * or the base of such an expression, e.g. `x` in `x.f1.f2 = foo()` + */ +private predicate isInWriteBackPosition(Expr expr) { + isInAssignmentContext(expr, _) + or + // Arguments to an 'inout' parameter must be prefixed with '&' + exists(UnaryExpr unary | + unary.getOperator().(PrefixOperator).getValue() = "&" and + unary.getOperand() = expr + // TODO: support custom operators with an 'inout' parameter, which lack the '&' indicator + ) + or + // The receiver of a call to a 'mutating' function counts as an 'inout' argument + exists(CallExpr call, Callable target | + target = T::resolveCallTarget(call) and + target.hasModifier("mutating") and + expr = call.getCallee().(MemberAccessExpr).getBase() + ) + or + exists(MemberAccessExpr member | + isInWriteBackPosition(member) and + expr = member.getBase() + ) } /** From 405e32277ff0e8071f7fb0f2488c522140556d2b Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 28 Oct 2026 11:20:49 +0100 Subject: [PATCH 33/38] unified: Remove superfluous forwarding-predicate isMutatedByCallOrAssign was a synonym for hasPostUpdateAfterCallOrAssign --- .../unified/internal/dataflow/DataFlowCall.qll | 2 +- .../unified/internal/dataflow/DataFlowNode.qll | 12 ++---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll index 95a20c70e4c4..d25d1fda51e1 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll @@ -20,7 +20,7 @@ private newtype TDataFlowCall = isInAssignmentContext(expr, _) or // Write back a value that was mutated in-place, such as `x.p` in `x.p.f = y` - isMutatedByCallOrAssign(expr, _) and + hasPostUpdateAfterCallOrAssign(expr, _) and mayWriteBackViaSetterCall(expr) ) and ( diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll index 337ec1a5cc21..a520ae9e10a6 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll @@ -28,7 +28,7 @@ private predicate hasIncomingValueAtCfgNode(Expr expr, ControlFlowNode cfgNode) } /** Holds if `expr` has a post-update at `cfgNode` resulting from a surrounding call or assignment. */ -private predicate hasPostUpdateAfterCallOrAssign(Expr expr, ControlFlowNode cfgNode) { +predicate hasPostUpdateAfterCallOrAssign(Expr expr, ControlFlowNode cfgNode) { exists(MemberAccessExpr member | (hasIncomingValueAtCfgNode(member, cfgNode) or hasPostUpdateAfterCallOrAssign(member, cfgNode)) and expr = member.getBase() @@ -46,14 +46,6 @@ private predicate hasPostUpdateAfterCallOrAssign(Expr expr, ControlFlowNode cfgN ) } -/** - * Holds if the value of `expr` is mutated at `cfgNode` by a surrounding call or assignment, - * and thus needs to be written back if `expr` refers to a computed value. - */ -predicate isMutatedByCallOrAssign(Expr expr, ControlFlowNode cfgNode) { - hasPostUpdateAfterCallOrAssign(expr, cfgNode) -} - /** * Holds if `expr` has a post-update at `cfgNode`. */ @@ -420,7 +412,7 @@ class BuilderNode extends TDataFlowNodeStage1 { or // Setter invoked to write back a mutated value not isInAssignmentContext(access, _) and - isMutatedByCallOrAssign(access, cfgNode) + hasPostUpdate(access, cfgNode) ) ) ) From bd2d5a435e1a231ba81cd7e2e0724e51000bf788 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 28 Oct 2026 10:23:48 +0100 Subject: [PATCH 34/38] Enhance step function to include valueMustFlow check Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll index 9a53e78af93b..1e0103469370 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll @@ -71,7 +71,7 @@ module CaptureSsaInput implements InputSig { node.(BuilderNode).isCallable(callable) or exists(Stage2Node prev | callableHasLocalAlias(callable, prev) | - step(prev, any(Step s | s.value() or s.callGraph()), node) + step(prev, any(Step s | s.value() or s.valueMustFlow() or s.callGraph()), node) or localSsaStep(prev, node, _) ) From 388acff266372b22a4c8fb4a5a6023419cace8fb Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 28 Oct 2026 11:44:21 +0100 Subject: [PATCH 35/38] unified: Exclude UnqualifiedMemberAccess in fallback UnqualifiedMemberAccess is only populated when static name binding positively identifies the resolution target, so either the first case catches it, and if unresolved, the second case will miss it anyway because it won't be a MemberAccess instance. --- .../lib/codeql/unified/internal/dataflow/DataFlowCall.qll | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll index d25d1fda51e1..f99f1f90e37a 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll @@ -12,7 +12,8 @@ private newtype TDataFlowCall = ( N::getStaticBindingTargetFromRef(expr) = any(GetterDeclaration g).getNameNode() or - expr instanceof MemberAccess + // Assume a qualified member accesses might resolve to a getter. + expr instanceof MemberAccessExpr ) } or TSetterCall(Expr expr) { @@ -26,7 +27,8 @@ private newtype TDataFlowCall = ( N::getStaticBindingTargetFromRef(expr) = any(SetterDeclaration g).getNameNode() or - expr instanceof MemberAccess + // Assume a qualified member accesses might resolve to a setter. + expr instanceof MemberAccessExpr ) } or TSummaryCall(SummarizedCallable callable, SummaryNode callee) { From 88c47861d19932eba55056902af5f80168bbb367 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 28 Oct 2026 12:50:12 +0100 Subject: [PATCH 36/38] unified: Fix bad join --- .../codeql/unified/internal/dataflow/CallGraph.qll | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll b/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll index 04672886c699..bd8314dcaee6 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll @@ -14,14 +14,19 @@ DataFlowCallable viableCallable(DataFlowCall c) { result.asSummarizedCallable().matchesCall(c.asExplicitCall()) } +bindingset[access] +pragma[inline_late] +private N::NamespaceNode resolveBase(MemberAccessExpr access) { + result.isInstanceMemberNamespace(T::inferType(access.getBase()).asClass()) +} + private AccessorDeclaration resolveAccessor(Expr access) { result.getNameNode() = N::getStaticBindingTargetFromRef(access) or not exists(N::getStaticBindingTargetFromRef(access)) and - exists(MemberAccessExpr m, N::NamespaceNode namespace | + exists(MemberAccessExpr m | access = m and // TODO: Make type inference own accessor-resolution. It also needs it internally. - namespace.isInstanceMemberNamespace(T::inferType(m.getBase()).asClass()) and - namespace.getMember(m.getMemberName()).isMember(result) + resolveBase(m).getMember(m.getMemberName()).isMember(result) ) } From 8ebaab957b9a913020848d37c4e08ac90446f9b8 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 29 Oct 2026 08:38:00 +0100 Subject: [PATCH 37/38] unified: Hide generated nodes --- .../dataflow/DataFlowInstantiation.qll | 1 + .../test/library-tests/dataflow/test.expected | 32 +++++-------------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll index d376be2cb84a..1c18b907c57c 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll @@ -235,6 +235,7 @@ module DataFlowInput implements InputSig { // additional predicate nodeIsVisible(Node node) { exists(Expr e | + e.fromSource() and node = TValueNode(e) and not e instanceof ExprPattern ) diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index 5f5ceee231f9..6d237e15124b 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -3,15 +3,13 @@ edges | accessors_instance.swift:8:25:8:38 | source(...) | accessors_instance.swift:10:10:10:14 | ... .x | provenance | | | accessors_instance.swift:18:19:18:32 | source(...) | accessors_instance.swift:21:10:21:14 | ... .x | provenance | | | accessors_instance.swift:27:19:27:32 | source(...) | accessors_instance.swift:31:10:31:14 | ... .x | provenance | | -| accessors_instance.swift:38:13:38:15 | newValue | accessors_instance.swift:38:24:38:31 | newValue | provenance | | -| accessors_instance.swift:41:5:41:9 | ... .x | accessors_instance.swift:38:13:38:15 | newValue | provenance | | +| accessors_instance.swift:41:5:41:9 | ... .x | accessors_instance.swift:38:24:38:31 | newValue | provenance | | | accessors_instance.swift:41:13:41:26 | source(...) | accessors_instance.swift:41:5:41:9 | ... .x | provenance | | | accessors_instance.swift:48:17:48:17 | v | accessors_instance.swift:48:27:48:27 | v | provenance | | | accessors_instance.swift:51:5:51:9 | ... .x | accessors_instance.swift:48:17:48:17 | v | provenance | | | accessors_instance.swift:51:13:51:27 | source(...) | accessors_instance.swift:51:5:51:9 | ... .x | provenance | | | accessors_instance.swift:57:19:57:32 | source(...) | accessors_instance.swift:62:11:62:13 | ... .x | provenance | | -| accessors_instance.swift:58:13:58:15 | newValue | accessors_instance.swift:58:24:58:31 | newValue | provenance | | -| accessors_instance.swift:62:5:62:7 | ... .x | accessors_instance.swift:58:13:58:15 | newValue | provenance | | +| accessors_instance.swift:62:5:62:7 | ... .x | accessors_instance.swift:58:24:58:31 | newValue | provenance | | | accessors_instance.swift:62:11:62:13 | ... .x | accessors_instance.swift:62:5:62:7 | ... .x | provenance | | | accessors_instance.swift:67:25:67:38 | source(...) | accessors_instance.swift:70:18:70:23 | ... .x | provenance | | | accessors_instance.swift:67:25:67:38 | source(...) | accessors_instance.swift:71:18:71:18 | x | provenance | | @@ -33,20 +31,17 @@ edges | accessors_instance.swift:112:10:112:10 | c [field] | accessors_instance.swift:112:10:112:16 | ... .field | provenance | | | accessors_instance.swift:119:13:119:17 | field | accessors_instance.swift:125:18:125:22 | field | provenance | | | accessors_instance.swift:119:21:119:35 | source(...) | accessors_instance.swift:119:13:119:17 | field | provenance | | -| accessors_instance.swift:134:13:134:15 | newValue [0] | accessors_instance.swift:134:24:134:31 | newValue [0] | provenance | | | accessors_instance.swift:134:24:134:31 | newValue [0] | accessors_instance.swift:134:24:134:33 | ... .0 | provenance | | -| accessors_instance.swift:138:5:138:7 | [post] ... .p [0] | accessors_instance.swift:134:13:134:15 | newValue [0] | provenance | | +| accessors_instance.swift:138:5:138:7 | [post] ... .p [0] | accessors_instance.swift:134:24:134:31 | newValue [0] | provenance | | | accessors_instance.swift:138:5:138:9 | ... .0 | accessors_instance.swift:138:5:138:7 | [post] ... .p [0] | provenance | | | accessors_instance.swift:138:13:138:27 | source(...) | accessors_instance.swift:138:5:138:9 | ... .0 | provenance | | | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | | | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | | | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | provenance | | -| accessors_local.swift:31:9:31:11 | newValue | accessors_local.swift:31:20:31:27 | newValue | provenance | | -| accessors_local.swift:33:5:33:5 | x | accessors_local.swift:31:9:31:11 | newValue | provenance | | +| accessors_local.swift:33:5:33:5 | x | accessors_local.swift:31:20:31:27 | newValue | provenance | | | accessors_local.swift:33:9:33:22 | source(...) | accessors_local.swift:33:5:33:5 | x | provenance | | | accessors_local.swift:38:15:38:28 | source(...) | accessors_local.swift:41:9:41:9 | x | provenance | | -| accessors_local.swift:39:9:39:11 | newValue | accessors_local.swift:39:20:39:27 | newValue | provenance | | -| accessors_local.swift:41:5:41:5 | x | accessors_local.swift:39:9:39:11 | newValue | provenance | | +| accessors_local.swift:41:5:41:5 | x | accessors_local.swift:39:20:39:27 | newValue | provenance | | | accessors_local.swift:41:9:41:9 | x | accessors_local.swift:41:5:41:5 | x | provenance | | | accessors_local.swift:45:9:45:9 | y | accessors_local.swift:46:21:46:21 | y | provenance | | | accessors_local.swift:45:9:45:9 | y | accessors_local.swift:47:10:47:10 | x | provenance | | @@ -59,20 +54,17 @@ edges | accessors_local.swift:72:9:72:22 | source(...) | accessors_local.swift:72:5:72:5 | y | provenance | | | accessors_local.swift:84:5:84:5 | y | accessors_local.swift:82:14:82:15 | x3 | provenance | | | accessors_local.swift:84:9:84:22 | source(...) | accessors_local.swift:84:5:84:5 | y | provenance | | -| accessors_local.swift:91:9:91:11 | newValue [0] | accessors_local.swift:91:20:91:27 | newValue [0] | provenance | | | accessors_local.swift:91:20:91:27 | newValue [0] | accessors_local.swift:91:20:91:29 | ... .0 | provenance | | -| accessors_local.swift:93:5:93:5 | [post] p [0] | accessors_local.swift:91:9:91:11 | newValue [0] | provenance | | +| accessors_local.swift:93:5:93:5 | [post] p [0] | accessors_local.swift:91:20:91:27 | newValue [0] | provenance | | | accessors_local.swift:93:5:93:7 | ... .0 | accessors_local.swift:93:5:93:5 | [post] p [0] | provenance | | | accessors_local.swift:93:11:93:25 | source(...) | accessors_local.swift:93:5:93:7 | ... .0 | provenance | | | accessors_static.swift:8:32:8:45 | source(...) | accessors_static.swift:10:10:10:12 | ... .x | provenance | | | accessors_static.swift:18:19:18:32 | source(...) | accessors_static.swift:21:10:21:12 | ... .x | provenance | | | accessors_static.swift:27:19:27:32 | source(...) | accessors_static.swift:31:10:31:12 | ... .x | provenance | | -| accessors_static.swift:38:13:38:15 | newValue | accessors_static.swift:38:24:38:31 | newValue | provenance | | -| accessors_static.swift:41:5:41:7 | ... .x | accessors_static.swift:38:13:38:15 | newValue | provenance | | +| accessors_static.swift:41:5:41:7 | ... .x | accessors_static.swift:38:24:38:31 | newValue | provenance | | | accessors_static.swift:41:11:41:24 | source(...) | accessors_static.swift:41:5:41:7 | ... .x | provenance | | | accessors_static.swift:47:19:47:32 | source(...) | accessors_static.swift:51:11:51:13 | ... .x | provenance | | -| accessors_static.swift:48:13:48:15 | newValue | accessors_static.swift:48:24:48:31 | newValue | provenance | | -| accessors_static.swift:51:5:51:7 | ... .x | accessors_static.swift:48:13:48:15 | newValue | provenance | | +| accessors_static.swift:51:5:51:7 | ... .x | accessors_static.swift:48:24:48:31 | newValue | provenance | | | accessors_static.swift:51:11:51:13 | ... .x | accessors_static.swift:51:5:51:7 | ... .x | provenance | | | accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:59:18:59:23 | ... .x | provenance | | | accessors_static.swift:56:32:56:45 | source(...) | accessors_static.swift:63:18:63:23 | ... .x | provenance | | @@ -528,7 +520,6 @@ nodes | accessors_instance.swift:21:10:21:14 | ... .x | semmle.label | ... .x | | accessors_instance.swift:27:19:27:32 | source(...) | semmle.label | source(...) | | accessors_instance.swift:31:10:31:14 | ... .x | semmle.label | ... .x | -| accessors_instance.swift:38:13:38:15 | newValue | semmle.label | newValue | | accessors_instance.swift:38:24:38:31 | newValue | semmle.label | newValue | | accessors_instance.swift:41:5:41:9 | ... .x | semmle.label | ... .x | | accessors_instance.swift:41:13:41:26 | source(...) | semmle.label | source(...) | @@ -537,7 +528,6 @@ nodes | accessors_instance.swift:51:5:51:9 | ... .x | semmle.label | ... .x | | accessors_instance.swift:51:13:51:27 | source(...) | semmle.label | source(...) | | accessors_instance.swift:57:19:57:32 | source(...) | semmle.label | source(...) | -| accessors_instance.swift:58:13:58:15 | newValue | semmle.label | newValue | | accessors_instance.swift:58:24:58:31 | newValue | semmle.label | newValue | | accessors_instance.swift:62:5:62:7 | ... .x | semmle.label | ... .x | | accessors_instance.swift:62:11:62:13 | ... .x | semmle.label | ... .x | @@ -566,7 +556,6 @@ nodes | accessors_instance.swift:119:13:119:17 | field | semmle.label | field | | accessors_instance.swift:119:21:119:35 | source(...) | semmle.label | source(...) | | accessors_instance.swift:125:18:125:22 | field | semmle.label | field | -| accessors_instance.swift:134:13:134:15 | newValue [0] | semmle.label | newValue [0] | | accessors_instance.swift:134:24:134:31 | newValue [0] | semmle.label | newValue [0] | | accessors_instance.swift:134:24:134:33 | ... .0 | semmle.label | ... .0 | | accessors_instance.swift:138:5:138:7 | [post] ... .p [0] | semmle.label | [post] ... .p [0] | @@ -578,12 +567,10 @@ nodes | accessors_local.swift:17:10:17:10 | x | semmle.label | x | | accessors_local.swift:22:15:22:28 | source(...) | semmle.label | source(...) | | accessors_local.swift:25:10:25:10 | x | semmle.label | x | -| accessors_local.swift:31:9:31:11 | newValue | semmle.label | newValue | | accessors_local.swift:31:20:31:27 | newValue | semmle.label | newValue | | accessors_local.swift:33:5:33:5 | x | semmle.label | x | | accessors_local.swift:33:9:33:22 | source(...) | semmle.label | source(...) | | accessors_local.swift:38:15:38:28 | source(...) | semmle.label | source(...) | -| accessors_local.swift:39:9:39:11 | newValue | semmle.label | newValue | | accessors_local.swift:39:20:39:27 | newValue | semmle.label | newValue | | accessors_local.swift:41:5:41:5 | x | semmle.label | x | | accessors_local.swift:41:9:41:9 | x | semmle.label | x | @@ -603,7 +590,6 @@ nodes | accessors_local.swift:82:14:82:15 | x3 | semmle.label | x3 | | accessors_local.swift:84:5:84:5 | y | semmle.label | y | | accessors_local.swift:84:9:84:22 | source(...) | semmle.label | source(...) | -| accessors_local.swift:91:9:91:11 | newValue [0] | semmle.label | newValue [0] | | accessors_local.swift:91:20:91:27 | newValue [0] | semmle.label | newValue [0] | | accessors_local.swift:91:20:91:29 | ... .0 | semmle.label | ... .0 | | accessors_local.swift:93:5:93:5 | [post] p [0] | semmle.label | [post] p [0] | @@ -615,12 +601,10 @@ nodes | accessors_static.swift:21:10:21:12 | ... .x | semmle.label | ... .x | | accessors_static.swift:27:19:27:32 | source(...) | semmle.label | source(...) | | accessors_static.swift:31:10:31:12 | ... .x | semmle.label | ... .x | -| accessors_static.swift:38:13:38:15 | newValue | semmle.label | newValue | | accessors_static.swift:38:24:38:31 | newValue | semmle.label | newValue | | accessors_static.swift:41:5:41:7 | ... .x | semmle.label | ... .x | | accessors_static.swift:41:11:41:24 | source(...) | semmle.label | source(...) | | accessors_static.swift:47:19:47:32 | source(...) | semmle.label | source(...) | -| accessors_static.swift:48:13:48:15 | newValue | semmle.label | newValue | | accessors_static.swift:48:24:48:31 | newValue | semmle.label | newValue | | accessors_static.swift:51:5:51:7 | ... .x | semmle.label | ... .x | | accessors_static.swift:51:11:51:13 | ... .x | semmle.label | ... .x | From 1bb712526efd3984c6a271a84d5b9e53c0002edb Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 29 Oct 2026 10:45:47 +0100 Subject: [PATCH 38/38] unified: Move mayWriteBackViaSetterCall into swift plugin This was accidentally put outside the plugin --- .../internal/dataflow/DataFlowPlugin.qll | 34 +------------------ .../internal/dataflow/DataFlowPluginSwift.qll | 34 ++++++++++++++++++- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll index b52a97816ddb..9b48861293e2 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPlugin.qll @@ -5,8 +5,6 @@ private import unified private import AllDataFlow private import codeql.util.Unit -private import codeql.unified.internal.ExprPositions -private import codeql.unified.internal.typeinference.TypeInference as T private module Plugins { private import DataFlowPluginSwift @@ -36,37 +34,7 @@ class DataFlowPlugin extends Unit { * `expr` is an expression in getter position, such as a member access. */ bindingset[expr] - predicate mayWriteBackViaSetterCall(Expr expr) { - isInWriteBackPosition(expr) - // TODO: Check that 'expr' has a value type. Reference types don't cause writebacks. - } -} - -/** - * Holds if `expr` is target of an assignment or 'inout' argument, - * or the base of such an expression, e.g. `x` in `x.f1.f2 = foo()` - */ -private predicate isInWriteBackPosition(Expr expr) { - isInAssignmentContext(expr, _) - or - // Arguments to an 'inout' parameter must be prefixed with '&' - exists(UnaryExpr unary | - unary.getOperator().(PrefixOperator).getValue() = "&" and - unary.getOperand() = expr - // TODO: support custom operators with an 'inout' parameter, which lack the '&' indicator - ) - or - // The receiver of a call to a 'mutating' function counts as an 'inout' argument - exists(CallExpr call, Callable target | - target = T::resolveCallTarget(call) and - target.hasModifier("mutating") and - expr = call.getCallee().(MemberAccessExpr).getBase() - ) - or - exists(MemberAccessExpr member | - isInWriteBackPosition(member) and - expr = member.getBase() - ) + predicate mayWriteBackViaSetterCall(Expr expr) { none() } } /** diff --git a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll index 7147a0a7101c..3d021a2f7aa5 100644 --- a/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll +++ b/unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll @@ -4,6 +4,8 @@ private import unified private import AllDataFlow +private import codeql.unified.internal.ExprPositions +private import codeql.unified.internal.typeinference.TypeInference as T private class SwiftDataFlowPlugin extends DataFlowPlugin { // Note: For now we assume all code is Swift, but in the future we must restrict these rules to Swift-files @@ -13,7 +15,10 @@ private class SwiftDataFlowPlugin extends DataFlowPlugin { // Swift writes back mutated values of computed properties via their setter bindingset[expr] - override predicate mayWriteBackViaSetterCall(Expr expr) { any() } + override predicate mayWriteBackViaSetterCall(Expr expr) { + isInWriteBackPosition(expr) + // TODO: Check that 'expr' has a value type. Reference types don't cause writebacks. + } override predicate exprStep(Expr expr1, Expr expr2, boolean mustFlow) { mustFlow = true and @@ -90,3 +95,30 @@ private class SwiftDataFlowPlugin extends DataFlowPlugin { ) } } + +/** + * Holds if `expr` is target of an assignment or 'inout' argument, + * or the base of such an expression, e.g. `x` in `x.f1.f2 = foo()` + */ +private predicate isInWriteBackPosition(Expr expr) { + isInAssignmentContext(expr, _) + or + // Arguments to an 'inout' parameter must be prefixed with '&' + exists(UnaryExpr unary | + unary.getOperator().(PrefixOperator).getValue() = "&" and + unary.getOperand() = expr + // TODO: support custom operators with an 'inout' parameter, which lack the '&' indicator + ) + or + // The receiver of a call to a 'mutating' function counts as an 'inout' argument + exists(CallExpr call, Callable target | + target = T::resolveCallTarget(call) and + target.hasModifier("mutating") and + expr = call.getCallee().(MemberAccessExpr).getBase() + ) + or + exists(MemberAccessExpr member | + isInWriteBackPosition(member) and + expr = member.getBase() + ) +}