Skip to content

Commit 2b7b1c6

Browse files
geoffw0geoffw0
committed
Swift: Use a barrier as a simple fix.
1 parent 7bf61d1 commit 2b7b1c6

File tree

3 files changed

+9
-21
lines changed

3 files changed

+9
-21
lines changed

‎swift/ql/lib/codeql/swift/security/CommandInjectionExtensions.qll

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,5 +64,12 @@ private class CommandInjectionDefaultBarrier extends CommandInjectionBarrier {
6464
CommandInjectionDefaultBarrier() {
6565
// any numeric type
6666
this.asExpr().getType().getUnderlyingType().getABaseType*().getName() = "Numeric"
67+
or
68+
// we get poor results when the tainted data is a directory list, such as
69+
// the result of `FileMananger.contentsOfDirectory` and similar functions.
70+
exists(CallExpr ce |
71+
ce.getStaticTarget().getName().matches(["%directory%", "%Directory%"]) and
72+
this.asExpr() = ce
73+
)
6774
}
6875
}

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

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,6 @@ edges
136136
| CommandInjection.swift:166:45:166:77 | call to URL.init(string:) [some:0] | CommandInjection.swift:166:45:166:78 | ...! | provenance | |
137137
| CommandInjection.swift:166:45:166:78 | ...! | file://:0:0:0:0 | url | provenance | |
138138
| CommandInjection.swift:166:57:166:57 | userControlledString | CommandInjection.swift:166:45:166:77 | call to URL.init(string:) [some:0] | provenance | |
139-
| CommandInjection.swift:170:19:170:83 | call to contentsOfDirectory(atPath:) | CommandInjection.swift:176:22:176:22 | files | provenance | |
140-
| CommandInjection.swift:170:19:170:83 | call to contentsOfDirectory(atPath:) | CommandInjection.swift:178:23:178:30 | ...[...] | provenance | |
141-
| CommandInjection.swift:176:3:176:3 | [post] task12 [arguments] | CommandInjection.swift:176:3:176:3 | [post] task12 | provenance | |
142-
| CommandInjection.swift:176:22:176:22 | files | CommandInjection.swift:176:3:176:3 | [post] task12 [arguments] | provenance | |
143-
| CommandInjection.swift:176:22:176:22 | files | CommandInjection.swift:176:22:176:22 | files | provenance | |
144-
| CommandInjection.swift:176:22:176:22 | files | CommandInjection.swift:178:23:178:30 | ...[...] | provenance | |
145-
| CommandInjection.swift:178:3:178:3 | [post] task12 [arguments, Collection element] | CommandInjection.swift:178:3:178:3 | [post] task12 | provenance | |
146-
| CommandInjection.swift:178:22:178:31 | [...] [Collection element] | CommandInjection.swift:178:3:178:3 | [post] task12 [arguments, Collection element] | provenance | |
147-
| CommandInjection.swift:178:23:178:30 | ...[...] | CommandInjection.swift:178:22:178:31 | [...] [Collection element] | provenance | |
148139
| CommandInjection.swift:193:3:193:3 | newValue [Collection element] | CommandInjection.swift:194:19:194:19 | newValue [Collection element] | provenance | |
149140
| CommandInjection.swift:193:3:193:3 | newValue [Collection element] | CommandInjection.swift:195:20:195:20 | newValue [Collection element] | provenance | |
150141
| CommandInjection.swift:193:3:193:3 | newValue [Collection element] | CommandInjection.swift:196:19:196:19 | newValue [Collection element] | provenance | |
@@ -293,14 +284,6 @@ nodes
293284
| CommandInjection.swift:166:45:166:78 | ...! | semmle.label | ...! |
294285
| CommandInjection.swift:166:45:166:78 | ...! | semmle.label | ...! |
295286
| CommandInjection.swift:166:57:166:57 | userControlledString | semmle.label | userControlledString |
296-
| CommandInjection.swift:170:19:170:83 | call to contentsOfDirectory(atPath:) | semmle.label | call to contentsOfDirectory(atPath:) |
297-
| CommandInjection.swift:176:3:176:3 | [post] task12 | semmle.label | [post] task12 |
298-
| CommandInjection.swift:176:3:176:3 | [post] task12 [arguments] | semmle.label | [post] task12 [arguments] |
299-
| CommandInjection.swift:176:22:176:22 | files | semmle.label | files |
300-
| CommandInjection.swift:178:3:178:3 | [post] task12 | semmle.label | [post] task12 |
301-
| CommandInjection.swift:178:3:178:3 | [post] task12 [arguments, Collection element] | semmle.label | [post] task12 [arguments, Collection element] |
302-
| CommandInjection.swift:178:22:178:31 | [...] [Collection element] | semmle.label | [...] [Collection element] |
303-
| CommandInjection.swift:178:23:178:30 | ...[...] | semmle.label | ...[...] |
304287
| CommandInjection.swift:193:3:193:3 | newValue [Collection element] | semmle.label | newValue [Collection element] |
305288
| CommandInjection.swift:194:4:194:4 | [post] getter for .p1 | semmle.label | [post] getter for .p1 |
306289
| CommandInjection.swift:194:4:194:4 | [post] getter for .p1 [arguments, Collection element] | semmle.label | [post] getter for .p1 [arguments, Collection element] |
@@ -368,8 +351,6 @@ subpaths
368351
| CommandInjection.swift:163:40:163:73 | ...! | CommandInjection.swift:105:40:105:94 | call to String.init(contentsOf:) | CommandInjection.swift:163:40:163:73 | ...! | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | call to String.init(contentsOf:) | user-provided value |
369352
| CommandInjection.swift:164:32:164:53 | [...] | CommandInjection.swift:105:40:105:94 | call to String.init(contentsOf:) | CommandInjection.swift:164:32:164:53 | [...] | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | call to String.init(contentsOf:) | user-provided value |
370353
| CommandInjection.swift:166:45:166:78 | ...! | CommandInjection.swift:105:40:105:94 | call to String.init(contentsOf:) | CommandInjection.swift:166:45:166:78 | ...! | This command depends on a $@. | CommandInjection.swift:105:40:105:94 | call to String.init(contentsOf:) | user-provided value |
371-
| CommandInjection.swift:176:3:176:3 | [post] task12 | CommandInjection.swift:170:19:170:83 | call to contentsOfDirectory(atPath:) | CommandInjection.swift:176:3:176:3 | [post] task12 | This command depends on a $@. | CommandInjection.swift:170:19:170:83 | call to contentsOfDirectory(atPath:) | user-provided value |
372-
| CommandInjection.swift:178:3:178:3 | [post] task12 | CommandInjection.swift:170:19:170:83 | call to contentsOfDirectory(atPath:) | CommandInjection.swift:178:3:178:3 | [post] task12 | This command depends on a $@. | CommandInjection.swift:170:19:170:83 | call to contentsOfDirectory(atPath:) | user-provided value |
373354
| CommandInjection.swift:194:4:194:4 | [post] getter for .p1 | CommandInjection.swift:201:41:201:95 | call to String.init(contentsOf:) | CommandInjection.swift:194:4:194:4 | [post] getter for .p1 | This command depends on a $@. | CommandInjection.swift:201:41:201:95 | call to String.init(contentsOf:) | user-provided value |
374355
| CommandInjection.swift:195:4:195:6 | [post] ...! | CommandInjection.swift:201:41:201:95 | call to String.init(contentsOf:) | CommandInjection.swift:195:4:195:6 | [post] ...! | This command depends on a $@. | CommandInjection.swift:201:41:201:95 | call to String.init(contentsOf:) | user-provided value |
375356
| CommandInjection.swift:196:4:196:4 | [post] ...! | CommandInjection.swift:201:41:201:95 | call to String.init(contentsOf:) | CommandInjection.swift:196:4:196:4 | [post] ...! | This command depends on a $@. | CommandInjection.swift:201:41:201:95 | call to String.init(contentsOf:) | user-provided value |

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,9 @@ func testCommandInjectionMore(mySafeString: String) {
173173
task12.launchPath = "/bin/rm" // GOOD
174174
task12.arguments = [file] // GOOD (cases like this vary, but our analysis doesn't work well on them)
175175
task12.launch()
176-
task12.arguments = files // GOOD (similar to previous) [FALSE POSITIVE]
176+
task12.arguments = files // GOOD (similar to previous)
177177
task12.launch()
178-
task12.arguments = [files[0]] // GOOD (similar to previous) [FALSE POSITIVE]
178+
task12.arguments = [files[0]] // GOOD (similar to previous)
179179
task12.launch()
180180
}
181181

0 commit comments

Comments
 (0)