Skip to content

Commit 86a40b9

Browse files
MathiasVPMathiasVP
authored
Merge pull request #138 from microsoft/powershell-tainted-command-query
PS: Add the first non-experimental query
2 parents 87cbfd1 + b3de6a2 commit 86a40b9

9 files changed

Lines changed: 178 additions & 0 deletions

File tree

‎powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about
3+
* command-injection vulnerabilities, as well as extension points for
4+
* adding your own.
5+
*/
6+
7+
private import semmle.code.powershell.dataflow.DataFlow
8+
private import semmle.code.powershell.dataflow.flowsources.FlowSources
9+
private import semmle.code.powershell.Cfg
10+
11+
module CommandInjection {
12+
/**
13+
* A data flow source for command-injection vulnerabilities.
14+
*/
15+
abstract class Source extends DataFlow::Node {
16+
/** Gets a string that describes the type of this flow source. */
17+
abstract string getSourceType();
18+
}
19+
20+
/**
21+
* A data flow sink for command-injection vulnerabilities.
22+
*/
23+
abstract class Sink extends DataFlow::Node { }
24+
25+
/**
26+
* A sanitizer for command-injection vulnerabilities.
27+
*/
28+
abstract class Sanitizer extends DataFlow::Node { }
29+
30+
/** A source of user input, considered as a flow source for command injection. */
31+
class FlowSourceAsSource extends Source instanceof SourceNode {
32+
override string getSourceType() { result = "user-provided value" }
33+
}
34+
35+
/**
36+
* A command argument to a function that initiates an operating system command.
37+
*/
38+
class SystemCommandExecutionSink extends Sink {
39+
SystemCommandExecutionSink() {
40+
// An argument to a call
41+
exists(DataFlow::CallNode call |
42+
call.getName() = "Invoke-Expression"
43+
or
44+
call instanceof DataFlow::CallOperatorNode
45+
|
46+
call.getAnArgument() = this
47+
)
48+
or
49+
// Or the call command itself in case it's a use of operator &.
50+
any(DataFlow::CallOperatorNode call).getCommand() = this
51+
}
52+
}
53+
54+
private class ExternalCommandInjectionSink extends Sink {
55+
ExternalCommandInjectionSink() {
56+
this = ModelOutput::getASinkNode("command-injection").asSink()
57+
}
58+
}
59+
}

‎powershell/ql/lib/semmle/code/powershell/security/CommandInjectionQuery.qll‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* Provides a taint tracking configuration for reasoning about
3+
* command-injection vulnerabilities (CWE-078).
4+
*
5+
* Note, for performance reasons: only import this file if
6+
* `CommandInjectionFlow` is needed, otherwise
7+
* `CommandInjectionCustomizations` should be imported instead.
8+
*/
9+
10+
import powershell
11+
import semmle.code.powershell.dataflow.TaintTracking
12+
import CommandInjectionCustomizations::CommandInjection
13+
import semmle.code.powershell.dataflow.DataFlow
14+
15+
private module Config implements DataFlow::ConfigSig {
16+
predicate isSource(DataFlow::Node source) { source instanceof Source }
17+
18+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
19+
20+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
21+
}
22+
23+
/**
24+
* Taint-tracking for reasoning about command-injection vulnerabilities.
25+
*/
26+
module CommandInjectionFlow = TaintTracking::Global<Config>;

‎powershell/ql/src/queries/security/cwe-078/CommandInjection.qhelp‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Code that passes user input directly to
7+
<code>Invoke-Expression</code>, <code>&</code>, or some other library
8+
routine that executes a command, allows the user to execute malicious
9+
code.</p>
10+
11+
</overview>
12+
<recommendation>
13+
14+
<p>If possible, use hard-coded string literals to specify the command to run
15+
or library to load. Instead of passing the user input directly to the
16+
process or library function, examine the user input and then choose
17+
among hard-coded string literals.</p>
18+
19+
<p>If the applicable libraries or commands cannot be determined at
20+
compile time, then add code to verify that the user input string is
21+
safe before using it.</p>
22+
23+
</recommendation>
24+
<example>
25+
26+
<p>The following example shows code that takes a shell script that can be changed
27+
maliciously by a user, and passes it straight to <code>Invoke-Expression</code>
28+
without examining it first.</p>
29+
30+
<sample src="examples/command_injection.ps1" />
31+
32+
</example>
33+
<references>
34+
35+
<li>
36+
OWASP:
37+
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
38+
</li>
39+
40+
<!-- LocalWords: CWE untrusted unsanitized Runtime
41+
-->
42+
43+
</references>
44+
</qhelp>

‎powershell/ql/src/queries/security/cwe-078/CommandInjection.ql‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/**
2+
* @name Uncontrolled command line
3+
* @description Using externally controlled strings in a command line may allow a malicious
4+
* user to change the meaning of the command.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 9.8
8+
* @precision high
9+
* @id powershell/command-injection
10+
* @tags correctness
11+
* security
12+
* external/cwe/cwe-078
13+
* external/cwe/cwe-088
14+
*/
15+
16+
import powershell
17+
import semmle.code.powershell.security.CommandInjectionQuery
18+
import CommandInjectionFlow::PathGraph
19+
20+
from CommandInjectionFlow::PathNode source, CommandInjectionFlow::PathNode sink, Source sourceNode
21+
where
22+
CommandInjectionFlow::flowPath(source, sink) and
23+
sourceNode = source.getNode()
24+
select sink.getNode(), source, sink, "This command depends on a $@.", sourceNode,
25+
sourceNode.getSourceType()

‎powershell/ql/src/queries/security/cwe-078/examples/command_injection.ps1‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
param ($x)
2+
3+
Invoke-Expression -Command "Get-Process -Id $x"

‎powershell/ql/test/qlpack.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ groups:
44
- test
55
dependencies:
66
microsoft-sdl/powershell-all: ${workspace}
7+
microsoft-sdl/powershell-queries: ${workspace}
78
extractor: powershell
89
tests: .
910
warnOnImplicitThis: true

‎powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
edges
2+
| test.ps1:1:8:1:10 | x | test.ps1:3:28:3:48 | Get-Process -Id $x | provenance | |
3+
| test.ps1:5:10:5:21 | Env:MY_VAR | test.ps1:7:3:7:20 | $code --enabled | provenance | |
4+
nodes
5+
| test.ps1:1:8:1:10 | x | semmle.label | x |
6+
| test.ps1:3:28:3:48 | Get-Process -Id $x | semmle.label | Get-Process -Id $x |
7+
| test.ps1:5:10:5:21 | Env:MY_VAR | semmle.label | Env:MY_VAR |
8+
| test.ps1:7:3:7:20 | $code --enabled | semmle.label | $code --enabled |
9+
subpaths
10+
#select
11+
| test.ps1:3:28:3:48 | Get-Process -Id $x | test.ps1:1:8:1:10 | x | test.ps1:3:28:3:48 | Get-Process -Id $x | This command depends on a $@. | test.ps1:1:8:1:10 | x | user-provided value |
12+
| test.ps1:7:3:7:20 | $code --enabled | test.ps1:5:10:5:21 | Env:MY_VAR | test.ps1:7:3:7:20 | $code --enabled | This command depends on a $@. | test.ps1:5:10:5:21 | Env:MY_VAR | user-provided value |

‎powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/security/cwe-078/CommandInjection.ql

‎powershell/ql/test/query-tests/security/cwe-078/CommandInjection/test.ps1‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
param ($x)
2+
3+
Invoke-Expression -Command "Get-Process -Id $x" # BAD
4+
5+
$code = "$Env:MY_VAR"
6+
7+
& "$code --enabled" # BAD

0 commit comments

Comments
 (0)