Skip to content

Commit 832173a

Browse files
asgerfasgerf
committed
unified: First round of heuristics
1 parent da3c888 commit 832173a

3 files changed

Lines changed: 50 additions & 7 deletions

File tree

‎unified/ql/src/queries/security/CWE-022/PathInjection.ql‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,35 @@
1616

1717
import unified
1818

19+
/**
20+
* A string that might be a label for a path argument.
21+
*/
22+
pragma[inline]
23+
private predicate pathLikeHeuristic(string label) {
24+
label =
25+
[
26+
"atFile", "atPath", "atDirectory", "toFile", "toPath", "toDirectory", "inFile", "inPath",
27+
"inDirectory", "contentsOfFile", "contentsOfPath", "contentsOfDirectory", "filePath",
28+
"directory", "directoryPath"
29+
]
30+
}
31+
32+
predicate heuristicSink(DataFlow::Node node) {
33+
node.isIncomingValue(any(Identifier id | id.getValue() = "sqlite3_temp_directory"))
34+
or
35+
exists(Argument arg |
36+
pathLikeHeuristic(arg.getName()) and
37+
node.asExpr() = arg.getValue()
38+
)
39+
}
40+
1941
module PathInjectionConfig implements DataFlow::ConfigSig {
2042
predicate isSource(DataFlow::Node node) { Models::isSource(node, _) }
2143

22-
predicate isSink(DataFlow::Node node) { Models::isSink(node, "path-injection") }
44+
predicate isSink(DataFlow::Node node) {
45+
Models::isSink(node, "path-injection") or
46+
heuristicSink(node)
47+
}
2348

2449
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
2550

‎unified/ql/test/query-tests/security/CWE-022/PathInjection/PathInjectionTest.expected‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,17 @@
8080
| testPathInjection.swift:509:37:509:48 | remoteString | testPathInjection.swift:503:24:503:78 | String(...) | testPathInjection.swift:509:37:509:48 | remoteString | This path depends on a $@. | testPathInjection.swift:503:24:503:78 | String(...) | user-provided value |
8181
| testPathInjection.swift:511:33:511:44 | remoteString | testPathInjection.swift:503:24:503:78 | String(...) | testPathInjection.swift:511:33:511:44 | remoteString | This path depends on a $@. | testPathInjection.swift:503:24:503:78 | String(...) | user-provided value |
8282
| testPathInjection.swift:526:28:526:29 | u1 | testPathInjection.swift:518:24:518:78 | String(...) | testPathInjection.swift:526:28:526:29 | u1 | This path depends on a $@. | testPathInjection.swift:518:24:518:78 | String(...) | user-provided value |
83+
| testPathInjection.swift:528:28:528:39 | remoteString | testPathInjection.swift:518:24:518:78 | String(...) | testPathInjection.swift:528:28:528:39 | remoteString | This path depends on a $@. | testPathInjection.swift:518:24:518:78 | String(...) | user-provided value |
8384
| testPathInjection.swift:540:32:540:43 | remoteString | testPathInjection.swift:518:24:518:78 | String(...) | testPathInjection.swift:540:32:540:43 | remoteString | This path depends on a $@. | testPathInjection.swift:518:24:518:78 | String(...) | user-provided value |
8485
| testPathInjection.swift:541:38:541:49 | remoteString | testPathInjection.swift:518:24:518:78 | String(...) | testPathInjection.swift:541:38:541:49 | remoteString | This path depends on a $@. | testPathInjection.swift:518:24:518:78 | String(...) | user-provided value |
8586
| testPathInjection.swift:542:45:542:56 | remoteString | testPathInjection.swift:518:24:518:78 | String(...) | testPathInjection.swift:542:45:542:56 | remoteString | This path depends on a $@. | testPathInjection.swift:518:24:518:78 | String(...) | user-provided value |
8687
| testPathInjection.swift:546:32:546:41 | ... .pointee | testPathInjection.swift:518:24:518:78 | String(...) | testPathInjection.swift:546:32:546:41 | ... .pointee | This path depends on a $@. | testPathInjection.swift:518:24:518:78 | String(...) | user-provided value |
8788
| testPathInjection.swift:558:35:558:46 | remoteString | testPathInjection.swift:518:24:518:78 | String(...) | testPathInjection.swift:558:35:558:46 | remoteString | This path depends on a $@. | testPathInjection.swift:518:24:518:78 | String(...) | user-provided value |
8889
| testPathInjection.swift:559:41:559:52 | remoteString | testPathInjection.swift:518:24:518:78 | String(...) | testPathInjection.swift:559:41:559:52 | remoteString | This path depends on a $@. | testPathInjection.swift:518:24:518:78 | String(...) | user-provided value |
90+
| testPathInjection.swift:581:25:581:36 | remoteString | testPathInjection.swift:579:24:579:78 | String(...) | testPathInjection.swift:581:25:581:36 | remoteString | This path depends on a $@. | testPathInjection.swift:579:24:579:78 | String(...) | user-provided value |
91+
| testPathInjection.swift:583:41:583:52 | remoteString | testPathInjection.swift:579:24:579:78 | String(...) | testPathInjection.swift:583:41:583:52 | remoteString | This path depends on a $@. | testPathInjection.swift:579:24:579:78 | String(...) | user-provided value |
92+
| testPathInjection.swift:585:38:585:49 | remoteString | testPathInjection.swift:579:24:579:78 | String(...) | testPathInjection.swift:585:38:585:49 | remoteString | This path depends on a $@. | testPathInjection.swift:579:24:579:78 | String(...) | user-provided value |
93+
| testPathInjection.swift:587:22:587:33 | remoteString | testPathInjection.swift:579:24:579:78 | String(...) | testPathInjection.swift:587:22:587:33 | remoteString | This path depends on a $@. | testPathInjection.swift:579:24:579:78 | String(...) | user-provided value |
8994
edges
9095
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:351:34:351:45 | remoteString | provenance | |
9196
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:352:26:352:37 | remoteString | provenance | |
@@ -171,6 +176,7 @@ edges
171176
| testPathInjection.swift:503:9:503:20 | remoteString | testPathInjection.swift:511:33:511:44 | remoteString | provenance | |
172177
| testPathInjection.swift:503:24:503:78 | String(...) | testPathInjection.swift:503:9:503:20 | remoteString | provenance | |
173178
| testPathInjection.swift:518:9:518:20 | remoteString | testPathInjection.swift:526:28:526:29 | u1 | provenance | |
179+
| testPathInjection.swift:518:9:518:20 | remoteString | testPathInjection.swift:528:28:528:39 | remoteString | provenance | |
174180
| testPathInjection.swift:518:9:518:20 | remoteString | testPathInjection.swift:540:32:540:43 | remoteString | provenance | |
175181
| testPathInjection.swift:518:9:518:20 | remoteString | testPathInjection.swift:541:38:541:49 | remoteString | provenance | |
176182
| testPathInjection.swift:518:9:518:20 | remoteString | testPathInjection.swift:542:45:542:56 | remoteString | provenance | |
@@ -182,6 +188,11 @@ edges
182188
| testPathInjection.swift:545:5:545:14 | ... .pointee | testPathInjection.swift:545:5:545:6 | [post] s1 [pointee] | provenance | |
183189
| testPathInjection.swift:545:18:545:29 | remoteString | testPathInjection.swift:545:5:545:14 | ... .pointee | provenance | |
184190
| testPathInjection.swift:546:32:546:33 | s1 [pointee] | testPathInjection.swift:546:32:546:41 | ... .pointee | provenance | |
191+
| testPathInjection.swift:579:9:579:20 | remoteString | testPathInjection.swift:581:25:581:36 | remoteString | provenance | |
192+
| testPathInjection.swift:579:9:579:20 | remoteString | testPathInjection.swift:583:41:583:52 | remoteString | provenance | |
193+
| testPathInjection.swift:579:9:579:20 | remoteString | testPathInjection.swift:585:38:585:49 | remoteString | provenance | |
194+
| testPathInjection.swift:579:9:579:20 | remoteString | testPathInjection.swift:587:22:587:33 | remoteString | provenance | |
195+
| testPathInjection.swift:579:24:579:78 | String(...) | testPathInjection.swift:579:9:579:20 | remoteString | provenance | |
185196
nodes
186197
| testPathInjection.swift:340:9:340:20 | remoteString | semmle.label | remoteString |
187198
| testPathInjection.swift:340:24:340:78 | String(...) | semmle.label | String(...) |
@@ -272,6 +283,7 @@ nodes
272283
| testPathInjection.swift:518:9:518:20 | remoteString | semmle.label | remoteString |
273284
| testPathInjection.swift:518:24:518:78 | String(...) | semmle.label | String(...) |
274285
| testPathInjection.swift:526:28:526:29 | u1 | semmle.label | u1 |
286+
| testPathInjection.swift:528:28:528:39 | remoteString | semmle.label | remoteString |
275287
| testPathInjection.swift:540:32:540:43 | remoteString | semmle.label | remoteString |
276288
| testPathInjection.swift:541:38:541:49 | remoteString | semmle.label | remoteString |
277289
| testPathInjection.swift:542:45:542:56 | remoteString | semmle.label | remoteString |
@@ -282,4 +294,10 @@ nodes
282294
| testPathInjection.swift:546:32:546:41 | ... .pointee | semmle.label | ... .pointee |
283295
| testPathInjection.swift:558:35:558:46 | remoteString | semmle.label | remoteString |
284296
| testPathInjection.swift:559:41:559:52 | remoteString | semmle.label | remoteString |
297+
| testPathInjection.swift:579:9:579:20 | remoteString | semmle.label | remoteString |
298+
| testPathInjection.swift:579:24:579:78 | String(...) | semmle.label | String(...) |
299+
| testPathInjection.swift:581:25:581:36 | remoteString | semmle.label | remoteString |
300+
| testPathInjection.swift:583:41:583:52 | remoteString | semmle.label | remoteString |
301+
| testPathInjection.swift:585:38:585:49 | remoteString | semmle.label | remoteString |
302+
| testPathInjection.swift:587:22:587:33 | remoteString | semmle.label | remoteString |
285303
subpaths

‎unified/ql/test/query-tests/security/CWE-022/PathInjection/testPathInjection.swift‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ func testPathInjection2(
525525
u1.appendPathComponent(remoteString)
526526
_ = NSData(contentsOf: u1) // $ Alert
527527

528-
let u2 = URL(filePath: remoteString) // $ MISSING: Alert
528+
let u2 = URL(filePath: remoteString) // $ Alert
529529
_ = NSData(contentsOf: u2) // $ MISSING: Alert
530530

531531
let u3 = NSURL(string: "")!
@@ -576,15 +576,15 @@ class MyFile {
576576
}
577577

578578
func testPathInjectionHeuristics() {
579-
let remoteString = String(contentsOf: URL(string: "http://example.com/")!) // $ MISSING: Source
579+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!) // $ Source
580580

581-
myOpenFile1(atPath: remoteString) // $ MISSING: Alert
581+
myOpenFile1(atPath: remoteString) // $ Alert
582582
myOpenFile2(remoteString) // $ MISSING: Alert
583-
myFindFiles(ofType: 0, inDirectory: remoteString) // $ MISSING: Alert
583+
myFindFiles(ofType: 0, inDirectory: remoteString) // $ Alert
584584

585-
let mc = MyClass(contentsOfFile: remoteString) // $ MISSING: Alert
585+
let mc = MyClass(contentsOfFile: remoteString) // $ Alert
586586
mc.doSomething(keyPath: remoteString) // good - not a path
587-
mc.write(toFile: remoteString) // $ MISSING: Alert
587+
mc.write(toFile: remoteString) // $ Alert
588588

589589
let mf1 = MyFile(path: "")
590590
let mf2 = MyFile(path: remoteString) // $ MISSING: Alert // Originally missing

0 commit comments

Comments
 (0)