Skip to content

Commit 206acea

Browse files
geoffw0geoffw0
committed
Swift: Fix defaultImplicitTaintRead for sinks that are field accesses on a subclass of the type containing the field.
1 parent 554007b commit 206acea

File tree

3 files changed

+57
-7
lines changed

3 files changed

+57
-7
lines changed

‎swift/ql/lib/codeql/swift/dataflow/internal/TaintTrackingPublic.qll

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,12 @@ predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet cs)
3232
// So when the node is a `PostUpdateNode` we allow any sequence of implicit read steps of an appropriate
3333
// type to make sure we arrive at the sink with an empty access path.
3434
exists(NominalTypeDecl d, Decl cx |
35-
node.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr().getType().getUnderlyingType() =
36-
d.getType().getABaseType*() and
35+
node.(DataFlow::PostUpdateNode)
36+
.getPreUpdateNode()
37+
.asExpr()
38+
.getType()
39+
.getUnderlyingType()
40+
.getABaseType*() = d.getType() and
3741
cx.asNominalTypeDecl() = d and
3842
cs.getAReadContent().(DataFlow::Content::FieldContent).getField() = cx.getAMember()
3943
)

‎swift/ql/test/query-tests/Security/CWE-078/CommandInjection.expected

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ edges
2929
| CommandInjection.swift:81:6:81:6 | [post] task2 [arguments, Collection element] | CommandInjection.swift:81:6:81:6 | [post] task2 |
3030
| CommandInjection.swift:81:24:81:46 | [...] [Collection element] | CommandInjection.swift:81:6:81:6 | [post] task2 [arguments, Collection element] |
3131
| CommandInjection.swift:81:31:81:31 | validatedString | CommandInjection.swift:81:24:81:46 | [...] [Collection element] |
32+
| CommandInjection.swift:93:20:93:40 | arguments [Collection element] | CommandInjection.swift:94:20:94:20 | arguments [Collection element] |
33+
| CommandInjection.swift:94:3:94:3 | [post] self [arguments, Collection element] | CommandInjection.swift:94:3:94:3 | [post] self |
34+
| CommandInjection.swift:94:20:94:20 | arguments [Collection element] | CommandInjection.swift:94:3:94:3 | [post] self [arguments, Collection element] |
3235
| CommandInjection.swift:99:8:99:12 | let ...? [some:0] | CommandInjection.swift:99:12:99:12 | userControlledString |
3336
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:114:36:114:36 | userControlledString |
3437
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:115:28:115:28 | userControlledString |
@@ -37,6 +40,10 @@ edges
3740
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:121:28:121:36 | ... .+(_:_:) ... |
3841
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:125:46:125:46 | userControlledString |
3942
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:126:22:126:22 | userControlledString |
43+
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:130:45:130:45 | userControlledString |
44+
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:131:36:131:36 | userControlledString |
45+
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:132:21:132:21 | userControlledString |
46+
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:133:22:133:22 | userControlledString |
4047
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:134:24:134:24 | userControlledString |
4148
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:144:42:144:42 | userControlledString |
4249
| CommandInjection.swift:99:12:99:12 | userControlledString | CommandInjection.swift:145:75:145:75 | userControlledString |
@@ -54,6 +61,10 @@ edges
5461
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:121:28:121:36 | ... .+(_:_:) ... |
5562
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:125:46:125:46 | userControlledString |
5663
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:126:22:126:22 | userControlledString |
64+
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:130:45:130:45 | userControlledString |
65+
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:131:36:131:36 | userControlledString |
66+
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:132:21:132:21 | userControlledString |
67+
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:133:22:133:22 | userControlledString |
5768
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:134:24:134:24 | userControlledString |
5869
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:144:42:144:42 | userControlledString |
5970
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:145:75:145:75 | userControlledString |
@@ -87,6 +98,18 @@ edges
8798
| CommandInjection.swift:126:2:126:7 | [post] ...? [arguments, Collection element] | CommandInjection.swift:126:2:126:7 | [post] ...? |
8899
| CommandInjection.swift:126:21:126:42 | [...] [Collection element] | CommandInjection.swift:126:2:126:7 | [post] ...? [arguments, Collection element] |
89100
| CommandInjection.swift:126:22:126:22 | userControlledString | CommandInjection.swift:126:21:126:42 | [...] [Collection element] |
101+
| CommandInjection.swift:130:2:130:2 | [post] task6 [executableURL] | CommandInjection.swift:130:2:130:2 | [post] task6 |
102+
| CommandInjection.swift:130:24:130:65 | call to URL.init(fileURLWithPath:) | CommandInjection.swift:130:2:130:2 | [post] task6 [executableURL] |
103+
| CommandInjection.swift:130:45:130:45 | userControlledString | CommandInjection.swift:130:24:130:65 | call to URL.init(fileURLWithPath:) |
104+
| CommandInjection.swift:131:2:131:2 | [post] task6 [executableURL] | CommandInjection.swift:131:2:131:2 | [post] task6 |
105+
| CommandInjection.swift:131:24:131:56 | call to URL.init(string:) [some:0] | CommandInjection.swift:131:24:131:57 | ...! |
106+
| CommandInjection.swift:131:24:131:57 | ...! | CommandInjection.swift:131:2:131:2 | [post] task6 [executableURL] |
107+
| CommandInjection.swift:131:36:131:36 | userControlledString | CommandInjection.swift:131:24:131:56 | call to URL.init(string:) [some:0] |
108+
| CommandInjection.swift:132:2:132:2 | [post] task6 [arguments, Collection element] | CommandInjection.swift:132:2:132:2 | [post] task6 |
109+
| CommandInjection.swift:132:20:132:41 | [...] [Collection element] | CommandInjection.swift:132:2:132:2 | [post] task6 [arguments, Collection element] |
110+
| CommandInjection.swift:132:21:132:21 | userControlledString | CommandInjection.swift:132:20:132:41 | [...] [Collection element] |
111+
| CommandInjection.swift:133:21:133:42 | [...] [Collection element] | CommandInjection.swift:93:20:93:40 | arguments [Collection element] |
112+
| CommandInjection.swift:133:22:133:22 | userControlledString | CommandInjection.swift:133:21:133:42 | [...] [Collection element] |
90113
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:144:42:144:42 | userControlledString |
91114
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:145:75:145:75 | userControlledString |
92115
| CommandInjection.swift:134:24:134:24 | userControlledString | CommandInjection.swift:148:35:148:35 | userControlledString |
@@ -183,6 +206,10 @@ nodes
183206
| CommandInjection.swift:81:6:81:6 | [post] task2 [arguments, Collection element] | semmle.label | [post] task2 [arguments, Collection element] |
184207
| CommandInjection.swift:81:24:81:46 | [...] [Collection element] | semmle.label | [...] [Collection element] |
185208
| CommandInjection.swift:81:31:81:31 | validatedString | semmle.label | validatedString |
209+
| CommandInjection.swift:93:20:93:40 | arguments [Collection element] | semmle.label | arguments [Collection element] |
210+
| CommandInjection.swift:94:3:94:3 | [post] self | semmle.label | [post] self |
211+
| CommandInjection.swift:94:3:94:3 | [post] self [arguments, Collection element] | semmle.label | [post] self [arguments, Collection element] |
212+
| CommandInjection.swift:94:20:94:20 | arguments [Collection element] | semmle.label | arguments [Collection element] |
186213
| CommandInjection.swift:99:8:99:12 | let ...? [some:0] | semmle.label | let ...? [some:0] |
187214
| CommandInjection.swift:99:12:99:12 | userControlledString | semmle.label | userControlledString |
188215
| CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | semmle.label | call to String.init(contentsOf:) |
@@ -217,6 +244,21 @@ nodes
217244
| CommandInjection.swift:126:2:126:7 | [post] ...? [arguments, Collection element] | semmle.label | [post] ...? [arguments, Collection element] |
218245
| CommandInjection.swift:126:21:126:42 | [...] [Collection element] | semmle.label | [...] [Collection element] |
219246
| CommandInjection.swift:126:22:126:22 | userControlledString | semmle.label | userControlledString |
247+
| CommandInjection.swift:130:2:130:2 | [post] task6 | semmle.label | [post] task6 |
248+
| CommandInjection.swift:130:2:130:2 | [post] task6 [executableURL] | semmle.label | [post] task6 [executableURL] |
249+
| CommandInjection.swift:130:24:130:65 | call to URL.init(fileURLWithPath:) | semmle.label | call to URL.init(fileURLWithPath:) |
250+
| CommandInjection.swift:130:45:130:45 | userControlledString | semmle.label | userControlledString |
251+
| CommandInjection.swift:131:2:131:2 | [post] task6 | semmle.label | [post] task6 |
252+
| CommandInjection.swift:131:2:131:2 | [post] task6 [executableURL] | semmle.label | [post] task6 [executableURL] |
253+
| CommandInjection.swift:131:24:131:56 | call to URL.init(string:) [some:0] | semmle.label | call to URL.init(string:) [some:0] |
254+
| CommandInjection.swift:131:24:131:57 | ...! | semmle.label | ...! |
255+
| CommandInjection.swift:131:36:131:36 | userControlledString | semmle.label | userControlledString |
256+
| CommandInjection.swift:132:2:132:2 | [post] task6 | semmle.label | [post] task6 |
257+
| CommandInjection.swift:132:2:132:2 | [post] task6 [arguments, Collection element] | semmle.label | [post] task6 [arguments, Collection element] |
258+
| CommandInjection.swift:132:20:132:41 | [...] [Collection element] | semmle.label | [...] [Collection element] |
259+
| CommandInjection.swift:132:21:132:21 | userControlledString | semmle.label | userControlledString |
260+
| CommandInjection.swift:133:21:133:42 | [...] [Collection element] | semmle.label | [...] [Collection element] |
261+
| CommandInjection.swift:133:22:133:22 | userControlledString | semmle.label | userControlledString |
220262
| CommandInjection.swift:134:24:134:24 | userControlledString | semmle.label | userControlledString |
221263
| CommandInjection.swift:144:42:144:42 | userControlledString | semmle.label | userControlledString |
222264
| CommandInjection.swift:145:67:145:95 | [...] | semmle.label | [...] |
@@ -290,13 +332,17 @@ subpaths
290332
#select
291333
| CommandInjection.swift:75:2:75:2 | [post] task1 | CommandInjection.swift:69:40:69:94 | call to String.init(contentsOf:) | CommandInjection.swift:75:2:75:2 | [post] task1 | This command depends on a $@. | CommandInjection.swift:69:40:69:94 | call to String.init(contentsOf:) | user-provided value |
292334
| CommandInjection.swift:81:6:81:6 | [post] task2 | CommandInjection.swift:69:40:69:94 | call to String.init(contentsOf:) | CommandInjection.swift:81:6:81:6 | [post] task2 | This command depends on a $@. | CommandInjection.swift:69:40:69:94 | call to String.init(contentsOf:) | user-provided value |
335+
| CommandInjection.swift:94:3:94:3 | [post] self | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:94:3:94:3 | [post] self | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
293336
| CommandInjection.swift:114:2:114:2 | [post] task3 | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:114:2:114:2 | [post] task3 | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
294337
| CommandInjection.swift:115:2:115:2 | [post] task3 | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:115:2:115:2 | [post] task3 | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
295338
| CommandInjection.swift:119:2:119:2 | [post] task4 | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:119:2:119:2 | [post] task4 | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
296339
| CommandInjection.swift:120:2:120:2 | [post] task4 | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:120:2:120:2 | [post] task4 | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
297340
| CommandInjection.swift:121:2:121:2 | [post] task4 | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:121:2:121:2 | [post] task4 | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
298341
| CommandInjection.swift:125:2:125:7 | [post] ...? | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:125:2:125:7 | [post] ...? | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
299342
| CommandInjection.swift:126:2:126:7 | [post] ...? | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:126:2:126:7 | [post] ...? | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
343+
| CommandInjection.swift:130:2:130:2 | [post] task6 | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:130:2:130:2 | [post] task6 | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
344+
| CommandInjection.swift:131:2:131:2 | [post] task6 | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:131:2:131:2 | [post] task6 | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
345+
| CommandInjection.swift:132:2:132:2 | [post] task6 | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:132:2:132:2 | [post] task6 | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
300346
| CommandInjection.swift:144:42:144:42 | userControlledString | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:144:42:144:42 | userControlledString | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
301347
| CommandInjection.swift:145:67:145:95 | [...] | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:145:67:145:95 | [...] | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |
302348
| CommandInjection.swift:148:23:148:56 | ...! | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | CommandInjection.swift:148:23:148:56 | ...! | This command depends on a $@. | CommandInjection.swift:99:40:99:94 | call to String.init(contentsOf:) | user-provided value |

‎swift/ql/test/query-tests/Security/CWE-078/CommandInjection.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class MyProcess : Process {
9191
var harmlessField: String?
9292

9393
func setArguments(_ arguments: [String]) {
94-
self.arguments = arguments
94+
self.arguments = arguments // BAD
9595
}
9696
}
9797

@@ -127,10 +127,10 @@ func testCommandInjectionMore(mySafeString: String) {
127127
try! task5?.run()
128128

129129
let task6 = MyProcess()
130-
task6.executableURL = URL(fileURLWithPath: userControlledString) // BAD [NOT DETECTED]
131-
task6.executableURL = URL(string: userControlledString)! // BAD [NOT DETECTED]
132-
task6.arguments = [userControlledString] // BAD [NOT DETECTED]
133-
task6.setArguments([userControlledString]) // BAD [NOT DETECTED]
130+
task6.executableURL = URL(fileURLWithPath: userControlledString) // BAD
131+
task6.executableURL = URL(string: userControlledString)! // BAD
132+
task6.arguments = [userControlledString] // BAD
133+
task6.setArguments([userControlledString]) // BAD (flagged inside `setArguments`)
134134
task6.harmlessField = userControlledString // GOOD
135135
try! task6.run()
136136

0 commit comments

Comments
 (0)