Skip to content

Commit 93d641f

Browse files
committed
unified: Handle flow through +=
1 parent c5acdaa commit 93d641f

4 files changed

Lines changed: 25 additions & 3 deletions

File tree

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ predicate step(Node node1, Step step, Node node2) {
1616
node2.isIncomingValue(assign.getTarget())
1717
)
1818
or
19+
// For compound assignments, the result of the expression represents the result of the operator.
20+
// Make it flow to the target of the assignment.
21+
exists(CompoundAssignExpr assign |
22+
node1.isResultValue(assign) and
23+
step.value() and
24+
node2.isIncomingValue(assign.getTarget())
25+
)
26+
or
1927
exists(LocalVariableAccess access |
2028
node1.isLocalVariableRead(access, access.getLocalVariable()) and
2129
step.value() and

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ private class SwiftDataFlowPlugin extends DataFlowPlugin {
99
// Note: For now we assume all code is Swift, but in the future we must restrict these rules to Swift-files
1010
override predicate step(Node node1, Step step, Node node2) {
1111
exists(BinaryExpr expr |
12-
expr.getOperator().getValue() = "+" and
12+
expr.getOperator().getValue() = ["+", "+="] and
1313
node1.isResultValue([expr.getLeft(), expr.getRight()]) and
1414
step.taint() and
1515
node2.isResultValue(expr)

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ edges
9797
| test.swift:120:17:120:21 | tuple [1] | test.swift:120:9:120:13 | TupleExpr [1] | provenance | |
9898
| test.swift:131:5:131:5 | a | test.swift:132:10:132:10 | a | provenance | |
9999
| test.swift:131:19:131:33 | CallExpr | test.swift:131:5:131:5 | a | provenance | |
100+
| test.swift:137:5:137:5 | [incoming] a | test.swift:138:10:138:10 | a | provenance | |
101+
| test.swift:137:10:137:24 | CallExpr | test.swift:137:5:137:5 | [incoming] a | provenance | |
102+
| test.swift:143:5:143:5 | [incoming] a | test.swift:144:10:144:10 | a | provenance | |
103+
| test.swift:143:20:143:34 | CallExpr | test.swift:143:5:143:5 | [incoming] a | provenance | |
100104
nodes
101105
| implicit-self.swift:11:9:11:12 | [post] self [x] | semmle.label | [post] self [x] |
102106
| implicit-self.swift:11:9:11:14 | MemberAccessExpr | semmle.label | MemberAccessExpr |
@@ -225,8 +229,16 @@ nodes
225229
| test.swift:131:5:131:5 | a | semmle.label | a |
226230
| test.swift:131:19:131:33 | CallExpr | semmle.label | CallExpr |
227231
| test.swift:132:10:132:10 | a | semmle.label | a |
232+
| test.swift:137:5:137:5 | [incoming] a | semmle.label | [incoming] a |
233+
| test.swift:137:10:137:24 | CallExpr | semmle.label | CallExpr |
234+
| test.swift:138:10:138:10 | a | semmle.label | a |
235+
| test.swift:143:5:143:5 | [incoming] a | semmle.label | [incoming] a |
236+
| test.swift:143:20:143:34 | CallExpr | semmle.label | CallExpr |
237+
| test.swift:144:10:144:10 | a | semmle.label | a |
228238
subpaths
229239
testFailures
240+
| test.swift:138:10:138:10 | a | Fixed missing result: hasTaintFlow=t15.1 |
241+
| test.swift:144:10:144:10 | a | Fixed missing result: hasTaintFlow=t16.1 |
230242
#select
231243
| implicit-self.swift:12:14:12:19 | MemberAccessExpr | implicit-self.swift:11:18:11:31 | CallExpr | implicit-self.swift:12:14:12:19 | MemberAccessExpr | $@ | implicit-self.swift:11:18:11:31 | CallExpr | CallExpr |
232244
| implicit-self.swift:18:14:18:14 | x | implicit-self.swift:17:13:17:26 | CallExpr | implicit-self.swift:18:14:18:14 | x | $@ | implicit-self.swift:17:13:17:26 | CallExpr | CallExpr |
@@ -260,3 +272,5 @@ testFailures
260272
| test.swift:125:10:125:10 | a | test.swift:117:18:117:33 | CallExpr | test.swift:125:10:125:10 | a | $@ | test.swift:117:18:117:33 | CallExpr | CallExpr |
261273
| test.swift:126:10:126:10 | b | test.swift:117:35:117:49 | CallExpr | test.swift:126:10:126:10 | b | $@ | test.swift:117:35:117:49 | CallExpr | CallExpr |
262274
| test.swift:132:10:132:10 | a | test.swift:131:19:131:33 | CallExpr | test.swift:132:10:132:10 | a | $@ | test.swift:131:19:131:33 | CallExpr | CallExpr |
275+
| test.swift:138:10:138:10 | a | test.swift:137:10:137:24 | CallExpr | test.swift:138:10:138:10 | a | $@ | test.swift:137:10:137:24 | CallExpr | CallExpr |
276+
| test.swift:144:10:144:10 | a | test.swift:143:20:143:34 | CallExpr | test.swift:144:10:144:10 | a | $@ | test.swift:143:20:143:34 | CallExpr | CallExpr |

unified/ql/test/library-tests/dataflow/test.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,11 @@ func t14() {
135135
func t15() {
136136
var a = "safe";
137137
a += source("t15.1");
138-
sink(a); // $ MISSING: hasTaintFlow=t15.1
138+
sink(a); // $ hasTaintFlow=t15.1
139139
}
140140

141141
func t16() {
142142
var a = "safe";
143143
a += sink(a) + source("t16.1");
144-
sink(a); // $ MISSING: hasTaintFlow=t16.1
144+
sink(a); // $ hasTaintFlow=t16.1
145145
}

0 commit comments

Comments
 (0)