From b34e36984dc2c9bdba8eee44927685a76703165f Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 9 Oct 2024 19:24:14 +0100 Subject: [PATCH 1/4] PS: Add failing tests. --- .../dataflow/returns/test.expected | 6 ++++ .../library-tests/dataflow/returns/test.ps1 | 31 +++++++++++++++++++ .../library-tests/dataflow/returns/test.ql | 13 ++++++++ 3 files changed, 50 insertions(+) create mode 100644 powershell/ql/test/library-tests/dataflow/returns/test.expected create mode 100644 powershell/ql/test/library-tests/dataflow/returns/test.ps1 create mode 100644 powershell/ql/test/library-tests/dataflow/returns/test.ql diff --git a/powershell/ql/test/library-tests/dataflow/returns/test.expected b/powershell/ql/test/library-tests/dataflow/returns/test.expected new file mode 100644 index 00000000000..4e4a41dfc62 --- /dev/null +++ b/powershell/ql/test/library-tests/dataflow/returns/test.expected @@ -0,0 +1,6 @@ +models +edges +nodes +subpaths +testFailures +#select diff --git a/powershell/ql/test/library-tests/dataflow/returns/test.ps1 b/powershell/ql/test/library-tests/dataflow/returns/test.ps1 new file mode 100644 index 00000000000..f54421b3ffe --- /dev/null +++ b/powershell/ql/test/library-tests/dataflow/returns/test.ps1 @@ -0,0 +1,31 @@ +function callSourceOnce { + Source "1" +} + +$x = callSourceOnce +Sink $x # $ MISSING: hasValueFlow=1 + +function callSourceTwice { + Source "2" + Source "3" +} + +$x = callSourceTwice +Sink $x # $ MISSING: hasValueFlow=2 hasValueFlow=3 + +function returnSource1 { + return Source "4" +} + +$x = returnSource1 +Sink $x # $ MISSING: hasValueFlow=4 + +function returnSource2 { + $x = Source "5" + $x + $y = Source "6" + return $y +} + +$x = returnSource2 +Sink $x # $ MISSING: hasValueFlow=5 hasValueFlow=6 \ No newline at end of file diff --git a/powershell/ql/test/library-tests/dataflow/returns/test.ql b/powershell/ql/test/library-tests/dataflow/returns/test.ql new file mode 100644 index 00000000000..9a27a79c447 --- /dev/null +++ b/powershell/ql/test/library-tests/dataflow/returns/test.ql @@ -0,0 +1,13 @@ +/** + * @kind path-problem + */ + +import powershell +import semmle.code.powershell.dataflow.DataFlow +private import TestUtilities.InlineFlowTest +import DefaultFlowTest +import ValueFlow::PathGraph + +from ValueFlow::PathNode source, ValueFlow::PathNode sink +where ValueFlow::flowPath(source, sink) +select sink, source, sink, "$@", source, source.toString() From 54521ad54d15c0da32e5e3b136c4e702c3104af8 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 9 Oct 2024 19:22:14 +0100 Subject: [PATCH 2/4] PS: Add a 'CallNode' helper class. --- .../dataflow/internal/DataFlowPublic.qll | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) 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 76b21030a89..7c2e37eb6ea 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll @@ -33,6 +33,16 @@ class Node extends TNode { Node getASuccessor() { localFlowStep(this, result) } } +/** A control-flow node, viewed as a node in a data flow graph. */ +abstract private class AbstractAstNode extends Node { + CfgNodes::AstCfgNode n; + + /** Gets the control-flow node corresponding to this node. */ + CfgNodes::AstCfgNode getCfgNode() { result = n } +} + +final class AstNode = AbstractAstNode; + /** * An expression, viewed as a node in a data flow graph. * @@ -40,8 +50,8 @@ class Node extends TNode { * to multiple `ExprNode`s, just like it may correspond to multiple * `ControlFlow::Node`s. */ -class ExprNode extends Node, TExprNode { - private CfgNodes::ExprCfgNode n; +class ExprNode extends AbstractAstNode, TExprNode { + override CfgNodes::ExprCfgNode n; ExprNode() { this = TExprNode(n) } @@ -56,8 +66,8 @@ class ExprNode extends Node, TExprNode { * to multiple `StmtNode`s, just like it may correspond to multiple * `ControlFlow::Node`s. */ -class StmtNode extends Node, TStmtNode { - private CfgNodes::StmtCfgNode n; +class StmtNode extends AbstractAstNode, TStmtNode { + override CfgNodes::StmtCfgNode n; StmtNode() { this = TStmtNode(n) } @@ -312,3 +322,12 @@ class ObjectCreationNode extends Node { final CfgNodes::ObjectCreationCfgNode getObjectCreationNode() { result = objectCreation } } + +/** A call, viewed as a node in a data flow graph. */ +class CallNode extends AstNode { + CfgNodes::CallCfgNode call; + + CallNode() { call = this.getCfgNode() } + + CfgNodes::CallCfgNode getCallNode() { result = call } +} From a6b256371f209075323d743c3d0c3e70a02cbbef Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 9 Oct 2024 19:25:02 +0100 Subject: [PATCH 3/4] PS: Add return and out nodes. --- .../dataflow/internal/DataFlowPrivate.qll | 92 ++++++++++++++++++- 1 file changed, 90 insertions(+), 2 deletions(-) diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll index d2ed0af64ab..d253ac48a73 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll @@ -489,7 +489,89 @@ abstract class ReturnNode extends Node { } private module ReturnNodes { - // TODO + /** An AST element that may produce return values when evaluated. */ + abstract private class ReturnContainer extends Ast { + /** + * Gets a direct node that will may be returned when evaluating this node. + */ + Node getANode() { none() } + + /** Gets a child that may produce more nodes that may be returned. */ + abstract ReturnContainer getAChild(); + + /** + * Gets a (possibly transitive) node that may be returned when evaluating + * this node. + */ + final Node getAReturnedNode() { + result = this.getANode() + or + result = this.getAChild().getAReturnedNode() + } + } + + class ScriptBlockReturnContainer extends ReturnContainer, ScriptBlock { + final override ReturnContainer getAChild() { result = this.getEndBlock() } + } + + class NamedBlockReturnContainer extends ReturnContainer, NamedBlock { + final override ReturnContainer getAChild() { result = this.getAStmt() } + } + + class CmdExprReturnContainer extends ReturnContainer, CmdExpr { + final override ExprNode getANode() { result.getExprNode().getExpr() = this.getExpr() } + + final override ReturnContainer getAChild() { none() } + } + + class LoopStmtReturnContainer extends ReturnContainer, LoopStmt { + final override ReturnContainer getAChild() { result = this.getBody() } + } + + class StmtBlockReturnConainer extends ReturnContainer, StmtBlock { + final override ReturnContainer getAChild() { result = this.getAStmt() } + } + + class TryStmtReturnContainer extends ReturnContainer, TryStmt { + final override ReturnContainer getAChild() { + result = this.getBody() or result = this.getACatchClause() or result = this.getFinally() + } + } + + class ReturnStmtReturnContainer extends ReturnContainer, ReturnStmt { + final override ReturnContainer getAChild() { result = this.getPipeline() } + } + + class CatchClausReturnContainer extends ReturnContainer, CatchClause { + final override ReturnContainer getAChild() { result = this.getBody() } + } + + class SwitchStmtReturnContainer extends ReturnContainer, SwitchStmt { + final override ReturnContainer getAChild() { result = this.getACase() } + } + + class CmdBaseReturnContainer extends ReturnContainer, CmdExpr { + final override ExprNode getANode() { result.getExprNode().getExpr() = this.getExpr() } + + final override ReturnContainer getAChild() { none() } + } + + class CmdReturnContainer extends ReturnContainer, Cmd { + final override StmtNode getANode() { result.getStmtNode().getStmt() = this } + + final override ReturnContainer getAChild() { none() } + } + + class NormalReturnNode extends ReturnNode instanceof NodeImpl { + NormalReturnNode() { + exists(ReturnContainer container | + container = this.getEnclosingCallable().asCfgScope() and + this = container.getAReturnedNode() + ) + } + + final override NormalReturnKind getKind() { any() } + } } import ReturnNodes @@ -501,7 +583,13 @@ abstract class OutNode extends Node { } private module OutNodes { - // TODO + /** A data-flow node that reads a value returned directly by a callable */ + class CallOutNode extends OutNode instanceof CallNode { + override DataFlowCall getCall(ReturnKind kind) { + result.asCall() = super.getCallNode() and + kind instanceof NormalReturnKind + } + } } import OutNodes From 1527479518e0932ce84a3facd998b9700e064d5e Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 9 Oct 2024 19:25:58 +0100 Subject: [PATCH 4/4] PS: Accept test changes. --- .../dataflow/returns/test.expected | 34 +++++++++++++++++++ .../library-tests/dataflow/returns/test.ps1 | 8 ++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/powershell/ql/test/library-tests/dataflow/returns/test.expected b/powershell/ql/test/library-tests/dataflow/returns/test.expected index 4e4a41dfc62..dbd31969033 100644 --- a/powershell/ql/test/library-tests/dataflow/returns/test.expected +++ b/powershell/ql/test/library-tests/dataflow/returns/test.expected @@ -1,6 +1,40 @@ models edges +| test.ps1:2:5:2:15 | Source | test.ps1:5:6:5:20 | callSourceOnce | provenance | | +| test.ps1:5:6:5:20 | callSourceOnce | test.ps1:6:6:6:8 | x | provenance | | +| test.ps1:9:5:9:15 | Source | test.ps1:13:6:13:21 | callSourceTwice | provenance | | +| test.ps1:10:5:10:15 | Source | test.ps1:13:6:13:21 | callSourceTwice | provenance | | +| test.ps1:13:6:13:21 | callSourceTwice | test.ps1:14:6:14:8 | x | provenance | | +| test.ps1:17:12:17:22 | Source | test.ps1:20:6:20:19 | returnSource1 | provenance | | +| test.ps1:20:6:20:19 | returnSource1 | test.ps1:21:6:21:8 | x | provenance | | +| test.ps1:24:10:24:20 | Source | test.ps1:25:5:25:7 | x | provenance | | +| test.ps1:25:5:25:7 | x | test.ps1:30:6:30:19 | returnSource2 | provenance | | +| test.ps1:26:10:26:20 | Source | test.ps1:27:12:27:14 | y | provenance | | +| test.ps1:27:12:27:14 | y | test.ps1:30:6:30:19 | returnSource2 | provenance | | +| test.ps1:30:6:30:19 | returnSource2 | test.ps1:31:6:31:8 | x | provenance | | nodes +| test.ps1:2:5:2:15 | Source | semmle.label | Source | +| test.ps1:5:6:5:20 | callSourceOnce | semmle.label | callSourceOnce | +| test.ps1:6:6:6:8 | x | semmle.label | x | +| test.ps1:9:5:9:15 | Source | semmle.label | Source | +| test.ps1:10:5:10:15 | Source | semmle.label | Source | +| test.ps1:13:6:13:21 | callSourceTwice | semmle.label | callSourceTwice | +| test.ps1:14:6:14:8 | x | semmle.label | x | +| test.ps1:17:12:17:22 | Source | semmle.label | Source | +| test.ps1:20:6:20:19 | returnSource1 | semmle.label | returnSource1 | +| test.ps1:21:6:21:8 | x | semmle.label | x | +| test.ps1:24:10:24:20 | Source | semmle.label | Source | +| test.ps1:25:5:25:7 | x | semmle.label | x | +| test.ps1:26:10:26:20 | Source | semmle.label | Source | +| test.ps1:27:12:27:14 | y | semmle.label | y | +| test.ps1:30:6:30:19 | returnSource2 | semmle.label | returnSource2 | +| test.ps1:31:6:31:8 | x | semmle.label | x | subpaths testFailures #select +| test.ps1:6:6:6:8 | x | test.ps1:2:5:2:15 | Source | test.ps1:6:6:6:8 | x | $@ | test.ps1:2:5:2:15 | Source | Source | +| test.ps1:14:6:14:8 | x | test.ps1:9:5:9:15 | Source | test.ps1:14:6:14:8 | x | $@ | test.ps1:9:5:9:15 | Source | Source | +| test.ps1:14:6:14:8 | x | test.ps1:10:5:10:15 | Source | test.ps1:14:6:14:8 | x | $@ | test.ps1:10:5:10:15 | Source | Source | +| test.ps1:21:6:21:8 | x | test.ps1:17:12:17:22 | Source | test.ps1:21:6:21:8 | x | $@ | test.ps1:17:12:17:22 | Source | Source | +| test.ps1:31:6:31:8 | x | test.ps1:24:10:24:20 | Source | test.ps1:31:6:31:8 | x | $@ | test.ps1:24:10:24:20 | Source | Source | +| test.ps1:31:6:31:8 | x | test.ps1:26:10:26:20 | Source | test.ps1:31:6:31:8 | x | $@ | test.ps1:26:10:26:20 | Source | Source | diff --git a/powershell/ql/test/library-tests/dataflow/returns/test.ps1 b/powershell/ql/test/library-tests/dataflow/returns/test.ps1 index f54421b3ffe..df9f690126a 100644 --- a/powershell/ql/test/library-tests/dataflow/returns/test.ps1 +++ b/powershell/ql/test/library-tests/dataflow/returns/test.ps1 @@ -3,7 +3,7 @@ function callSourceOnce { } $x = callSourceOnce -Sink $x # $ MISSING: hasValueFlow=1 +Sink $x # $ hasValueFlow=1 function callSourceTwice { Source "2" @@ -11,14 +11,14 @@ function callSourceTwice { } $x = callSourceTwice -Sink $x # $ MISSING: hasValueFlow=2 hasValueFlow=3 +Sink $x # $ hasValueFlow=2 hasValueFlow=3 function returnSource1 { return Source "4" } $x = returnSource1 -Sink $x # $ MISSING: hasValueFlow=4 +Sink $x # $ hasValueFlow=4 function returnSource2 { $x = Source "5" @@ -28,4 +28,4 @@ function returnSource2 { } $x = returnSource2 -Sink $x # $ MISSING: hasValueFlow=5 hasValueFlow=6 \ No newline at end of file +Sink $x # $ hasValueFlow=5 hasValueFlow=6 \ No newline at end of file