8000 Merge pull request #13726 from maikypedia/maikypedia/swift-command-in… · github/codeql@e534afe · GitHub
[go: up one dir, main page]
Skip to content

Commit e534afe

Browse files
geoffw0geoffw0
authored
Merge pull request #13726 from maikypedia/maikypedia/swift-command-injection
Swift: Add Command Injection query (CWE-078)
2 parents 2562f8a + 90ac5b9 commit e534afe

File tree

7 files changed

+205
-0
lines changed

7 files changed

+205
-0
lines changed

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

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Provides classes and predicates for reasoning about system
3+
* commands built from user-controlled sources (that is, command injection
4+
* vulnerabilities).
5+
*/
6+
7+
import swift
8+
import codeql.swift.dataflow.DataFlow
9+
import codeql.swift.dataflow.ExternalFlow
10+
11+
/**
12+
* A dataflow sink for command injection vulnerabilities.
13+
*/
14+
abstract class CommandInjectionSink extends DataFlow::Node { }
15+
16+
/**
17+
* A barrier for command injection vulnerabilities.
18+
*/
19+
abstract class CommandInjectionBarrier extends DataFlow::Node { }
20+
21+
/**
22+
* A unit class for adding additional flow steps.
23+
*/
24+
class CommandInjectionAdditionalFlowStep extends Unit {
25+
/**
26+
* Holds if the step from `node1` to `node2` should be considered a flow
27+
* step for paths related to command injection vulnerabilities.
28+
*/
29+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
30+
}
31+
32+
private class ProcessSink2 extends CommandInjectionSink instanceof DataFlow::Node {
33+
ProcessSink2() {
34+
exists(AssignExpr assign, ProcessHost s |
35+
assign.getDest() = s and
36+
this.asExpr() = assign.getSource()
37+
)
38+
or
39+
exists(AssignExpr assign, ProcessHost s, ArrayExpr a |
40+
assign.getDest() = s and
41+
a = assign.getSource() and
42+
this.asExpr() = a.getAnElement()
43+
)
44+
}
45+
}
46+
47+
private class ProcessHost extends MemberRefExpr {
48+
ProcessHost() { this.getBase() instanceof ProcessRef }
49+
}
50+
51+
/** An expression of type `Process`. */
52+
private class ProcessRef extends Expr {
53+
ProcessRef() {
54+
this.getType() instanceof ProcessType or
55+
this.getType() = any(OptionalType t | t.getBaseType() instanceof ProcessType)
56+
}
57+
}
58+
59+
/** The type `Process`. */
60+
private class ProcessType extends NominalType {
61+
ProcessType() { this.getFullName() = "Process" }
62+
}
63+
64+
/**
65+
* A `DataFlow::Node` that is written into a `Process` object.
66+
*/
67+
private class ProcessSink extends CommandInjectionSink instanceof DataFlow::Node {
68+
ProcessSink() {
69+
// any write into a class derived from `Process` is a sink. For
70+
// example in `Process.launchPath = sensitive` the post-update node corresponding
71+
// with `Process.launchPath` is a sink.
72+
exists(NominalType t, Expr e |
73+
t.getABaseType*().getUnderlyingType().getName() = "Process" and
74+
e.getFullyConverted() = this.asExpr() and
75+
e.getFullyConverted().getType() = t
76+
)
77+
}
78+
}
79+
80+
/**
81+
* A sink defined in a CSV model.
82+
*/
83+
private class DefaultCommandInjectionSink extends CommandInjectionSink {
84+
DefaultCommandInjectionSink() { sinkNode(this, "command-injection") }
85+
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about system
3+
* commands built from user-controlled sources (that is, Command injection
4+
* vulnerabilities).
5+
*/
6+
7+
import swift
8+
import codeql.swift.dataflow.DataFlow
9+
import codeql.swift.dataflow.TaintTracking
10+
import codeql.swift.dataflow.FlowSources
11+
import codeql.swift.security.CommandInjectionExtensions
12+
13+
/**
14+
* A taint configuration for tainted data that reaches a Command Injection sink.
15+
*/
16+
module CommandInjectionConfig implements DataFlow::ConfigSig {
17+
predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
18+
19+
predicate isSink(DataFlow::Node node) { node instanceof CommandInjectionSink }
20+
21+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof CommandInjectionBarrier }
22+
23+
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
24+
any(CommandInjectionAdditionalFlowStep s).step(nodeFrom, nodeTo)
25+
}
26+
}
27+
28+
/**
29+
* Detect taint flow of tainted data that reaches a Command Injection sink.
30+
*/
31+
module CommandInjectionFlow = TaintTracking::Global<CommandInjectionConfig>;

‎swift/ql/src/change-notes/2023-07-12-command-injection.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added new query "Command injection" (`swift/command-line-injection`). The query finds places where user input is used to execute system commands without proper escaping.

‎swift/ql/src/experimental/Security/CWE-078/CommandInjection.qhelp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Constructing a system command with unsanitized user input is dangerous,
9+
since a malicious user may be able to craft input that executes arbitrary code.
10+
</p>
11+
</overview>
12+
13+
<recommendation>
14+
<p>
15+
If possible, use hard-coded string literals to specify the command to run. Instead of interpreting
16+
user input directly as command names, examine the input and then choose among hard-coded string
17+
literals.
18+
</p>
19+
<p>
20+
If this is not possible, then add sanitization code to verify that the user input is safe before
21+
using it.
22+
</p>
23+
</recommendation>
24+
25+
<example>
26+
<p>
27+
The following examples execute code from user input without
28+
sanitizing it first:
29+
</p>
30+
<sample src="CommandInjectionBad.swift" />
31+
<p>
32+
If user input is used to construct a command it should be checked
33+
first. This ensures that the user cannot insert characters that have special
34+
meanings.
35+
</p>
36+
<sample src="CommandInjectionGood.swift" />
37+
</example>
38+
39+
<references>
40+
<li>
41+
OWASP:
42+
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
43+
</li>
44+
</references>
45+
</qhelp>

‎swift/ql/src/experimental/Security/CWE-078/CommandInjection.ql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @name System command built from user-controlled sources
3+
* @description Building a system command from user-controlled sources is vulnerable to insertion of malicious code by the user.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 9.8
7+
* @precision high
8+
* @id swift/command-line-injection
9+
* @tags security
10+
* external/cwe/cwe-078
11+
* external/cwe/cwe-088
12+
*/
13+
14+
import swift
15+
import codeql.swift.dataflow.DataFlow
16+
import codeql.swift.security.CommandInjectionQuery
17+
import CommandInjectionFlow::PathGraph
18+
19+
from CommandInjectionFlow::PathNode sourceNode, CommandInjectionFlow::PathNode sinkNode
20+
where CommandInjectionFlow::flowPath(sourceNode, sinkNode)
21+
select sinkNode.getNode(), sourceNode, sinkNode, "This command depends on a $@.",
22+
sourceNode.getNode(), "user-provided value"

‎swift/ql/src/experimental/Security/CWE-078/CommandInjectionBad.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
var task = Process()
2+
task.launchPath = "/bin/bash"
3+
task.arguments = ["-c", userControlledString] // BAD
4+
5+
task.launch()

‎swift/ql/src/experimental/Security/CWE-078/CommandInjectionGood.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
func validateCommand(_ command: String) -> String? {
2+
let allowedCommands = ["ls -l", "pwd", "echo"]
3+
if allowedCommands.contains(command) {
4+
return command
5+
}
6+
return nil
7+
}
8+
9+
var task = Process()
10+
task.launchPath = "/bin/bash"
11+
task.arguments = ["-c", validateCommand(userControlledString)] // GOOD
12+
13+
task.launch()

0 commit comments

Comments
 (0)
0