From a1bd3a581e1730e28469b09fbe6246c782324525 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 1 Oct 2025 19:52:40 +0100 Subject: [PATCH 1/2] PS: Convert tests to do string interpolation. Notice the missing result. --- .../security/cwe-089/SqlInjection.expected | 13 +++---------- .../ql/test/query-tests/security/cwe-089/test.ps1 | 4 ++-- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected index b99d45072abb..e14f07432624 100644 --- a/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected +++ b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected @@ -3,16 +3,13 @@ edges | test.ps1:1:1:1:10 | userinput | test.ps1:8:1:8:6 | query | provenance | | | test.ps1:1:1:1:10 | userinput | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | | | test.ps1:1:1:1:10 | userinput | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | | -| test.ps1:1:1:1:10 | userinput | test.ps1:78:13:78:22 | userinput | provenance | | -| test.ps1:1:1:1:10 | userinput | test.ps1:128:28:128:37 | userinput | provenance | | +| test.ps1:1:1:1:10 | userinput | test.ps1:78:13:78:59 | SELECT * FROM Customers WHERE id = $userinput | provenance | | | test.ps1:1:14:1:45 | Call to read-host | test.ps1:1:1:1:10 | userinput | provenance | Src:MaD:0 | | test.ps1:4:1:4:6 | query | test.ps1:5:72:5:77 | query | provenance | | | test.ps1:8:1:8:6 | query | test.ps1:9:72:9:77 | query | provenance | | | test.ps1:72:1:72:11 | QueryConn2 [element Query] | test.ps1:81:15:81:25 | QueryConn2 | provenance | | | test.ps1:72:15:79:1 | ${...} [element Query] | test.ps1:72:1:72:11 | QueryConn2 [element Query] | provenance | | -| test.ps1:78:13:78:22 | userinput | test.ps1:72:15:79:1 | ${...} [element Query] | provenance | | -| test.ps1:121:9:121:56 | unvalidated | test.ps1:125:92:125:103 | unvalidated | provenance | | -| test.ps1:128:28:128:37 | userinput | test.ps1:121:9:121:56 | unvalidated | provenance | | +| test.ps1:78:13:78:59 | SELECT * FROM Customers WHERE id = $userinput | test.ps1:72:15:79:1 | ${...} [element Query] | provenance | | nodes | test.ps1:1:1:1:10 | userinput | semmle.label | userinput | | test.ps1:1:14:1:45 | Call to read-host | semmle.label | Call to read-host | @@ -24,11 +21,8 @@ nodes | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | semmle.label | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | | test.ps1:72:1:72:11 | QueryConn2 [element Query] | semmle.label | QueryConn2 [element Query] | | test.ps1:72:15:79:1 | ${...} [element Query] | semmle.label | ${...} [element Query] | -| test.ps1:78:13:78:22 | userinput | semmle.label | userinput | +| test.ps1:78:13:78:59 | SELECT * FROM Customers WHERE id = $userinput | semmle.label | SELECT * FROM Customers WHERE id = $userinput | | test.ps1:81:15:81:25 | QueryConn2 | semmle.label | QueryConn2 | -| test.ps1:121:9:121:56 | unvalidated | semmle.label | unvalidated | -| test.ps1:125:92:125:103 | unvalidated | semmle.label | unvalidated | -| test.ps1:128:28:128:37 | userinput | semmle.label | userinput | subpaths #select | test.ps1:5:72:5:77 | query | test.ps1:1:14:1:45 | Call to read-host | test.ps1:5:72:5:77 | query | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | @@ -36,4 +30,3 @@ subpaths | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:1:14:1:45 | Call to read-host | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:1:14:1:45 | Call to read-host | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | | test.ps1:81:15:81:25 | QueryConn2 | test.ps1:1:14:1:45 | Call to read-host | test.ps1:81:15:81:25 | QueryConn2 | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | -| test.ps1:125:92:125:103 | unvalidated | test.ps1:1:14:1:45 | Call to read-host | test.ps1:125:92:125:103 | unvalidated | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | diff --git a/powershell/ql/test/query-tests/security/cwe-089/test.ps1 b/powershell/ql/test/query-tests/security/cwe-089/test.ps1 index e97ba8d3676c..2c023aa26f36 100644 --- a/powershell/ql/test/query-tests/security/cwe-089/test.ps1 +++ b/powershell/ql/test/query-tests/security/cwe-089/test.ps1 @@ -75,7 +75,7 @@ $QueryConn2 = @{ Username = "MyUserName" Password = "MyPassword" ConnectionTimeout = 0 - Query = $userinput + Query = "SELECT * FROM Customers WHERE id = $userinput" } Invoke-Sqlcmd @QueryConn2 # BAD @@ -122,7 +122,7 @@ function With-Validation() { ) Invoke-Sqlcmd -unknown $userinput -ServerInstance "MyServer" -Database "MyDatabase" -q $validated # GOOD - Invoke-Sqlcmd -unknown $userinput -ServerInstance "MyServer" -Database "MyDatabase" -q $unvalidated # BAD + Invoke-Sqlcmd -unknown $userinput -ServerInstance "MyServer" -Database "MyDatabase" -q "SELECT * FROM Customers where id = $($unvalidated)" # BAD } With-Validation $userinput $userinput From bfb10a216b59c85fe295ea54888dea699951e590 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 1 Oct 2025 19:54:09 +0100 Subject: [PATCH 2/2] PS: Add flow through string subexpressions and accept test changes. --- .../code/powershell/controlflow/CfgNodes.qll | 2 +- .../internal/TaintTrackingPrivate.qll | 49 ++++++++++++++++--- .../dataflow/internal/TaintTrackingPublic.qll | 8 ++- .../security/cwe-089/SqlInjection.expected | 7 +++ 4 files changed, 55 insertions(+), 11 deletions(-) diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll index 8a3589afea8f..06de5ac7bef4 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll @@ -965,7 +965,7 @@ module ExprNodes { override ExpandableSubExpr getExpr() { result = e } - ExprCfgNode getSubExpr() { e.hasCfgChild(e.getExpr(), this, result) } + StmtNodes::StmtBlockCfgNode getSubExpr() { e.hasCfgChild(e.getExpr(), this, result) } } private class UsingExprChildMapping extends ExprChildMapping, UsingExpr { diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPrivate.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPrivate.qll index 2a61ba2cf210..e40a6415e97c 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPrivate.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPrivate.qll @@ -4,6 +4,7 @@ private import TaintTrackingPublic private import semmle.code.powershell.Cfg private import semmle.code.powershell.dataflow.DataFlow private import FlowSummaryImpl as FlowSummaryImpl +private import PipelineReturns as PipelineReturns /** * Holds if `node` should be a sanitizer in all global taint flow configurations @@ -21,6 +22,44 @@ predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) c.isAnyPositional() } +private module ExpandableSubExprInput implements PipelineReturns::InputSig { + additional predicate isSourceImpl( + CfgNodes::ExprNodes::ExpandableSubExprCfgNode expandableSubExpr, CfgNodes::AstCfgNode source + ) { + source = expandableSubExpr.getSubExpr() + } + + predicate isSource(CfgNodes::AstCfgNode source) { isSourceImpl(_, source) } +} + +/** + * Holds if `nodeFrom` flows to `nodeTo` via string interpolation. + */ +predicate stringInterpolationTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { + // flow from $x to "Hello $x" + exists(CfgNodes::ExprNodes::ExpandableStringExprCfgNode es | + nodeFrom.asExpr() = es.getAnExpr() and + nodeTo.asExpr() = es + ) + or + // Flow from $x to $($x) + exists( + CfgNodes::ExprNodes::ExpandableSubExprCfgNode es, + CfgNodes::StmtNodes::StmtBlockCfgNode blockStmt + | + ExpandableSubExprInput::isSourceImpl(es, blockStmt) and + nodeFrom.asExpr() = PipelineReturns::Make::getAReturn(blockStmt) and + nodeTo.asExpr() = es + ) +} + +predicate operationTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { + exists(CfgNodes::ExprNodes::OperationCfgNode op | + op = nodeTo.asExpr() and + op.getAnOperand() = nodeFrom.asExpr() + ) +} + cached private module Cached { private import semmle.code.powershell.dataflow.internal.DataFlowImplCommon as DataFlowImplCommon @@ -36,16 +75,10 @@ private module Cached { predicate defaultAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo, string model) { ( // Flow from an operand to an operation - exists(CfgNodes::ExprNodes::OperationCfgNode op | - op = nodeTo.asExpr() and - op.getAnOperand() = nodeFrom.asExpr() - ) + operationTaintStep(nodeFrom, nodeTo) or // Flow through string interpolation - exists(CfgNodes::ExprNodes::ExpandableStringExprCfgNode es | - nodeFrom.asExpr() = es.getAnExpr() and - nodeTo.asExpr() = es - ) + stringInterpolationTaintStep(nodeFrom, nodeTo) or // Although flow through collections is modeled precisely using stores/reads, we still // allow flow out of a _tainted_ collection. This is needed in order to support taint- diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPublic.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPublic.qll index 17e071e783c2..253c89b628d3 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPublic.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPublic.qll @@ -1,5 +1,5 @@ private import powershell -private import TaintTrackingPrivate +private import TaintTrackingPrivate as Private private import semmle.code.powershell.Cfg private import semmle.code.powershell.dataflow.DataFlow @@ -19,4 +19,8 @@ predicate localExprTaint(CfgNodes::ExprCfgNode e1, CfgNodes::ExprCfgNode e2) { localTaintStep*(DataFlow::exprNode(e1), DataFlow::exprNode(e2)) } -predicate localTaintStep = localTaintStepCached/2; +predicate stringInterpolationTaintStep = Private::stringInterpolationTaintStep/2; + +predicate operationTaintStep = Private::operationTaintStep/2; + +predicate localTaintStep = Private::localTaintStepCached/2; diff --git a/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected index e14f07432624..bacce10af7ea 100644 --- a/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected +++ b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected @@ -4,12 +4,15 @@ edges | test.ps1:1:1:1:10 | userinput | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | | | test.ps1:1:1:1:10 | userinput | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | | | test.ps1:1:1:1:10 | userinput | test.ps1:78:13:78:59 | SELECT * FROM Customers WHERE id = $userinput | provenance | | +| test.ps1:1:1:1:10 | userinput | test.ps1:128:28:128:37 | userinput | provenance | | | test.ps1:1:14:1:45 | Call to read-host | test.ps1:1:1:1:10 | userinput | provenance | Src:MaD:0 | | test.ps1:4:1:4:6 | query | test.ps1:5:72:5:77 | query | provenance | | | test.ps1:8:1:8:6 | query | test.ps1:9:72:9:77 | query | provenance | | | test.ps1:72:1:72:11 | QueryConn2 [element Query] | test.ps1:81:15:81:25 | QueryConn2 | provenance | | | test.ps1:72:15:79:1 | ${...} [element Query] | test.ps1:72:1:72:11 | QueryConn2 [element Query] | provenance | | | test.ps1:78:13:78:59 | SELECT * FROM Customers WHERE id = $userinput | test.ps1:72:15:79:1 | ${...} [element Query] | provenance | | +| test.ps1:121:9:121:56 | unvalidated | test.ps1:125:92:125:143 | SELECT * FROM Customers where id = $($unvalidated) | provenance | | +| test.ps1:128:28:128:37 | userinput | test.ps1:121:9:121:56 | unvalidated | provenance | | nodes | test.ps1:1:1:1:10 | userinput | semmle.label | userinput | | test.ps1:1:14:1:45 | Call to read-host | semmle.label | Call to read-host | @@ -23,6 +26,9 @@ nodes | test.ps1:72:15:79:1 | ${...} [element Query] | semmle.label | ${...} [element Query] | | test.ps1:78:13:78:59 | SELECT * FROM Customers WHERE id = $userinput | semmle.label | SELECT * FROM Customers WHERE id = $userinput | | test.ps1:81:15:81:25 | QueryConn2 | semmle.label | QueryConn2 | +| test.ps1:121:9:121:56 | unvalidated | semmle.label | unvalidated | +| test.ps1:125:92:125:143 | SELECT * FROM Customers where id = $($unvalidated) | semmle.label | SELECT * FROM Customers where id = $($unvalidated) | +| test.ps1:128:28:128:37 | userinput | semmle.label | userinput | subpaths #select | test.ps1:5:72:5:77 | query | test.ps1:1:14:1:45 | Call to read-host | test.ps1:5:72:5:77 | query | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | @@ -30,3 +36,4 @@ subpaths | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:1:14:1:45 | Call to read-host | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:1:14:1:45 | Call to read-host | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | | test.ps1:81:15:81:25 | QueryConn2 | test.ps1:1:14:1:45 | Call to read-host | test.ps1:81:15:81:25 | QueryConn2 | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | +| test.ps1:125:92:125:143 | SELECT * FROM Customers where id = $($unvalidated) | test.ps1:1:14:1:45 | Call to read-host | test.ps1:125:92:125:143 | SELECT * FROM Customers where id = $($unvalidated) | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin |