Skip to content

Commit 27bdee8

Browse files
committed
Swift: Replace additional taint step with implict read.
Now that we have array content, this is a more principled approach than having a special case data step.
1 parent 2684a22 commit 27bdee8

File tree

3 files changed

+137
-119
lines changed

3 files changed

+137
-119
lines changed

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

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,6 @@ class CommandInjectionAdditionalFlowStep extends Unit {
2929
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
3030
}
3131

32-
/**
33-
* An additional taint step for command injection vulnerabilities.
34-
*/
35-
private class CommandInjectionArrayAdditionalFlowStep extends CommandInjectionAdditionalFlowStep {
36-
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
37-
// needed until we have proper content flow through arrays.
38-
exists(ArrayExpr arr |
39-
nodeFrom.asExpr() = arr.getAnElement() and
40-
nodeTo.asExpr() = arr
41-
)
42-
}
43-
}
44-
4532
/**
4633
* A sink defined in a CSV model.
4734
*/

swift/ql/lib/codeql/swift/security/CommandInjectionQuery.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ module CommandInjectionConfig implements DataFlow::ConfigSig {
2323
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
2424
any(CommandInjectionAdditionalFlowStep s).step(nodeFrom, nodeTo)
2525
}
26+
27+
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
28+
// flow out from array elements of at the sink, for example in `task.arguments = [tainted]`.
29+
isSink(node) and
30+
c.getAReadContent() instanceof DataFlow::Content::ArrayContent
31+
}
2632
}
2733

2834
/**

0 commit comments

Comments
 (0)