Skip to content

Commit 14d2cd8

Browse files
committed
unified: Support short-hand returns
1 parent 1cce292 commit 14d2cd8

4 files changed

Lines changed: 32 additions & 1 deletion

File tree

‎unified/ql/lib/codeql/unified/internal/FacadeAst.qll‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,4 +437,14 @@ module Unified {
437437
/** Gets the number of elements in this array literal. */
438438
int getNumberOfElements() { result = count(this.getAnElement()) }
439439
}
440+
441+
/** Declaration of a getter accessor associated with a variable or field. */
442+
final class GetterDeclaration extends AccessorDeclaration {
443+
GetterDeclaration() { this.getAccessorKind().getValue() = "get" }
444+
}
445+
446+
/** Declaration of a setter accessor associated with a variable or field. */
447+
final class SetterDeclaration extends AccessorDeclaration {
448+
SetterDeclaration() { this.getAccessorKind().getValue() = "set" }
449+
}
440450
}

‎unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,22 @@ private class SwiftDataFlowPlugin extends DataFlowPlugin {
7676
step.taint() and
7777
node2.isResultValue(call)
7878
)
79+
or
80+
// Short-hand return from a function body, e.g. `func foo() -> String { "42" }`
81+
// This happens when the body contains a single expression and this expression resolves to a value.
82+
// We don't detect precisely whether the expression resolves to a value, but we restrict to cases
83+
// where a spurious flow edge would be harmless.
84+
exists(Callable callable, Block body |
85+
(
86+
callable instanceof FunctionExpr or
87+
callable instanceof FunctionDeclaration or
88+
callable instanceof GetterDeclaration
89+
) and
90+
body = callable.getBody() and
91+
not exists(body.getStmt(1)) and // Check that only one statement exists
92+
node1.isResultValue(body.getStmt(0)) and
93+
step.value() and
94+
node2.isReturnNode(callable)
95+
)
7996
}
8097
}

‎unified/ql/test/library-tests/dataflow/short-return.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ func sink(_ s: Any) -> String { return "" }
55

66
func t1() {
77
func getValue() -> String { source("t1.1") }
8-
sink(getValue()) // $ MISSING: hasValueFlow=t1.1
8+
sink(getValue()) // $ hasValueFlow=t1.1
99
}

‎unified/ql/test/library-tests/dataflow/test.expected‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ edges
290290
| implicit-self.swift:85:9:85:14 | [incoming] ... .x | implicit-self.swift:85:9:85:12 | [post] self [x] | provenance | |
291291
| implicit-self.swift:85:29:85:43 | source(...) | implicit-self.swift:85:9:85:14 | [incoming] ... .x | provenance | |
292292
| implicit-self.swift:87:14:87:17 | self [x] | implicit-self.swift:87:14:87:19 | ... .x | provenance | |
293+
| short-return.swift:7:33:7:46 | source(...) | short-return.swift:8:10:8:19 | getValue(...) | provenance | |
293294
| test.swift:17:10:17:23 | source(...) | test.swift:17:10:17:32 | ... + ... | provenance | |
294295
| test.swift:18:19:18:32 | source(...) | test.swift:18:10:18:32 | ... + ... | provenance | |
295296
| test.swift:20:13:20:26 | source(...) | test.swift:20:10:20:33 | StringInterpolationExpr | provenance | |
@@ -799,6 +800,8 @@ nodes
799800
| implicit-self.swift:86:14:86:14 | x | semmle.label | x |
800801
| implicit-self.swift:87:14:87:17 | self [x] | semmle.label | self [x] |
801802
| implicit-self.swift:87:14:87:19 | ... .x | semmle.label | ... .x |
803+
| short-return.swift:7:33:7:46 | source(...) | semmle.label | source(...) |
804+
| short-return.swift:8:10:8:19 | getValue(...) | semmle.label | getValue(...) |
802805
| test.swift:9:10:9:21 | source(...) | semmle.label | source(...) |
803806
| test.swift:17:10:17:23 | source(...) | semmle.label | source(...) |
804807
| test.swift:17:10:17:32 | ... + ... | semmle.label | ... + ... |
@@ -1103,6 +1106,7 @@ testFailures
11031106
| implicit-self.swift:80:14:80:19 | ... .x | implicit-self.swift:78:24:78:38 | source(...) | implicit-self.swift:80:14:80:19 | ... .x | $@ | implicit-self.swift:78:24:78:38 | source(...) | source(...) |
11041107
| implicit-self.swift:86:14:86:14 | x | implicit-self.swift:85:29:85:43 | source(...) | implicit-self.swift:86:14:86:14 | x | $@ | implicit-self.swift:85:29:85:43 | source(...) | source(...) |
11051108
| implicit-self.swift:87:14:87:19 | ... .x | implicit-self.swift:85:29:85:43 | source(...) | implicit-self.swift:87:14:87:19 | ... .x | $@ | implicit-self.swift:85:29:85:43 | source(...) | source(...) |
1109+
| short-return.swift:8:10:8:19 | getValue(...) | short-return.swift:7:33:7:46 | source(...) | short-return.swift:8:10:8:19 | getValue(...) | $@ | short-return.swift:7:33:7:46 | source(...) | source(...) |
11061110
| test.swift:9:10:9:21 | source(...) | test.swift:9:10:9:21 | source(...) | test.swift:9:10:9:21 | source(...) | $@ | test.swift:9:10:9:21 | source(...) | source(...) |
11071111
| test.swift:17:10:17:32 | ... + ... | test.swift:17:10:17:23 | source(...) | test.swift:17:10:17:32 | ... + ... | $@ | test.swift:17:10:17:23 | source(...) | source(...) |
11081112
| test.swift:18:10:18:32 | ... + ... | test.swift:18:19:18:32 | source(...) | test.swift:18:10:18:32 | ... + ... | $@ | test.swift:18:19:18:32 | source(...) | source(...) |

0 commit comments

Comments
 (0)