diff --git a/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll b/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll new file mode 100644 index 000000000000..5f8ae6701f49 --- /dev/null +++ b/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll @@ -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() + } + } +} diff --git a/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionQuery.qll b/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionQuery.qll new file mode 100644 index 000000000000..dd7283d15467 --- /dev/null +++ b/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionQuery.qll @@ -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; diff --git a/powershell/ql/src/queries/security/cwe-078/CommandInjection.qhelp b/powershell/ql/src/queries/security/cwe-078/CommandInjection.qhelp new file mode 100644 index 000000000000..b75401a5d708 --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-078/CommandInjection.qhelp @@ -0,0 +1,44 @@ + + + +

Code that passes user input directly to +Invoke-Expression, &, or some other library +routine that executes a command, allows the user to execute malicious +code.

+ +
+ + +

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.

+ +

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.

+ +
+ + +

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

+ + + +
+ + +
  • +OWASP: +Command Injection. +
  • + + + +
    +
    diff --git a/powershell/ql/src/queries/security/cwe-078/CommandInjection.ql b/powershell/ql/src/queries/security/cwe-078/CommandInjection.ql new file mode 100644 index 000000000000..ca28bea3aa0e --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-078/CommandInjection.ql @@ -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() diff --git a/powershell/ql/src/queries/security/cwe-078/examples/command_injection.ps1 b/powershell/ql/src/queries/security/cwe-078/examples/command_injection.ps1 new file mode 100644 index 000000000000..8874669360e7 --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-078/examples/command_injection.ps1 @@ -0,0 +1,3 @@ +param ($x) + +Invoke-Expression -Command "Get-Process -Id $x" \ No newline at end of file diff --git a/powershell/ql/test/qlpack.yml b/powershell/ql/test/qlpack.yml index ccd69e233fe4..b9e644ea2c3a 100644 --- a/powershell/ql/test/qlpack.yml +++ b/powershell/ql/test/qlpack.yml @@ -4,6 +4,7 @@ groups: - test dependencies: microsoft-sdl/powershell-all: ${workspace} + microsoft-sdl/powershell-queries: ${workspace} extractor: powershell tests: . warnOnImplicitThis: true \ No newline at end of file diff --git a/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected new file mode 100644 index 000000000000..53e66a64f682 --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected @@ -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 | diff --git a/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.qlref b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.qlref new file mode 100644 index 000000000000..06653bc5ac7c --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.qlref @@ -0,0 +1 @@ +queries/security/cwe-078/CommandInjection.ql \ No newline at end of file diff --git a/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/test.ps1 b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/test.ps1 new file mode 100644 index 000000000000..682b1af37528 --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/test.ps1 @@ -0,0 +1,7 @@ +param ($x) + +Invoke-Expression -Command "Get-Process -Id $x" # BAD + +$code = "$Env:MY_VAR" + +& "$code --enabled" # BAD \ No newline at end of file