diff --git a/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll b/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll index b78d0a561e44..5ed0c2d0b671 100644 --- a/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll +++ b/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll @@ -34,12 +34,19 @@ module CommandInjection { class FlowSourceAsSource extends Source { FlowSourceAsSource() { this instanceof SourceNode and - not this instanceof EnvironmentVariableSource + not this instanceof EnvironmentVariableSource and + not this instanceof InvokeWebRequest } override string getSourceType() { result = "user-provided value" } } + class InvokeWebRequest extends DataFlow::CallNode { + InvokeWebRequest(){ + this.matchesName("Invoke-WebRequest") + } + } + /** * A command argument to a function that initiates an operating system command. */ @@ -60,6 +67,16 @@ module CommandInjection { override string getSinkType() { result = "call to Invoke-Expression" } } + class StartProcessSink extends Sink { + StartProcessSink(){ + exists(DataFlow::CallNode call | + call.matchesName("Start-Process") and + call.getAnArgument() = this + ) + } + override string getSinkType(){ result = "call to Start-Process"} + } + class AddTypeSink extends Sink { AddTypeSink() { exists(DataFlow::CallNode call | @@ -218,6 +235,17 @@ module CommandInjection { } } + class ValidateAttributeSanitizer extends Sanitizer { + ValidateAttributeSanitizer() { + exists(Function f, Attribute a, Parameter p | + p = f.getAParameter() and + p.getAnAttribute() = a and + a.getName() = ["ValidateScript", "ValidateSet", "ValidatePattern"] and + this.asParameter() = p + ) + } + } + class SingleQuoteSanitizer extends Sanitizer { SingleQuoteSanitizer() { exists(ExpandableStringExpr e, VarReadAccess v | diff --git a/powershell/ql/src/queries/security/cwe-078/CommandInjectionCritical.qhelp b/powershell/ql/src/queries/security/cwe-078/CommandInjectionCritical.qhelp new file mode 100644 index 000000000000..b7186c49e01d --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-078/CommandInjectionCritical.qhelp @@ -0,0 +1,63 @@ + + + +

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.

+ +

The following are considered dangerous sinks:

+ + +
+ + +

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. +
  • +
  • +Injection Hunter: +PowerShell Injection Hunter: Security Auditing for PowerShell Scripts. +
  • + + + +
    +
    diff --git a/powershell/ql/src/queries/security/cwe-078/CommandInjectionCritical.ql b/powershell/ql/src/queries/security/cwe-078/CommandInjectionCritical.ql new file mode 100644 index 000000000000..c83fb4ed98d6 --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-078/CommandInjectionCritical.ql @@ -0,0 +1,59 @@ +/** + * @name Uncontrolled command line from param to CmdletBinding + * @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/microsoft/public/command-injection-critical + * @tags correctness + * security + * external/cwe/cwe-078 + * external/cwe/cwe-088 + */ + +import powershell +import semmle.code.powershell.security.CommandInjectionCustomizations::CommandInjection +import semmle.code.powershell.dataflow.TaintTracking +import semmle.code.powershell.dataflow.DataFlow + +abstract class CriticalSource extends DataFlow::Node { + /** Gets a string that describes the type of this flow source. */ + abstract string getSourceType(); +} + +class CmdletBindingParam extends CriticalSource { + CmdletBindingParam(){ + exists(Attribute a, Function f | + a.getName() = "CmdletBinding" and + f = a.getEnclosingFunction() and + this.asParameter() = f.getAParameter() + ) + } + override string getSourceType(){ + result = "param to CmdletBinding function" + } +} + + +private module Config implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof CriticalSource } + + 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 CommandInjectionCriticalFlow = TaintTracking::Global; +import CommandInjectionCriticalFlow::PathGraph + +from CommandInjectionCriticalFlow::PathNode source, CommandInjectionCriticalFlow::PathNode sink, CriticalSource sourceNode +where + CommandInjectionCriticalFlow::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/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected index fd567d0d46f0..9128aef9ba66 100644 --- a/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected +++ b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected @@ -18,51 +18,57 @@ edges | test.ps1:129:11:129:20 | userinput | test.ps1:131:28:131:37 | UserInput | provenance | | | test.ps1:136:11:136:20 | userinput | test.ps1:139:50:139:59 | UserInput | provenance | | | test.ps1:144:11:144:20 | userinput | test.ps1:147:63:147:72 | UserInput | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:154:46:154:51 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:155:46:155:51 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:156:46:156:51 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:157:46:157:51 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:158:46:158:51 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:159:46:159:51 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:160:46:160:51 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:161:46:161:51 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:163:48:163:53 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:164:48:164:53 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:165:48:165:53 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:166:41:166:46 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:167:41:167:46 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:168:36:168:41 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:169:36:169:41 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:170:36:170:41 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:172:42:172:47 | input | provenance | | -| test.ps1:152:1:152:6 | input | test.ps1:173:42:173:47 | input | provenance | | -| test.ps1:152:10:152:32 | Call to read-host | test.ps1:152:1:152:6 | input | provenance | Src:MaD:0 | -| test.ps1:154:46:154:51 | input | test.ps1:3:11:3:20 | userinput | provenance | | -| test.ps1:155:46:155:51 | input | test.ps1:9:11:9:20 | userinput | provenance | | -| test.ps1:156:46:156:51 | input | test.ps1:15:11:15:20 | userinput | provenance | | -| test.ps1:157:46:157:51 | input | test.ps1:21:11:21:20 | userinput | provenance | | -| test.ps1:158:46:158:51 | input | test.ps1:27:11:27:20 | userinput | provenance | | -| test.ps1:159:46:159:51 | input | test.ps1:33:11:33:20 | userinput | provenance | | -| test.ps1:160:46:160:51 | input | test.ps1:39:11:39:20 | userinput | provenance | | -| test.ps1:161:46:161:51 | input | test.ps1:45:11:45:20 | userinput | provenance | | -| test.ps1:163:48:163:53 | input | test.ps1:73:11:73:20 | userinput | provenance | | -| test.ps1:164:48:164:53 | input | test.ps1:80:11:80:20 | userinput | provenance | | -| test.ps1:165:48:165:53 | input | test.ps1:87:11:87:20 | userinput | provenance | | -| test.ps1:166:41:166:46 | input | test.ps1:94:11:94:20 | userinput | provenance | | -| test.ps1:167:41:167:46 | input | test.ps1:104:11:104:20 | userinput | provenance | | -| test.ps1:168:36:168:41 | input | test.ps1:114:11:114:20 | userinput | provenance | | -| test.ps1:169:36:169:41 | input | test.ps1:121:11:121:20 | userinput | provenance | | -| test.ps1:170:36:170:41 | input | test.ps1:129:11:129:20 | userinput | provenance | | -| test.ps1:172:42:172:47 | input | test.ps1:136:11:136:20 | userinput | provenance | | -| test.ps1:173:42:173:47 | input | test.ps1:144:11:144:20 | userinput | provenance | | -| test.ps1:214:5:214:6 | o | test.ps1:217:7:217:10 | $o | provenance | | -| test.ps1:214:10:214:32 | Call to read-host | test.ps1:214:5:214:6 | o | provenance | Src:MaD:0 | -| test.ps1:225:5:225:10 | input | test.ps1:226:5:226:21 | env:bar | provenance | | -| test.ps1:225:5:225:10 | input | test.ps1:226:5:226:21 | env:bar | provenance | | -| test.ps1:225:14:225:36 | Call to read-host | test.ps1:225:5:225:10 | input | provenance | Src:MaD:0 | -| test.ps1:225:14:225:36 | Call to read-host | test.ps1:225:5:225:10 | input | provenance | Src:MaD:0 | -| test.ps1:226:5:226:21 | env:bar | test.ps1:228:5:228:6 | y | provenance | | -| test.ps1:228:5:228:6 | y | test.ps1:229:7:229:10 | $y | provenance | | +| test.ps1:153:11:153:20 | userinput | test.ps1:154:23:154:52 | Get-Process -Name $UserInput | provenance | | +| test.ps1:159:11:159:20 | userinput | test.ps1:160:29:160:38 | UserInput | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:166:46:166:51 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:167:46:167:51 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:168:46:168:51 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:169:46:169:51 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:170:46:170:51 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:171:46:171:51 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:172:46:172:51 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:173:46:173:51 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:175:48:175:53 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:176:48:176:53 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:177:48:177:53 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:178:41:178:46 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:179:41:179:46 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:180:36:180:41 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:181:36:181:41 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:182:36:182:41 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:184:42:184:47 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:185:42:185:47 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:186:58:186:63 | input | provenance | | +| test.ps1:164:1:164:6 | input | test.ps1:187:41:187:46 | input | provenance | | +| test.ps1:164:10:164:32 | Call to read-host | test.ps1:164:1:164:6 | input | provenance | Src:MaD:0 | +| test.ps1:166:46:166:51 | input | test.ps1:3:11:3:20 | userinput | provenance | | +| test.ps1:167:46:167:51 | input | test.ps1:9:11:9:20 | userinput | provenance | | +| test.ps1:168:46:168:51 | input | test.ps1:15:11:15:20 | userinput | provenance | | +| test.ps1:169:46:169:51 | input | test.ps1:21:11:21:20 | userinput | provenance | | +| test.ps1:170:46:170:51 | input | test.ps1:27:11:27:20 | userinput | provenance | | +| test.ps1:171:46:171:51 | input | test.ps1:33:11:33:20 | userinput | provenance | | +| test.ps1:172:46:172:51 | input | test.ps1:39:11:39:20 | userinput | provenance | | +| test.ps1:173:46:173:51 | input | test.ps1:45:11:45:20 | userinput | provenance | | +| test.ps1:175:48:175:53 | input | test.ps1:73:11:73:20 | userinput | provenance | | +| test.ps1:176:48:176:53 | input | test.ps1:80:11:80:20 | userinput | provenance | | +| test.ps1:177:48:177:53 | input | test.ps1:87:11:87:20 | userinput | provenance | | +| test.ps1:178:41:178:46 | input | test.ps1:94:11:94:20 | userinput | provenance | | +| test.ps1:179:41:179:46 | input | test.ps1:104:11:104:20 | userinput | provenance | | +| test.ps1:180:36:180:41 | input | test.ps1:114:11:114:20 | userinput | provenance | | +| test.ps1:181:36:181:41 | input | test.ps1:121:11:121:20 | userinput | provenance | | +| test.ps1:182:36:182:41 | input | test.ps1:129:11:129:20 | userinput | provenance | | +| test.ps1:184:42:184:47 | input | test.ps1:136:11:136:20 | userinput | provenance | | +| test.ps1:185:42:185:47 | input | test.ps1:144:11:144:20 | userinput | provenance | | +| test.ps1:186:58:186:63 | input | test.ps1:153:11:153:20 | userinput | provenance | | +| test.ps1:187:41:187:46 | input | test.ps1:159:11:159:20 | userinput | provenance | | +| test.ps1:254:5:254:6 | o | test.ps1:257:7:257:10 | $o | provenance | | +| test.ps1:254:10:254:32 | Call to read-host | test.ps1:254:5:254:6 | o | provenance | Src:MaD:0 | +| test.ps1:265:5:265:10 | input | test.ps1:266:5:266:21 | env:bar | provenance | | +| test.ps1:265:5:265:10 | input | test.ps1:266:5:266:21 | env:bar | provenance | | +| test.ps1:265:14:265:36 | Call to read-host | test.ps1:265:5:265:10 | input | provenance | Src:MaD:0 | +| test.ps1:265:14:265:36 | Call to read-host | test.ps1:265:5:265:10 | input | provenance | Src:MaD:0 | +| test.ps1:266:5:266:21 | env:bar | test.ps1:268:5:268:6 | y | provenance | | +| test.ps1:268:5:268:6 | y | test.ps1:269:7:269:10 | $y | provenance | | nodes | test.ps1:3:11:3:20 | userinput | semmle.label | userinput | | test.ps1:4:23:4:52 | Get-Process -Name $UserInput | semmle.label | Get-Process -Name $UserInput | @@ -101,54 +107,62 @@ nodes | test.ps1:139:50:139:59 | UserInput | semmle.label | UserInput | | test.ps1:144:11:144:20 | userinput | semmle.label | userinput | | test.ps1:147:63:147:72 | UserInput | semmle.label | UserInput | -| test.ps1:152:1:152:6 | input | semmle.label | input | -| test.ps1:152:10:152:32 | Call to read-host | semmle.label | Call to read-host | -| test.ps1:154:46:154:51 | input | semmle.label | input | -| test.ps1:155:46:155:51 | input | semmle.label | input | -| test.ps1:156:46:156:51 | input | semmle.label | input | -| test.ps1:157:46:157:51 | input | semmle.label | input | -| test.ps1:158:46:158:51 | input | semmle.label | input | -| test.ps1:159:46:159:51 | input | semmle.label | input | -| test.ps1:160:46:160:51 | input | semmle.label | input | -| test.ps1:161:46:161:51 | input | semmle.label | input | -| test.ps1:163:48:163:53 | input | semmle.label | input | -| test.ps1:164:48:164:53 | input | semmle.label | input | -| test.ps1:165:48:165:53 | input | semmle.label | input | -| test.ps1:166:41:166:46 | input | semmle.label | input | -| test.ps1:167:41:167:46 | input | semmle.label | input | -| test.ps1:168:36:168:41 | input | semmle.label | input | -| test.ps1:169:36:169:41 | input | semmle.label | input | -| test.ps1:170:36:170:41 | input | semmle.label | input | -| test.ps1:172:42:172:47 | input | semmle.label | input | -| test.ps1:173:42:173:47 | input | semmle.label | input | -| test.ps1:214:5:214:6 | o | semmle.label | o | -| test.ps1:214:10:214:32 | Call to read-host | semmle.label | Call to read-host | -| test.ps1:217:7:217:10 | $o | semmle.label | $o | -| test.ps1:225:5:225:10 | input | semmle.label | input | -| test.ps1:225:5:225:10 | input | semmle.label | input | -| test.ps1:225:14:225:36 | Call to read-host | semmle.label | Call to read-host | -| test.ps1:226:5:226:21 | env:bar | semmle.label | env:bar | -| test.ps1:228:5:228:6 | y | semmle.label | y | -| test.ps1:229:7:229:10 | $y | semmle.label | $y | +| test.ps1:153:11:153:20 | userinput | semmle.label | userinput | +| test.ps1:154:23:154:52 | Get-Process -Name $UserInput | semmle.label | Get-Process -Name $UserInput | +| test.ps1:159:11:159:20 | userinput | semmle.label | userinput | +| test.ps1:160:29:160:38 | UserInput | semmle.label | UserInput | +| test.ps1:164:1:164:6 | input | semmle.label | input | +| test.ps1:164:10:164:32 | Call to read-host | semmle.label | Call to read-host | +| test.ps1:166:46:166:51 | input | semmle.label | input | +| test.ps1:167:46:167:51 | input | semmle.label | input | +| test.ps1:168:46:168:51 | input | semmle.label | input | +| test.ps1:169:46:169:51 | input | semmle.label | input | +| test.ps1:170:46:170:51 | input | semmle.label | input | +| test.ps1:171:46:171:51 | input | semmle.label | input | +| test.ps1:172:46:172:51 | input | semmle.label | input | +| test.ps1:173:46:173:51 | input | semmle.label | input | +| test.ps1:175:48:175:53 | input | semmle.label | input | +| test.ps1:176:48:176:53 | input | semmle.label | input | +| test.ps1:177:48:177:53 | input | semmle.label | input | +| test.ps1:178:41:178:46 | input | semmle.label | input | +| test.ps1:179:41:179:46 | input | semmle.label | input | +| test.ps1:180:36:180:41 | input | semmle.label | input | +| test.ps1:181:36:181:41 | input | semmle.label | input | +| test.ps1:182:36:182:41 | input | semmle.label | input | +| test.ps1:184:42:184:47 | input | semmle.label | input | +| test.ps1:185:42:185:47 | input | semmle.label | input | +| test.ps1:186:58:186:63 | input | semmle.label | input | +| test.ps1:187:41:187:46 | input | semmle.label | input | +| test.ps1:254:5:254:6 | o | semmle.label | o | +| test.ps1:254:10:254:32 | Call to read-host | semmle.label | Call to read-host | +| test.ps1:257:7:257:10 | $o | semmle.label | $o | +| test.ps1:265:5:265:10 | input | semmle.label | input | +| test.ps1:265:5:265:10 | input | semmle.label | input | +| test.ps1:265:14:265:36 | Call to read-host | semmle.label | Call to read-host | +| test.ps1:266:5:266:21 | env:bar | semmle.label | env:bar | +| test.ps1:268:5:268:6 | y | semmle.label | y | +| test.ps1:269:7:269:10 | $y | semmle.label | $y | subpaths #select -| test.ps1:4:23:4:52 | Get-Process -Name $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:4:23:4:52 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:10:9:10:38 | Get-Process -Name $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:10:9:10:38 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:16:50:16:79 | Get-Process -Name $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:16:50:16:79 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:22:41:22:70 | Get-Process -Name $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:22:41:22:70 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:28:38:28:67 | Get-Process -Name $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:28:38:28:67 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:34:14:34:46 | public class Foo { $UserInput } | test.ps1:152:10:152:32 | Call to read-host | test.ps1:34:14:34:46 | public class Foo { $UserInput } | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:40:30:40:62 | public class Foo { $UserInput } | test.ps1:152:10:152:32 | Call to read-host | test.ps1:40:30:40:62 | public class Foo { $UserInput } | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:48:30:48:34 | code | test.ps1:152:10:152:32 | Call to read-host | test.ps1:48:30:48:34 | code | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:75:25:75:54 | Get-Process -Name $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:75:25:75:54 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:82:16:82:45 | Get-Process -Name $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:82:16:82:45 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:89:12:89:28 | ping $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:89:12:89:28 | ping $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:98:33:98:62 | Get-Process -Name $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:98:33:98:62 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:108:58:108:87 | Get-Process -Name $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:108:58:108:87 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:116:34:116:43 | UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:116:34:116:43 | UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:123:28:123:37 | UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:123:28:123:37 | UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:131:28:131:37 | UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:131:28:131:37 | UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:139:50:139:59 | UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:139:50:139:59 | UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:147:63:147:72 | UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:147:63:147:72 | UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value | -| test.ps1:217:7:217:10 | $o | test.ps1:214:10:214:32 | Call to read-host | test.ps1:217:7:217:10 | $o | This command depends on a $@. | test.ps1:214:10:214:32 | Call to read-host | user-provided value | -| test.ps1:229:7:229:10 | $y | test.ps1:225:14:225:36 | Call to read-host | test.ps1:229:7:229:10 | $y | This command depends on a $@. | test.ps1:225:14:225:36 | Call to read-host | user-provided value | +| test.ps1:4:23:4:52 | Get-Process -Name $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:4:23:4:52 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:10:9:10:38 | Get-Process -Name $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:10:9:10:38 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:16:50:16:79 | Get-Process -Name $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:16:50:16:79 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:22:41:22:70 | Get-Process -Name $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:22:41:22:70 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:28:38:28:67 | Get-Process -Name $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:28:38:28:67 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:34:14:34:46 | public class Foo { $UserInput } | test.ps1:164:10:164:32 | Call to read-host | test.ps1:34:14:34:46 | public class Foo { $UserInput } | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:40:30:40:62 | public class Foo { $UserInput } | test.ps1:164:10:164:32 | Call to read-host | test.ps1:40:30:40:62 | public class Foo { $UserInput } | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:48:30:48:34 | code | test.ps1:164:10:164:32 | Call to read-host | test.ps1:48:30:48:34 | code | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:75:25:75:54 | Get-Process -Name $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:75:25:75:54 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:82:16:82:45 | Get-Process -Name $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:82:16:82:45 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:89:12:89:28 | ping $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:89:12:89:28 | ping $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:98:33:98:62 | Get-Process -Name $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:98:33:98:62 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:108:58:108:87 | Get-Process -Name $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:108:58:108:87 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:116:34:116:43 | UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:116:34:116:43 | UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:123:28:123:37 | UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:123:28:123:37 | UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:131:28:131:37 | UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:131:28:131:37 | UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:139:50:139:59 | UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:139:50:139:59 | UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:147:63:147:72 | UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:147:63:147:72 | UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:154:23:154:52 | Get-Process -Name $UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:154:23:154:52 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:160:29:160:38 | UserInput | test.ps1:164:10:164:32 | Call to read-host | test.ps1:160:29:160:38 | UserInput | This command depends on a $@. | test.ps1:164:10:164:32 | Call to read-host | user-provided value | +| test.ps1:257:7:257:10 | $o | test.ps1:254:10:254:32 | Call to read-host | test.ps1:257:7:257:10 | $o | This command depends on a $@. | test.ps1:254:10:254:32 | Call to read-host | user-provided value | +| test.ps1:269:7:269:10 | $y | test.ps1:265:14:265:36 | Call to read-host | test.ps1:269:7:269:10 | $y | This command depends on a $@. | test.ps1:265:14:265:36 | Call to read-host | user-provided value | diff --git a/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjectionCritical.expected b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjectionCritical.expected new file mode 100644 index 000000000000..b55cefb43dc7 --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjectionCritical.expected @@ -0,0 +1,8 @@ +edges +| test.ps1:153:11:153:20 | userinput | test.ps1:154:23:154:52 | Get-Process -Name $UserInput | provenance | | +nodes +| test.ps1:153:11:153:20 | userinput | semmle.label | userinput | +| test.ps1:154:23:154:52 | Get-Process -Name $UserInput | semmle.label | Get-Process -Name $UserInput | +subpaths +#select +| test.ps1:154:23:154:52 | Get-Process -Name $UserInput | test.ps1:153:11:153:20 | userinput | test.ps1:154:23:154:52 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:153:11:153:20 | userinput | param to CmdletBinding function | diff --git a/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjectionCritical.qlref b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjectionCritical.qlref new file mode 100644 index 000000000000..5c564e56c99b --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjectionCritical.qlref @@ -0,0 +1 @@ +queries/security/cwe-078/CommandInjectionCritical.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 index b69bee0c18b8..f79decd563c7 100644 --- a/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/test.ps1 +++ b/powershell/ql/test/query-tests/security/cwe-078/CommandInjection/test.ps1 @@ -147,6 +147,18 @@ function Invoke-ExpandStringInjection2 $executionContext.SessionState.InvokeCommand.ExpandString($UserInput) # BAD } +function Invoke-InvokeExpressionInjectionCmdletBinding +{ + [CmdletBinding()] + param($UserInput) + Invoke-Expression "Get-Process -Name $UserInput" # BAD +} + +function Invoke-StartProcessInjection +{ + param($UserInput) + Start-Process -FilePath $UserInput # BAD +} $input = Read-Host "enter input" @@ -171,6 +183,17 @@ Invoke-MethodInjection3 -UserInput $input Invoke-PropertyInjection -UserInput $input Invoke-ExpandStringInjection1 -UserInput $input Invoke-ExpandStringInjection2 -UserInput $input +Invoke-InvokeExpressionInjectionCmdletBinding -userInput $input +Invoke-StartProcessInjection -UserInput $input + +function Get-NugetHardcoded +{ + Invoke-WebRequest "https://somehardcodedwebsite.org/somefile.exe" -OutFile $webRequestResultSafe + return $webRequestResultSafe +} + +$nugetPathSafe = Get-NugetHardcoded +. $nugetPathSafe #typed input function Invoke-InvokeExpressionInjectionSafe1 @@ -204,10 +227,27 @@ function Invoke-InvokeExpressionInjectionSafe4 Invoke-Expression "Get-Process -Name $UserInputClean" } +#ValidatePattern Attribute +function Invoke-InvokeExpressionInjectionSafe5 +{ + param( + [ValidateScript({ + if ($_ -eq "GoodValue") { + $true + } else { + throw "$_ is invalid." + } + })] + $UserInput + ) + Invoke-Expression "Get-Process -Name $UserInput" +} + Invoke-InvokeExpressionInjectionSafe1 -UserInput $input Invoke-InvokeExpressionInjectionSafe2 -UserInput $input Invoke-InvokeExpressionInjectionSafe3 -UserInput $input Invoke-InvokeExpressionInjectionSafe4 -UserInput $input +Invoke-InvokeExpressionInjectionSafe5 -UserInput $input function false-positive-in-call-operator($d) {