Skip to content

Commit

Permalink
Merge pull request #138 from microsoft/powershell-tainted-command-query
Browse files Browse the repository at this point in the history
PS: Add the first non-experimental query
  • Loading branch information
MathiasVP authored Nov 8, 2024
2 parents 87cbfd1 + b3de6a2 commit 86a40b9
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* Provides default sources, sinks and sanitizers for reasoning about
* command-injection vulnerabilities, as well as extension points for
* adding your own.
*/

private import semmle.code.powershell.dataflow.DataFlow
private import semmle.code.powershell.dataflow.flowsources.FlowSources
private import semmle.code.powershell.Cfg

module CommandInjection {
/**
* A data flow source for command-injection vulnerabilities.
*/
abstract class Source extends DataFlow::Node {
/** Gets a string that describes the type of this flow source. */
abstract string getSourceType();
}

/**
* A data flow sink for command-injection vulnerabilities.
*/
abstract class Sink extends DataFlow::Node { }

/**
* A sanitizer for command-injection vulnerabilities.
*/
abstract class Sanitizer extends DataFlow::Node { }

/** A source of user input, considered as a flow source for command injection. */
class FlowSourceAsSource extends Source instanceof SourceNode {
override string getSourceType() { result = "user-provided value" }
}

/**
* A command argument to a function that initiates an operating system command.
*/
class SystemCommandExecutionSink extends Sink {
SystemCommandExecutionSink() {
// An argument to a call
exists(DataFlow::CallNode call |
call.getName() = "Invoke-Expression"
or
call instanceof DataFlow::CallOperatorNode
|
call.getAnArgument() = this
)
or
// Or the call command itself in case it's a use of operator &.
any(DataFlow::CallOperatorNode call).getCommand() = this
}
}

private class ExternalCommandInjectionSink extends Sink {
ExternalCommandInjectionSink() {
this = ModelOutput::getASinkNode("command-injection").asSink()
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Provides a taint tracking configuration for reasoning about
* command-injection vulnerabilities (CWE-078).
*
* Note, for performance reasons: only import this file if
* `CommandInjectionFlow` is needed, otherwise
* `CommandInjectionCustomizations` should be imported instead.
*/

import powershell
import semmle.code.powershell.dataflow.TaintTracking
import CommandInjectionCustomizations::CommandInjection
import semmle.code.powershell.dataflow.DataFlow

private module Config implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof Source }

predicate isSink(DataFlow::Node sink) { sink instanceof Sink }

predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
}

/**
* Taint-tracking for reasoning about command-injection vulnerabilities.
*/
module CommandInjectionFlow = TaintTracking::Global<Config>;
44 changes: 44 additions & 0 deletions powershell/ql/src/queries/security/cwe-078/CommandInjection.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Code that passes user input directly to
<code>Invoke-Expression</code>, <code>&</code>, or some other library
routine that executes a command, allows the user to execute malicious
code.</p>

</overview>
<recommendation>

<p>If possible, use hard-coded string literals to specify the command to run
or library to load. Instead of passing the user input directly to the
process or library function, examine the user input and then choose
among hard-coded string literals.</p>

<p>If the applicable libraries or commands cannot be determined at
compile time, then add code to verify that the user input string is
safe before using it.</p>

</recommendation>
<example>

<p>The following example shows code that takes a shell script that can be changed
maliciously by a user, and passes it straight to <code>Invoke-Expression</code>
without examining it first.</p>

<sample src="examples/command_injection.ps1" />

</example>
<references>

<li>
OWASP:
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
</li>

<!-- LocalWords: CWE untrusted unsanitized Runtime
-->

</references>
</qhelp>
25 changes: 25 additions & 0 deletions powershell/ql/src/queries/security/cwe-078/CommandInjection.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* @name Uncontrolled command line
* @description Using externally controlled strings in a command line may allow a malicious
* user to change the meaning of the command.
* @kind path-problem
* @problem.severity error
* @security-severity 9.8
* @precision high
* @id powershell/command-injection
* @tags correctness
* security
* external/cwe/cwe-078
* external/cwe/cwe-088
*/

import powershell
import semmle.code.powershell.security.CommandInjectionQuery
import CommandInjectionFlow::PathGraph

from CommandInjectionFlow::PathNode source, CommandInjectionFlow::PathNode sink, Source sourceNode
where
CommandInjectionFlow::flowPath(source, sink) and
sourceNode = source.getNode()
select sink.getNode(), source, sink, "This command depends on a $@.", sourceNode,
sourceNode.getSourceType()
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
param ($x)

Invoke-Expression -Command "Get-Process -Id $x"
1 change: 1 addition & 0 deletions powershell/ql/test/qlpack.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ groups:
- test
dependencies:
microsoft-sdl/powershell-all: ${workspace}
microsoft-sdl/powershell-queries: ${workspace}
extractor: powershell
tests: .
warnOnImplicitThis: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
edges
| test.ps1:1:8:1:10 | x | test.ps1:3:28:3:48 | Get-Process -Id $x | provenance | |
| test.ps1:5:10:5:21 | Env:MY_VAR | test.ps1:7:3:7:20 | $code --enabled | provenance | |
nodes
| test.ps1:1:8:1:10 | x | semmle.label | x |
| test.ps1:3:28:3:48 | Get-Process -Id $x | semmle.label | Get-Process -Id $x |
| test.ps1:5:10:5:21 | Env:MY_VAR | semmle.label | Env:MY_VAR |
| test.ps1:7:3:7:20 | $code --enabled | semmle.label | $code --enabled |
subpaths
#select
| 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 |
| 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 |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
queries/security/cwe-078/CommandInjection.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
param ($x)

Invoke-Expression -Command "Get-Process -Id $x" # BAD

$code = "$Env:MY_VAR"

& "$code --enabled" # BAD

0 comments on commit 86a40b9

Please sign in to comment.