diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/Command.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/Command.qll index c1984d5286e..a2ae0bde31e 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/Command.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/Command.qll @@ -76,14 +76,14 @@ class CmdCall extends CallExpr, TCmd { class CallOperator extends CmdCall { CallOperator() { getRawAst(this) instanceof Raw::CallOperator } - Expr getCommand() { result = this.getArgument(0) } + Expr getCommand() { result = this.getCallee() } } /** A call to the dot-sourcing `.`. */ class DotSourcingOperator extends CmdCall { DotSourcingOperator() { getRawAst(this) instanceof Raw::DotSourcingOperator } - Expr getPath() { result = this.getArgument(0) } + Expr getCommand() { result = this.getCallee() } } class JoinPath extends CmdCall { diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll index dcab50e1f02..b4401861aae 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll @@ -605,7 +605,22 @@ module ExprNodes { override CallOperator getExpr() { result = e } - ExprCfgNode getCommand() { result = this.getArgument(0) } + ExprCfgNode getCommand() { result = this.getCallee() } + } + + private class DotSourcingOperatorChildMapping extends CallExprChildMapping instanceof DotSourcingOperator + { + override predicate relevantChild(Ast child) { super.relevantChild(child) } + } + + class DotSourcingOperatorCfgNode extends CallExprCfgNode { + override string getAPrimaryQlClass() { result = "DotSourcingOperatorCfgNode" } + + override DotSourcingOperatorChildMapping e; + + override DotSourcingOperator getExpr() { result = e } + + ExprCfgNode getCommand() { result = this.getCallee() } } private class ToStringCallChildmapping extends CallExprChildMapping instanceof ToStringCall { diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll index e8e522cc494..7f8f6109b0b 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll @@ -540,11 +540,18 @@ class CallNode extends ExprNode { Node getCallee() { result.asExpr() = call.getCallee() } } -/** A call to operator `&`, viwed as a node in a data flow graph. */ +/** A call to operator `&`, viewed as a node in a data flow graph. */ class CallOperatorNode extends CallNode { override CfgNodes::ExprNodes::CallOperatorCfgNode call; - Node getCommand() { result.asExpr() = call.getCommand() } // TODO: Alternatively, we could remap calls to & as command expressions. + Node getCommand() { result.asExpr() = call.getCommand() } +} + +/** A call to operator `.`, viewed as a node in a data flow graph. */ +class DotSourcingOperatorNode extends CallNode { + override CfgNodes::ExprNodes::DotSourcingOperatorCfgNode call; + + Node getCommand() { result.asExpr() = call.getCommand() } } /** diff --git a/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll b/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll index 2309490cb2a..e88b50ed499 100644 --- a/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll +++ b/powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll @@ -46,8 +46,10 @@ module CommandInjection { call.getAnArgument() = this ) or - // Or the call command itself in case it's a use of operator &. + // Or the call command itself in case it's a use of "operator &" or "operator .". any(DataFlow::CallOperatorNode call).getCommand() = this + or + any(DataFlow::DotSourcingOperatorNode call).getCommand() = this } override string getSinkType() { result = "call to Invoke-Expression" } 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 40bbaeed74a..3e424294511 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 @@ -53,6 +53,7 @@ edges | 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:10:214:32 | Call to read-host | test.ps1:217:7:217:10 | $o | provenance | Src:MaD:0 | 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 | @@ -109,6 +110,8 @@ nodes | 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:10:214:32 | Call to read-host | semmle.label | Call to read-host | +| test.ps1:217:7:217:10 | $o | semmle.label | $o | 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 | @@ -129,3 +132,4 @@ subpaths | 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 | 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 f956795d6cd..2a4aca74de5 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 @@ -207,4 +207,12 @@ function Invoke-InvokeExpressionInjectionSafe4 Invoke-InvokeExpressionInjectionSafe1 -UserInput $input Invoke-InvokeExpressionInjectionSafe2 -UserInput $input Invoke-InvokeExpressionInjectionSafe3 -UserInput $input -Invoke-InvokeExpressionInjectionSafe4 -UserInput $input \ No newline at end of file +Invoke-InvokeExpressionInjectionSafe4 -UserInput $input + +function false-positive-in-call-operator($d) +{ + $o = Read-Host "enter input" + & unzip -o "$o" -d $d # GOOD + + . "$o" # BAD +} \ No newline at end of file