From f39d08ecfa305cbed8aede00da1280a450589d0a Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Jul 2025 13:45:09 +0100 Subject: [PATCH 1/6] PS: Fix spelling. --- .../semmle/code/powershell/dataflow/internal/DataFlowPublic.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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..5b8694263f4 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll @@ -540,7 +540,7 @@ 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; From 75d37dceaddeeddccec8f7fe2f98c99dc90718fa Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Jul 2025 13:46:44 +0100 Subject: [PATCH 2/6] PS: Add false positive. --- .../cwe-078/CommandInjection/CommandInjection.expected | 4 ++++ .../security/cwe-078/CommandInjection/test.ps1 | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) 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..7eace546a2c 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:215:16:215:19 | $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:215:16:215:19 | $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:215:16:215:19 | $o | test.ps1:214:10:214:32 | Call to read-host | test.ps1:215:16:215:19 | $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..689b3b7c9bb 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,10 @@ 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 [FALSE POSITIVE] +} \ No newline at end of file From 5f07641bd391865375bed3023f9128e3550190d7 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Jul 2025 13:56:27 +0100 Subject: [PATCH 3/6] PS: Fix false positive by fixing the 'getCommand' predicates in 'CallOperatorCfgNode' and 'CallOperator'. Also fix 'DotSourcingOperator::getPath' while here. --- .../ql/lib/semmle/code/powershell/ast/internal/Command.qll | 4 ++-- .../ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll | 2 +- .../cwe-078/CommandInjection/CommandInjection.expected | 4 ---- .../query-tests/security/cwe-078/CommandInjection/test.ps1 | 2 +- 4 files changed, 4 insertions(+), 8 deletions(-) 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..c0f8e327987 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 getPath() { 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..8710e66e85b 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll @@ -605,7 +605,7 @@ module ExprNodes { override CallOperator getExpr() { result = e } - ExprCfgNode getCommand() { result = this.getArgument(0) } + ExprCfgNode getCommand() { result = this.getCallee() } } private class ToStringCallChildmapping extends CallExprChildMapping instanceof ToStringCall { 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 7eace546a2c..40bbaeed74a 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,7 +53,6 @@ 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:215:16:215:19 | $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 | @@ -110,8 +109,6 @@ 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:215:16:215:19 | $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 | @@ -132,4 +129,3 @@ 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:215:16:215:19 | $o | test.ps1:214:10:214:32 | Call to read-host | test.ps1:215:16:215:19 | $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 689b3b7c9bb..c8260f9c5f5 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 @@ -212,5 +212,5 @@ Invoke-InvokeExpressionInjectionSafe4 -UserInput $input function false-positive-in-call-operator($d) { $o = Read-Host "enter input" - & unzip -o "$o" -d $d # GOOD [FALSE POSITIVE] + & unzip -o "$o" -d $d # GOOD } \ No newline at end of file From 670ad745ca9a87a76e99b00e0e8f868bd233e583 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Jul 2025 14:32:42 +0100 Subject: [PATCH 4/6] PS: Add false negative. --- .../test/query-tests/security/cwe-078/CommandInjection/test.ps1 | 2 ++ 1 file changed, 2 insertions(+) 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 c8260f9c5f5..dd4b10c7859 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 @@ -213,4 +213,6 @@ function false-positive-in-call-operator($d) { $o = Read-Host "enter input" & unzip -o "$o" -d $d # GOOD + + . "$o" # BAD [NOT DETECTED] } \ No newline at end of file From 205d2e58ff282842337c1a3152f5fa596a05094a Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Jul 2025 14:22:23 +0100 Subject: [PATCH 5/6] PS: Add dot sourcing as a sink. --- .../code/powershell/ast/internal/Command.qll | 2 +- .../code/powershell/controlflow/CfgNodes.qll | 15 +++++++++++++++ .../dataflow/internal/DataFlowPublic.qll | 9 ++++++++- .../security/CommandInjectionCustomizations.qll | 4 +++- 4 files changed, 27 insertions(+), 3 deletions(-) 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 c0f8e327987..a2ae0bde31e 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/Command.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/Command.qll @@ -83,7 +83,7 @@ class CallOperator extends CmdCall { class DotSourcingOperator extends CmdCall { DotSourcingOperator() { getRawAst(this) instanceof Raw::DotSourcingOperator } - Expr getPath() { result = this.getCallee() } + 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 8710e66e85b..b4401861aae 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll @@ -608,6 +608,21 @@ module ExprNodes { 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 { override predicate relevantChild(Ast child) { super.relevantChild(child) } } 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 5b8694263f4..7f8f6109b0b 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll @@ -544,7 +544,14 @@ class CallNode extends ExprNode { 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" } From 72af80010127200267dda70e04e536b40e571d75 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Jul 2025 14:22:35 +0100 Subject: [PATCH 6/6] PS: Accept test changes. --- .../cwe-078/CommandInjection/CommandInjection.expected | 4 ++++ .../query-tests/security/cwe-078/CommandInjection/test.ps1 | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) 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 dd4b10c7859..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 @@ -214,5 +214,5 @@ function false-positive-in-call-operator($d) $o = Read-Host "enter input" & unzip -o "$o" -d $d # GOOD - . "$o" # BAD [NOT DETECTED] + . "$o" # BAD } \ No newline at end of file