Merge pull request #118 from microsoft/powershell-add-return-and-out-nodes

PS: Add flow out of functions

Author: MathiasVP

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

powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll

@@ -33,15 +33,25 @@ 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.
  *
  * Note that because of control-flow splitting, one `Expr` may correspond
  * 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 }
+}

powershell/ql/test/library-tests/dataflow/returns/test.expected

@@ -0,0 +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 |

powershell/ql/test/library-tests/dataflow/returns/test.ps1

@@ -0,0 +1,31 @@
+function callSourceOnce {
+    Source "1"
+}
+
+$x = callSourceOnce
+Sink $x # $ hasValueFlow=1
+
+function callSourceTwice {
+    Source "2"
+    Source "3"
+}
+
+$x = callSourceTwice
+Sink $x # $ hasValueFlow=2 hasValueFlow=3
+
+function returnSource1 {
+    return Source "4"
+}
+
+$x = returnSource1
+Sink $x # $ hasValueFlow=4
+
+function returnSource2 {
+    $x = Source "5"
+    $x
+    $y = Source "6"
+    return $y
+}
+
+$x = returnSource2
+Sink $x # $ hasValueFlow=5 hasValueFlow=6

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()