diff --git a/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll b/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll index 9db28d702497..7f88d1c496bf 100644 --- a/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll +++ b/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll @@ -480,6 +480,8 @@ module Public { } } +final class PotentialLocalNameAccess = PotentialLocalNameAccessImpl; + /** * A name node that is possibly a reference to a local name, but could also refer to a member * visible through imports or inheritance. @@ -493,17 +495,27 @@ module Public { * } * ``` */ -class PotentialLocalNameAccess extends IdentifierExpr { +abstract private class PotentialLocalNameAccessImpl extends AstNode { /** Gets the representative for the local name being accessed. */ LocalName getLocalName() { result = this.(LocalNameBindingOutput::LocalAccess).getLocal() } /** Gets the name being accessed. */ - string getName() { result = this.getValue() } + abstract string getName(); /** Holds if this is one of the binding sites for a name, such as the `x` in `let x = 123`. */ predicate isBindingSite() { this instanceof NameBinding } } +private class IdentifierPotentialLocalNameAccess extends PotentialLocalNameAccessImpl, + IdentifierExpr +{ + override string getName() { result = this.getValue() } +} + +private class SuperPotentialLocalNameAccess extends PotentialLocalNameAccessImpl, SuperExpr { + override string getName() { result = any(NameBindingPlugin p).getSuperLocalName(this) } +} + /** Gets the implicitly-declared variable through which the given callable refers to its receiver. */ LocalVariable getImplicitReceiverVariable(Callable callable) { exists(string name | diff --git a/unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll b/unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll index 067b60eea683..3c290ef637c0 100644 --- a/unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll +++ b/unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll @@ -68,6 +68,12 @@ class NameBindingPlugin extends Unit { /** Gets the name of the implicit receiver parameter in `callable`, if it has one. */ string getImplicitReceiverParameterName(Callable callable) { none() } + /** + * Gets the name to be used when resolving `super`, for example `this` or `self`. + */ + bindingset[se] + string getSuperLocalName(SuperExpr se) { none() } + /** * Gets the name through which static members of the enclosing class `cls` can be * accessed, for example `Self` in Swift. diff --git a/unified/ql/lib/codeql/unified/internal/NameBindingPluginSwift.qll b/unified/ql/lib/codeql/unified/internal/NameBindingPluginSwift.qll index 846bb2ba8c67..2e439943a1e1 100644 --- a/unified/ql/lib/codeql/unified/internal/NameBindingPluginSwift.qll +++ b/unified/ql/lib/codeql/unified/internal/NameBindingPluginSwift.qll @@ -107,6 +107,9 @@ class NameBindingPluginSwift extends NameBindingPlugin { result = "self" } + bindingset[se] + override string getSuperLocalName(SuperExpr se) { exists(se) and result = "self" } + bindingset[cls] override string getStaticSelfName(ClassLikeDeclaration cls) { exists(cls) and result = "Self" } } diff --git a/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll b/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll index 2dd7596e29cc..34b6c34dc47f 100644 --- a/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll +++ b/unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll @@ -219,7 +219,11 @@ module Input3 implements InputSig3 { } class Variable extends LocalNameBindingOutput::Local { - Expr getAnAccess() { result = super.getAnAccess() } + Expr getAnAccess() { + result = super.getAnAccess() and + // for type inference, we want super expressions to have the type of the base class + not result instanceof SuperExpr + } } final class VariableDeclaration = VariableDeclarationImpl; diff --git a/unified/ql/test/library-tests/dataflow/accessors_instance.swift b/unified/ql/test/library-tests/dataflow/accessors_instance.swift index 6c76138c5506..d81d54dd0d9c 100644 --- a/unified/ql/test/library-tests/dataflow/accessors_instance.swift +++ b/unified/ql/test/library-tests/dataflow/accessors_instance.swift @@ -159,3 +159,21 @@ func t12() { let c = C() c.s.update() } + +func t13() { + class C1 { + var x: String = "safe" + } + class C2: C1 { + func updateX(newValue: String) { + super.x = newValue + } + + func getX() -> String { + return super.x + } + } + var c2 = C2() + c2.updateX(newValue: source("t13.1")) + sink(c2.getX()) // $ hasValueFlow=t13.1 +} diff --git a/unified/ql/test/library-tests/dataflow/test.expected b/unified/ql/test/library-tests/dataflow/test.expected index 40fb146a8fde..a479255f6d23 100644 --- a/unified/ql/test/library-tests/dataflow/test.expected +++ b/unified/ql/test/library-tests/dataflow/test.expected @@ -42,6 +42,14 @@ edges | accessors_instance.swift:151:42:151:56 | source(...) | accessors_instance.swift:151:34:151:38 | field | provenance | | | accessors_instance.swift:156:24:156:31 | newValue [field] | accessors_instance.swift:156:24:156:37 | ... .field | provenance | | | accessors_instance.swift:160:5:160:7 | [post] ... .s [field] | accessors_instance.swift:156:24:156:31 | newValue [field] | provenance | | +| accessors_instance.swift:168:22:168:29 | newValue | accessors_instance.swift:169:23:169:30 | newValue | provenance | | +| accessors_instance.swift:169:13:169:19 | ... .x | accessors_instance.swift:169:13:169:17 | [post] super [x] | provenance | | +| accessors_instance.swift:169:23:169:30 | newValue | accessors_instance.swift:169:13:169:19 | ... .x | provenance | | +| accessors_instance.swift:173:20:173:24 | super [x] | accessors_instance.swift:173:20:173:26 | ... .x | provenance | | +| accessors_instance.swift:177:26:177:40 | source(...) | accessors_instance.swift:168:22:168:29 | newValue | provenance | | +| accessors_instance.swift:177:26:177:40 | source(...) | accessors_instance.swift:178:10:178:11 | c2 [x] | provenance | | +| accessors_instance.swift:178:10:178:11 | c2 [x] | accessors_instance.swift:173:20:173:24 | super [x] | provenance | | +| accessors_instance.swift:178:10:178:11 | c2 [x] | accessors_instance.swift:178:10:178:18 | ... .getX(...) | 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 | | @@ -580,6 +588,15 @@ nodes | accessors_instance.swift:156:24:156:31 | newValue [field] | semmle.label | newValue [field] | | accessors_instance.swift:156:24:156:37 | ... .field | semmle.label | ... .field | | accessors_instance.swift:160:5:160:7 | [post] ... .s [field] | semmle.label | [post] ... .s [field] | +| accessors_instance.swift:168:22:168:29 | newValue | semmle.label | newValue | +| accessors_instance.swift:169:13:169:17 | [post] super [x] | semmle.label | [post] super [x] | +| accessors_instance.swift:169:13:169:19 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:169:23:169:30 | newValue | semmle.label | newValue | +| accessors_instance.swift:173:20:173:24 | super [x] | semmle.label | super [x] | +| accessors_instance.swift:173:20:173:26 | ... .x | semmle.label | ... .x | +| accessors_instance.swift:177:26:177:40 | source(...) | semmle.label | source(...) | +| accessors_instance.swift:178:10:178:11 | c2 [x] | semmle.label | c2 [x] | +| accessors_instance.swift:178:10:178:18 | ... .getX(...) | semmle.label | ... .getX(...) | | 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(...) | @@ -1183,6 +1200,8 @@ nodes subpaths | 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_instance.swift:177:26:177:40 | source(...) | accessors_instance.swift:168:22:168:29 | newValue | accessors_instance.swift:169:13:169:17 | [post] super [x] | accessors_instance.swift:178:10:178:11 | c2 [x] | +| accessors_instance.swift:178:10:178:11 | c2 [x] | accessors_instance.swift:173:20:173:24 | super [x] | accessors_instance.swift:173:20:173:26 | ... .x | accessors_instance.swift:178:10:178:18 | ... .getX(...) | | 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(...) | @@ -1218,6 +1237,7 @@ testFailures | accessors_instance.swift:134:24:134:33 | ... .0 | accessors_instance.swift:143:13:143:27 | source(...) | accessors_instance.swift:134:24:134:33 | ... .0 | $@ | accessors_instance.swift:143:13:143:27 | source(...) | source(...) | | accessors_instance.swift:134:24:134:33 | ... .0 | accessors_instance.swift:145:14:145:28 | source(...) | accessors_instance.swift:134:24:134:33 | ... .0 | $@ | accessors_instance.swift:145:14:145:28 | source(...) | source(...) | | accessors_instance.swift:156:24:156:37 | ... .field | accessors_instance.swift:151:42:151:56 | source(...) | accessors_instance.swift:156:24:156:37 | ... .field | $@ | accessors_instance.swift:151:42:151:56 | source(...) | source(...) | +| accessors_instance.swift:178:10:178:18 | ... .getX(...) | accessors_instance.swift:177:26:177:40 | source(...) | accessors_instance.swift:178:10:178:18 | ... .getX(...) | $@ | accessors_instance.swift:177:26:177:40 | 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(...) | diff --git a/unified/ql/test/library-tests/local-name-binding/self_access.swift b/unified/ql/test/library-tests/local-name-binding/self_access.swift index da1132f41913..e3050195e4ce 100644 --- a/unified/ql/test/library-tests/local-name-binding/self_access.swift +++ b/unified/ql/test/library-tests/local-name-binding/self_access.swift @@ -38,3 +38,9 @@ class C { } } } + +class Sub: C { // $ access=C + func t6() { // implicit-self=t6.self + super.t1() // $ access=t6.self (we treat `super` as an alias for `self`) + } +}