Skip to content

Commit b5dd815

Browse files
committed
Swift: Flow through optional binding.
1 parent c598d9b commit b5dd815

File tree

7 files changed

+51
-6
lines changed

7 files changed

+51
-6
lines changed

swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,14 @@ private module Cached {
153153
or
154154
nodeFrom.asExpr() = nodeTo.asExpr().(OptionalEvaluationExpr).getSubExpr()
155155
or
156+
// flow through optional binding `if let`
157+
exists(ConditionElement ce, ConcreteVarDecl v |
158+
ce.getInitializer() = nodeFrom.asExpr() and
159+
ce.getPattern() = v.getParentPattern() and
160+
//.(OptionalSomePattern).getSubPattern().(BindingPattern).getSubPattern().(NamedPattern) ?
161+
nodeTo.asDefinition().getSourceVariable() = v
162+
)
163+
or
156164
// flow through nil-coalescing operator `??`
157165
exists(BinaryExpr nco |
158166
nco.getOperator().(FreeFunctionDecl).getName() = "??(_:_:)" and

swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,21 @@ edges
107107
| test.swift:263:13:263:28 | call to optionalSource() : | test.swift:275:15:275:27 | ... ??(_:_:) ... |
108108
| test.swift:263:13:263:28 | call to optionalSource() : | test.swift:279:15:279:31 | ... ? ... : ... |
109109
| test.swift:263:13:263:28 | call to optionalSource() : | test.swift:280:15:280:38 | ... ? ... : ... |
110+
| test.swift:263:13:263:28 | call to optionalSource() : | test.swift:285:19:285:19 | z |
111+
| test.swift:263:13:263:28 | call to optionalSource() : | test.swift:291:16:291:17 | ...? : |
112+
| test.swift:263:13:263:28 | call to optionalSource() : | test.swift:300:15:300:15 | z1 |
110113
| test.swift:263:13:263:28 | call to optionalSource() : | test.swift:303:15:303:16 | ...! : |
114+
| test.swift:263:13:263:28 | call to optionalSource() : | test.swift:307:19:307:19 | z |
111115
| test.swift:270:15:270:22 | call to source() : | file://:0:0:0:0 | [summary param] this in signum() : |
112116
| test.swift:270:15:270:22 | call to source() : | test.swift:270:15:270:31 | call to signum() |
113117
| test.swift:271:15:271:16 | ...? : | file://:0:0:0:0 | [summary param] this in signum() : |
114118
| test.swift:271:15:271:16 | ...? : | test.swift:271:15:271:25 | call to signum() : |
115119
| test.swift:271:15:271:25 | call to signum() : | test.swift:271:15:271:25 | OptionalEvaluationExpr |
116120
| test.swift:280:31:280:38 | call to source() : | test.swift:280:15:280:38 | ... ? ... : ... |
117121
| test.swift:282:31:282:38 | call to source() : | test.swift:282:15:282:38 | ... ? ... : ... |
122+
| test.swift:291:16:291:17 | ...? : | file://:0:0:0:0 | [summary param] this in signum() : |
123+
| test.swift:291:16:291:17 | ...? : | test.swift:291:16:291:26 | call to signum() : |
124+
| test.swift:291:16:291:26 | call to signum() : | test.swift:292:19:292:19 | z |
118125
| test.swift:303:15:303:16 | ...! : | file://:0:0:0:0 | [summary param] this in signum() : |
119126
| test.swift:303:15:303:16 | ...! : | test.swift:303:15:303:25 | call to signum() |
120127
| test.swift:331:14:331:26 | (...) [Tuple element at index 1] : | test.swift:335:15:335:15 | t1 [Tuple element at index 1] : |
@@ -261,8 +268,14 @@ nodes
261268
| test.swift:280:31:280:38 | call to source() : | semmle.label | call to source() : |
262269
| test.swift:282:15:282:38 | ... ? ... : ... | semmle.label | ... ? ... : ... |
263270
| test.swift:282:31:282:38 | call to source() : | semmle.label | call to source() : |
271+
| test.swift:285:19:285:19 | z | semmle.label | z |
272+
| test.swift:291:16:291:17 | ...? : | semmle.label | ...? : |
273+
| test.swift:291:16:291:26 | call to signum() : | semmle.label | call to signum() : |
274+
| test.swift:292:19:292:19 | z | semmle.label | z |
275+
| test.swift:300:15:300:15 | z1 | semmle.label | z1 |
264276
| test.swift:303:15:303:16 | ...! : | semmle.label | ...! : |
265277
| test.swift:303:15:303:25 | call to signum() | semmle.label | call to signum() |
278+
| test.swift:307:19:307:19 | z | semmle.label | z |
266279
| test.swift:331:14:331:26 | (...) [Tuple element at index 1] : | semmle.label | (...) [Tuple element at index 1] : |
267280
| test.swift:331:18:331:25 | call to source() : | semmle.label | call to source() : |
268281
| test.swift:335:15:335:15 | t1 [Tuple element at index 1] : | semmle.label | t1 [Tuple element at index 1] : |
@@ -311,6 +324,7 @@ subpaths
311324
| test.swift:219:13:219:15 | .a [x] : | test.swift:163:7:163:7 | self [x] : | file://:0:0:0:0 | .x : | test.swift:219:13:219:17 | .x |
312325
| test.swift:270:15:270:22 | call to source() : | file://:0:0:0:0 | [summary param] this in signum() : | file://:0:0:0:0 | [summary] to write: return (return) in signum() : | test.swift:270:15:270:31 | call to signum() |
313326
| test.swift:271:15:271:16 | ...? : | file://:0:0:0:0 | [summary param] this in signum() : | file://:0:0:0:0 | [summary] to write: return (return) in signum() : | test.swift:271:15:271:25 | call to signum() : |
327+
| test.swift:291:16:291:17 | ...? : | file://:0:0:0:0 | [summary param] this in signum() : | file://:0:0:0:0 | [summary] to write: return (return) in signum() : | test.swift:291:16:291:26 | call to signum() : |
314328
| test.swift:303:15:303:16 | ...! : | file://:0:0:0:0 | [summary param] this in signum() : | file://:0:0:0:0 | [summary] to write: return (return) in signum() : | test.swift:303:15:303:25 | call to signum() |
315329
#select
316330
| test.swift:7:15:7:15 | t1 | test.swift:6:19:6:26 | call to source() : | test.swift:7:15:7:15 | t1 | result |
@@ -351,7 +365,11 @@ subpaths
351365
| test.swift:280:15:280:38 | ... ? ... : ... | test.swift:259:12:259:19 | call to source() : | test.swift:280:15:280:38 | ... ? ... : ... | result |
352366
| test.swift:280:15:280:38 | ... ? ... : ... | test.swift:280:31:280:38 | call to source() : | test.swift:280:15:280:38 | ... ? ... : ... | result |
353367
| test.swift:282:15:282:38 | ... ? ... : ... | test.swift:282:31:282:38 | call to source() : | test.swift:282:15:282:38 | ... ? ... : ... | result |
368+
| test.swift:285:19:285:19 | z | test.swift:259:12:259:19 | call to source() : | test.swift:285:19:285:19 | z | result |
369+
| test.swift:292:19:292:19 | z | test.swift:259:12:259:19 | call to source() : | test.swift:292:19:292:19 | z | result |
370+
| test.swift:300:15:300:15 | z1 | test.swift:259:12:259:19 | call to source() : | test.swift:300:15:300:15 | z1 | result |
354371
| test.swift:303:15:303:25 | call to signum() | test.swift:259:12:259:19 | call to source() : | test.swift:303:15:303:25 | call to signum() | result |
372+
| test.swift:307:19:307:19 | z | test.swift:259:12:259:19 | call to source() : | test.swift:307:19:307:19 | z | result |
355373
| test.swift:335:15:335:18 | .1 | test.swift:331:18:331:25 | call to source() : | test.swift:335:15:335:18 | .1 | result |
356374
| test.swift:346:15:346:18 | .0 | test.swift:343:12:343:19 | call to source() : | test.swift:346:15:346:18 | .0 | result |
357375
| test.swift:356:15:356:18 | .0 | test.swift:351:18:351:25 | call to source() : | test.swift:356:15:356:18 | .0 | result |

swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,28 +242,36 @@
242242
| test.swift:282:26:282:27 | ...! | test.swift:282:15:282:38 | ... ? ... : ... |
243243
| test.swift:282:31:282:38 | call to source() | test.swift:282:15:282:38 | ... ? ... : ... |
244244
| test.swift:284:8:284:12 | SSA def(z) | test.swift:285:19:285:19 | z |
245+
| test.swift:284:16:284:16 | x | test.swift:284:8:284:12 | SSA def(z) |
245246
| test.swift:284:16:284:16 | x | test.swift:291:16:291:16 | x |
246247
| test.swift:287:8:287:12 | SSA def(z) | test.swift:288:19:288:19 | z |
248+
| test.swift:287:16:287:16 | y | test.swift:287:8:287:12 | SSA def(z) |
247249
| test.swift:287:16:287:16 | y | test.swift:294:16:294:16 | y |
248250
| test.swift:291:8:291:12 | SSA def(z) | test.swift:292:19:292:19 | z |
249251
| test.swift:291:16:291:16 | x | test.swift:291:16:291:17 | ...? |
250252
| test.swift:291:16:291:16 | x | test.swift:298:20:298:20 | x |
253+
| test.swift:291:16:291:26 | OptionalEvaluationExpr | test.swift:291:8:291:12 | SSA def(z) |
251254
| test.swift:291:16:291:26 | call to signum() | test.swift:291:16:291:26 | OptionalEvaluationExpr |
252255
| test.swift:294:8:294:12 | SSA def(z) | test.swift:295:19:295:19 | z |
253256
| test.swift:294:16:294:16 | y | test.swift:294:16:294:17 | ...? |
254257
| test.swift:294:16:294:16 | y | test.swift:299:20:299:20 | y |
258+
| test.swift:294:16:294:26 | OptionalEvaluationExpr | test.swift:294:8:294:12 | SSA def(z) |
255259
| test.swift:294:16:294:26 | call to signum() | test.swift:294:16:294:26 | OptionalEvaluationExpr |
256260
| test.swift:298:11:298:15 | SSA def(z1) | test.swift:300:15:300:15 | z1 |
261+
| test.swift:298:20:298:20 | x | test.swift:298:11:298:15 | SSA def(z1) |
257262
| test.swift:298:20:298:20 | x | test.swift:303:15:303:15 | x |
258263
| test.swift:299:11:299:15 | SSA def(z2) | test.swift:301:15:301:15 | z2 |
264+
| test.swift:299:20:299:20 | y | test.swift:299:11:299:15 | SSA def(z2) |
259265
| test.swift:299:20:299:20 | y | test.swift:304:15:304:15 | y |
260266
| test.swift:303:15:303:15 | x | test.swift:303:15:303:16 | ...! |
261267
| test.swift:303:15:303:15 | x | test.swift:306:28:306:28 | x |
262268
| test.swift:304:15:304:15 | y | test.swift:304:15:304:16 | ...! |
263269
| test.swift:304:15:304:15 | y | test.swift:309:28:309:28 | y |
264270
| test.swift:306:13:306:24 | SSA def(z) | test.swift:307:19:307:19 | z |
271+
| test.swift:306:28:306:28 | x | test.swift:306:13:306:24 | SSA def(z) |
265272
| test.swift:306:28:306:28 | x | test.swift:313:12:313:12 | x |
266273
| test.swift:309:13:309:24 | SSA def(z) | test.swift:310:19:310:19 | z |
274+
| test.swift:309:28:309:28 | y | test.swift:309:13:309:24 | SSA def(z) |
267275
| test.swift:309:28:309:28 | y | test.swift:319:12:319:12 | y |
268276
| test.swift:314:10:314:21 | SSA def(z) | test.swift:315:19:315:19 | z |
269277
| test.swift:320:10:320:21 | SSA def(z) | test.swift:321:19:321:19 | z |

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,29 +282,29 @@ func test_optionals(y: Int?) {
282282
sink(arg: y != nil ? y! : source()) // $ flow=282
283283

284284
if let z = x {
285-
sink(arg: z) // $ MISSING: flow=259
285+
sink(arg: z) // $ flow=259
286286
}
287287
if let z = y {
288288
sink(arg: z)
289289
}
290290

291-
if let z = x?.signum() { // $ MISSING: flow=259
292-
sink(arg: z)
291+
if let z = x?.signum() {
292+
sink(arg: z) // $ flow=259
293293
}
294294
if let z = y?.signum() {
295295
sink(arg: z)
296296
}
297297

298298
guard let z1 = x else { return }
299299
guard let z2 = y else { return }
300-
sink(arg: z1) // $ MISSING: flow=259
300+
sink(arg: z1) // $ flow=259
301301
sink(arg: z2)
302302

303303
sink(arg: x!.signum()) // $ flow=259
304304
sink(arg: y!.signum())
305305

306306
if case .some(let z) = x {
307-
sink(arg: z) // $ MISSING: flow=259
307+
sink(arg: z) // $ flow=259
308308
}
309309
if case .some(let z) = y {
310310
sink(arg: z)

swift/ql/test/library-tests/dataflow/taint/LocalTaint.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,10 +1277,12 @@
12771277
| url.swift:102:46:102:46 | urlTainted | url.swift:102:15:102:56 | call to URL.init(string:relativeTo:) |
12781278
| url.swift:102:46:102:46 | urlTainted | url.swift:120:46:120:46 | urlTainted |
12791279
| url.swift:104:5:104:9 | SSA def(x) | url.swift:105:13:105:13 | x |
1280+
| url.swift:104:13:104:30 | call to URL.init(string:) | url.swift:104:5:104:9 | SSA def(x) |
12801281
| url.swift:104:25:104:25 | [post] clean | url.swift:113:26:113:26 | clean |
12811282
| url.swift:104:25:104:25 | clean | url.swift:104:13:104:30 | call to URL.init(string:) |
12821283
| url.swift:104:25:104:25 | clean | url.swift:113:26:113:26 | clean |
12831284
| url.swift:108:5:108:9 | SSA def(y) | url.swift:109:13:109:13 | y |
1285+
| url.swift:108:13:108:32 | call to URL.init(string:) | url.swift:108:5:108:9 | SSA def(y) |
12841286
| url.swift:108:25:108:25 | [post] tainted | url.swift:117:28:117:28 | tainted |
12851287
| url.swift:108:25:108:25 | tainted | url.swift:108:13:108:32 | call to URL.init(string:) |
12861288
| url.swift:108:25:108:25 | tainted | url.swift:117:28:117:28 | tainted |

swift/ql/test/library-tests/dataflow/taint/Taint.expected

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ edges
326326
| url.swift:43:2:46:55 | [summary param] 0 in dataTask(with:completionHandler:) : | file://:0:0:0:0 | [summary] to write: argument 1.parameter 0 in dataTask(with:completionHandler:) : |
327327
| url.swift:57:16:57:23 | call to source() : | url.swift:59:31:59:31 | tainted : |
328328
| url.swift:57:16:57:23 | call to source() : | url.swift:83:24:83:24 | tainted : |
329+
| url.swift:57:16:57:23 | call to source() : | url.swift:108:25:108:25 | tainted : |
329330
| url.swift:57:16:57:23 | call to source() : | url.swift:117:28:117:28 | tainted : |
330331
| url.swift:59:19:59:38 | call to URL.init(string:) : | url.swift:62:12:62:12 | urlTainted |
331332
| url.swift:59:19:59:38 | call to URL.init(string:) : | url.swift:64:12:64:23 | .absoluteURL |
@@ -419,6 +420,9 @@ edges
419420
| url.swift:102:15:102:56 | call to URL.init(string:relativeTo:) : | url.swift:102:15:102:67 | ...! |
420421
| url.swift:102:46:102:46 | urlTainted : | url.swift:9:2:9:43 | [summary param] 1 in URL.init(string:relativeTo:) : |
421422
| url.swift:102:46:102:46 | urlTainted : | url.swift:102:15:102:56 | call to URL.init(string:relativeTo:) : |
423+
| url.swift:108:13:108:32 | call to URL.init(string:) : | url.swift:109:13:109:13 | y |
424+
| url.swift:108:25:108:25 | tainted : | url.swift:8:2:8:25 | [summary param] 0 in URL.init(string:) : |
425+
| url.swift:108:25:108:25 | tainted : | url.swift:108:13:108:32 | call to URL.init(string:) : |
422426
| url.swift:117:16:117:35 | call to URL.init(string:) : | url.swift:118:12:118:12 | ...! |
423427
| url.swift:117:28:117:28 | tainted : | url.swift:8:2:8:25 | [summary param] 0 in URL.init(string:) : |
424428
| url.swift:117:28:117:28 | tainted : | url.swift:117:16:117:35 | call to URL.init(string:) : |
@@ -1061,6 +1065,9 @@ nodes
10611065
| url.swift:102:15:102:56 | call to URL.init(string:relativeTo:) : | semmle.label | call to URL.init(string:relativeTo:) : |
10621066
| url.swift:102:15:102:67 | ...! | semmle.label | ...! |
10631067
| url.swift:102:46:102:46 | urlTainted : | semmle.label | urlTainted : |
1068+
| url.swift:108:13:108:32 | call to URL.init(string:) : | semmle.label | call to URL.init(string:) : |
1069+
| url.swift:108:25:108:25 | tainted : | semmle.label | tainted : |
1070+
| url.swift:109:13:109:13 | y | semmle.label | y |
10641071
| url.swift:117:16:117:35 | call to URL.init(string:) : | semmle.label | call to URL.init(string:) : |
10651072
| url.swift:117:28:117:28 | tainted : | semmle.label | tainted : |
10661073
| url.swift:118:12:118:12 | ...! | semmle.label | ...! |
@@ -1261,6 +1268,7 @@ subpaths
12611268
| url.swift:100:43:100:43 | urlTainted : | url.swift:9:2:9:43 | [summary param] 1 in URL.init(string:relativeTo:) : | file://:0:0:0:0 | [summary] to write: return (return) in URL.init(string:relativeTo:) : | url.swift:100:12:100:53 | call to URL.init(string:relativeTo:) : |
12621269
| url.swift:101:46:101:46 | urlTainted : | url.swift:9:2:9:43 | [summary param] 1 in URL.init(string:relativeTo:) : | file://:0:0:0:0 | [summary] to write: return (return) in URL.init(string:relativeTo:) : | url.swift:101:15:101:56 | call to URL.init(string:relativeTo:) : |
12631270
| url.swift:102:46:102:46 | urlTainted : | url.swift:9:2:9:43 | [summary param] 1 in URL.init(string:relativeTo:) : | file://:0:0:0:0 | [summary] to write: return (return) in URL.init(string:relativeTo:) : | url.swift:102:15:102:56 | call to URL.init(string:relativeTo:) : |
1271+
| url.swift:108:25:108:25 | tainted : | url.swift:8:2:8:25 | [summary param] 0 in URL.init(string:) : | file://:0:0:0:0 | [summary] to write: return (return) in URL.init(string:) : | url.swift:108:13:108:32 | call to URL.init(string:) : |
12641272
| url.swift:117:28:117:28 | tainted : | url.swift:8:2:8:25 | [summary param] 0 in URL.init(string:) : | file://:0:0:0:0 | [summary] to write: return (return) in URL.init(string:) : | url.swift:117:16:117:35 | call to URL.init(string:) : |
12651273
| webview.swift:84:10:84:10 | source : | webview.swift:36:5:36:41 | [summary param] this in toObject() : | file://:0:0:0:0 | [summary] to write: return (return) in toObject() : | webview.swift:84:10:84:26 | call to toObject() |
12661274
| webview.swift:85:10:85:10 | source : | webview.swift:37:5:37:55 | [summary param] this in toObjectOf(_:) : | file://:0:0:0:0 | [summary] to write: return (return) in toObjectOf(_:) : | webview.swift:85:10:85:41 | call to toObjectOf(_:) |
@@ -1422,6 +1430,7 @@ subpaths
14221430
| url.swift:100:12:100:56 | .standardizedFileURL | url.swift:57:16:57:23 | call to source() : | url.swift:100:12:100:56 | .standardizedFileURL | result |
14231431
| url.swift:101:15:101:63 | ...! | url.swift:57:16:57:23 | call to source() : | url.swift:101:15:101:63 | ...! | result |
14241432
| url.swift:102:15:102:67 | ...! | url.swift:57:16:57:23 | call to source() : | url.swift:102:15:102:67 | ...! | result |
1433+
| url.swift:109:13:109:13 | y | url.swift:57:16:57:23 | call to source() : | url.swift:109:13:109:13 | y | result |
14251434
| url.swift:118:12:118:12 | ...! | url.swift:57:16:57:23 | call to source() : | url.swift:118:12:118:12 | ...! | result |
14261435
| url.swift:121:15:121:19 | ...! | url.swift:57:16:57:23 | call to source() : | url.swift:121:15:121:19 | ...! | result |
14271436
| webview.swift:77:10:77:41 | .body | webview.swift:77:11:77:18 | call to source() : | webview.swift:77:10:77:41 | .body | result |

swift/ql/test/library-tests/dataflow/taint/url.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func taintThroughURL() {
106106
}
107107

108108
if let y = URL(string: tainted) {
109-
sink(arg: y) // $ MISSING: tainted=57
109+
sink(arg: y) // $ tainted=57
110110
}
111111

112112
var urlClean2 : URL!

0 commit comments

Comments
 (0)