Merge pull request #105 from microsoft/powershell-argument-parameter-matching

PS: Implement argument/parameter matching in dataflow

powershell/ql/lib/semmle/code/powershell/Command.qll

@@ -19,8 +19,14 @@ class Cmd extends @command, CmdBase {
 
   StringConstExpr getCmdName() { result = this.getElement(0) }
 
-  Expr getAnArgument() { result = this.getArgument(_) or result = this.getNamedArgument(_) }
+  /** Gets any argument to this command. */
+  Expr getAnArgument() { result = this.getArgument(_) }
 
+  /**
+   * Gets the `i`th argument to this command.
+   *
+   * The argument may be named or positional.
+   */
   Expr getArgument(int i) {
     result =
       rank[i + 1](CmdElement e, int j |
@@ -32,6 +38,18 @@ class Cmd extends @command, CmdBase {
       )
   }
 
+  /** Gets the `i`th positional argument to this command. */
+  Expr getPositionalArgument(int i) {
+    result =
+      rank[i + 1](Argument e, int j |
+        e = this.getArgument(j) and
+        e instanceof PositionalArgument
+      |
+        e order by j
+      )
+  }
+
+  /** Gets the named argument with the given name. */
   Expr getNamedArgument(string name) {
     exists(int i, CmdParameter p |
       this.getElement(i) = p and
@@ -53,11 +71,21 @@ class Cmd extends @command, CmdBase {
 class Argument extends Expr {
   Cmd cmd;
 
-  Argument() { cmd.getArgument(_) = this or cmd.getNamedArgument(_) = this }
+  Argument() { cmd.getAnArgument() = this }
 
   Cmd getCmd() { result = cmd }
 
-  int getIndex() { cmd.getArgument(result) = this }
+  int getPosition() { cmd.getPositionalArgument(result) = this }
 
   string getName() { cmd.getNamedArgument(result) = this }
 }
+
+/** A positional argument to a command. */
+class PositionalArgument extends Argument {
+  PositionalArgument() { not this instanceof NamedArgument }
+}
+
+/** A named argument to a command. */
+class NamedArgument extends Argument {
+  NamedArgument() { this = cmd.getNamedArgument(_) }
+}

powershell/ql/lib/semmle/code/powershell/Variable.qll

@@ -14,10 +14,10 @@ private predicate hasParameterBlockImpl(Internal::Parameter p, ParamBlock block,
 
 /**
  * Gets the enclosing scope of `p`.
- * 
+ *
  * For a function parameter, this is the function body. For a parameter from a
  * parameter block, this is the enclosing scope of the parameter block.
- * 
+ *
  * In both of the above cases, the enclosing scope is the function body.
  */
 private Scope getEnclosingScopeImpl(Internal::Parameter p) {
@@ -176,6 +176,8 @@ class Parameter extends AbstractLocalScopeVariable, TParameter {
 
   override string getName() { result = p.getName() }
 
+  final predicate hasName(string name) { name = p.getName() }
+
   final override Scope getDeclaringScope() { result = p.getEnclosingScope() }
 
   predicate hasParameterBlock(ParamBlock block, int i) { p.hasParameterBlock(block, i) }
@@ -186,7 +188,18 @@ class Parameter extends AbstractLocalScopeVariable, TParameter {
 
   predicate hasDefaultValue() { exists(this.getDefaultValue()) }
 
-  int getIndex() { this.isFunctionParameter(_, result) }
+  /**
+   * Gets the index of this parameter.
+   *
+   * The parameter may be in a parameter block or a function parameter.
+   */
+  int getIndex() { result = this.getFunctionIndex() or result = this.getBlockIndex() }
+
+  /** Gets the index of this parameter in the parameter block, if any. */
+  int getBlockIndex() { this.hasParameterBlock(_, result) }
+
+  /** Gets the index of this parameter in the function, if any. */
+  int getFunctionIndex() { this.isFunctionParameter(_, result) }
 
   Function getFunction() { result.getBody() = this.getDeclaringScope() }
 }

powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll

@@ -174,6 +174,14 @@ module ExprNodes {
     override Argument e;
 
     final override Argument getExpr() { result = super.getExpr() }
+
+    /** Gets the position of this argument, if any. */
+    int getPosition() { result = e.getPosition() }
+
+    /** Gets the name of this argument, if any. */
+    string getName() { result = e.getName() }
+
+    StmtNodes::CmdCfgNode getCmd() { result.getAnArgument() = this }
   }
 
   private class InvokeMemberChildMapping extends ExprChildMapping, InvokeMemberExpr {
@@ -253,7 +261,7 @@ module ExprNodes {
 
 module StmtNodes {
   private class CmdChildMapping extends NonExprChildMapping, Cmd {
-    override predicate relevantChild(Ast n) { n = this.getElement(_) }
+    override predicate relevantChild(Ast n) { n = this.getAnArgument() or n = this.getCommand() }
   }
 
   /** A control-flow node that wraps a `Cmd` AST expression. */
@@ -263,6 +271,20 @@ module StmtNodes {
     override CmdChildMapping s;
 
     override Cmd getStmt() { result = super.getStmt() }
+
+    ExprCfgNode getArgument(int i) { s.hasCfgChild(s.getArgument(i), this, result) }
+
+    ExprCfgNode getPositionalArgument(int i) {
+      s.hasCfgChild(s.getPositionalArgument(i), this, result)
+    }
+
+    ExprCfgNode getNamedArgument(string name) {
+      s.hasCfgChild(s.getNamedArgument(name), this, result)
+    }
+
+    ExprCfgNode getAnArgument() { s.hasCfgChild(s.getAnArgument(), this, result) }
+
+    ExprCfgNode getCommand() { s.hasCfgChild(s.getCommand(), this, result) }
   }
 
   private class AssignStmtChildMapping extends NonExprChildMapping, AssignStmt {

powershell/ql/lib/semmle/code/powershell/controlflow/internal/ControlFlowGraphImpl.qll

@@ -176,7 +176,7 @@ module Trees {
       exists(Parameter p |
         p =
           rank[i + 1](Parameter cand, int j |
-            cand.hasDefaultValue() and j = cand.getIndex()
+            cand.hasDefaultValue() and j = cand.getFunctionIndex()
           |
             cand order by j
           ) and

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

@@ -170,34 +170,75 @@ private module Cached {
 
   cached
   newtype TArgumentPosition =
-    TPositionalArgumentPosition(int pos) { exists(Cmd c | exists(c.getArgument(pos))) }
+    TKeywordArgumentPosition(string name) { name = any(CmdParameter p).getName() } or
+    TPositionalArgumentPosition(int pos, NamedSet ns) {
+      exists(Cmd cmd |
+        cmd = ns.getABindingCall() and
+        exists(cmd.getArgument(pos))
+      )
+    }
 
   cached
-  newtype TParameterPosition = TPositionalParameterPosition(int pos) { none() /* TODO */ }
+  newtype TParameterPosition =
+    TKeywordParameter(string name) { name = any(CmdParameter p).getName() } or
+    TPositionalParameter(int pos, NamedSet ns) {
+      exists(Cmd cmd |
+        cmd = ns.getABindingCall() and
+        exists(cmd.getArgument(pos))
+      )
+    }
 }
 
 import Cached
 
 /** A parameter position. */
 class ParameterPosition extends TParameterPosition {
-  /** Holds if this position represents a positional parameter at position `pos`. */
-  predicate isPositional(int pos) { this = TPositionalParameterPosition(pos) }
+  /**
+   * Holds if this position represents a positional parameter at position `pos`
+   * with function is called with exactly the named parameters from the set `ns`
+   */
+  predicate isPositional(int pos, NamedSet ns) { this = TPositionalParameter(pos, ns) }
+
+  /** Holds if this parameter is a keyword parameter with `name`. */
+  predicate isKeyword(string name) { this = TKeywordParameter(name) }
 
   /** Gets a textual representation of this position. */
-  string toString() { exists(int pos | this.isPositional(pos) and result = "position " + pos) }
+  string toString() {
+    exists(int pos, NamedSet ns |
+      this.isPositional(pos, ns) and result = "pos(" + pos + ", " + ns.toString() + ")"
+    )
+    or
+    exists(string name | this.isKeyword(name) and result = "kw(" + name + ")")
+  }
 }
 
 /** An argument position. */
 class ArgumentPosition extends TArgumentPosition {
   /** Holds if this position represents a positional argument at position `pos`. */
-  predicate isPositional(int pos) { this = TPositionalArgumentPosition(pos) }
+  predicate isPositional(int pos, NamedSet ns) { this = TPositionalArgumentPosition(pos, ns) }
+
+  predicate isKeyword(string name) { this = TKeywordArgumentPosition(name) }
 
   /** Gets a textual representation of this position. */
-  string toString() { exists(int pos | this.isPositional(pos) and result = "position " + pos) }
+  string toString() {
+    exists(int pos, NamedSet ns |
+      this.isPositional(pos, ns) and result = "pos(" + pos + ", " + ns.toString() + ")"
+    )
+    or
+    exists(string name | this.isKeyword(name) and result = "kw(" + name + ")")
+  }
 }
 
 /** Holds if arguments at position `apos` match parameters at position `ppos`. */
 pragma[nomagic]
 predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
-  exists(int pos | ppos.isPositional(pos) and apos.isPositional(pos))
+  exists(string name |
+    ppos.isKeyword(name) and
+    apos.isKeyword(name)
+  )
+  or
+  exists(int pos, NamedSet ns |
+    ppos.isPositional(pos, ns) and
+    apos.isPositional(pos, ns)
+  )
 }

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

@@ -229,18 +229,71 @@ class SsaInputNode extends SsaNode {
   override CfgScope getCfgScope() { result = node.getDefinitionExt().getBasicBlock().getScope() }
 }
 
+private string getANamedArgument(Cmd c) { exists(c.getNamedArgument(result)) }
+
+private module NamedSetModule = QlBuiltins::InternSets<Cmd, string, getANamedArgument/1>;
+
+private newtype NamedSet0 =
+  TEmptyNamedSet() or
+  TNonEmptyNamedSet(NamedSetModule::Set ns)
+
+/** A (possiby empty) set of argument names. */
+class NamedSet extends NamedSet0 {
+  /** Gets the non-empty set of names, if any. */
+  NamedSetModule::Set asNonEmpty() { this = TNonEmptyNamedSet(result) }
+
+  /** Holds if this is the empty set. */
+  predicate isEmpty() { this = TEmptyNamedSet() }
+
+  /** Gets a name in this set. */
+  string getAName() { this.asNonEmpty().contains(result) }
+
+  /** Gets the textual representation of this set. */
+  string toString() {
+    result = "{" + strictconcat(this.getAName(), ", ") + "}"
+    or
+    this.isEmpty() and
+    result = "{}"
+  }
+
+  /**
+   * Gets a `Cmd` that provides a named parameter for every name in `this`.
+   *
+   * NOTE: The `Cmd` may also provide more names.
+   */
+  Cmd getABindingCall() {
+    forex(string name | name = this.getAName() | exists(result.getNamedArgument(name)))
+    or
+    this.isEmpty() and
+    exists(result)
+  }
+
+  /**
+   * Gets a `Cmd` that provides exactly the named parameters represented by
+   * this set.
+   */
+  Cmd getAnExactBindingCall() {
+    forex(string name | name = this.getAName() | exists(result.getNamedArgument(name))) and
+    forex(string name | exists(result.getNamedArgument(name)) | name = this.getAName())
+    or
+    this.isEmpty() and
+    not exists(result.getNamedArgument(_))
+  }
+
+  /** Gets a function that has a parameter for each name in this set. */
+  Function getAFunction() {
+    forex(string name | name = this.getAName() | result.getAParameter().hasName(name))
+    or
+    this.isEmpty() and
+    exists(result)
+  }
+}
+
 private module ParameterNodes {
   abstract class ParameterNodeImpl extends NodeImpl {
     abstract Parameter getParameter();
 
     abstract predicate isParameterOf(DataFlowCallable c, ParameterPosition pos);
-
-    final predicate isSourceParameterOf(CfgScope c, ParameterPosition pos) {
-      exists(DataFlowCallable callable |
-        this.isParameterOf(callable, pos) and
-        c = callable.asCfgScope()
-      )
-    }
   }
 
   /**
@@ -255,8 +308,29 @@ private module ParameterNodes {
     override Parameter getParameter() { result = parameter }
 
     override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
-      exists(CfgScope callable, int i |
-        callable = c.asCfgScope() and pos.isPositional(i) and callable.getParameter(i) = parameter
+      exists(CfgScope callable | callable = c.asCfgScope() |
+        pos.isKeyword(parameter.getName())
+        or
+        // Given a function f with parameters x, y we map
+        // x to the positions:
+        // 1. keyword(x)
+        // 2. position(0, {y})
+        // 3. position(0, {})
+        // Likewise, y is mapped to the positions:
+        // 1. keyword(y)
+        // 2. position(0, {x})
+        // 3. position(1, {})
+        // The interpretation of `position(i, S)` is the position of the i'th unnamed parameter when the
+        // keywords in S are specified.
+        exists(int i, int j, string name, NamedSet ns, Function f |
+          pos.isPositional(j, ns) and
+          parameter.getIndex() = i and
+          f = parameter.getFunction() and
+          f = ns.getAFunction() and
+          name = parameter.getName() and
+          not name = ns.getAName() and
+          j = i - count(int k | k < i and f.getParameter(k).getName() = ns.getAName())
+        )
       )
     }
 
@@ -275,14 +349,29 @@ abstract class ArgumentNode extends Node {
   /** Holds if this argument occurs at the given position in the given call. */
   abstract predicate argumentOf(DataFlowCall call, ArgumentPosition pos);
 
-  abstract predicate sourceArgumentOf(CfgNodes::StmtNodes::CmdCfgNode call, ArgumentPosition pos);
-
   /** Gets the call in which this node is an argument. */
   final DataFlowCall getCall() { this.argumentOf(result, _) }
 }
 
 module ArgumentNodes {
-  // TODO
+  class ExplicitArgumentNode extends ArgumentNode {
+    CfgNodes::ExprNodes::ArgumentCfgNode arg;
+
+    ExplicitArgumentNode() { this.asExpr() = arg }
+
+    override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+      arg.getCmd() = call.asCall() and
+      (
+        pos.isKeyword(arg.getName())
+        or
+        exists(NamedSet ns, int i |
+          i = arg.getPosition() and
+          ns.getAnExactBindingCall() = call.asCall().getStmt() and
+          pos.isPositional(i, ns)
+        )
+      )
+    }
+  }
 }
 
 import ArgumentNodes

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

@@ -3,4 +3,37 @@ function Foo($a) {
 }
 
 $x = Source "1"
-Foo $x
+Foo $x
+
+function ThreeArgs($x, $y, $z) {
+    Sink $x # $ MISSING: hasValueFlow=x
+    Sink $y # $ MISSING: hasValueFlow=y
+    Sink $z # $ MISSING: hasValueFlow=z
+}
+
+$first = Source "x"
+$second = Source "y"
+$third = Source "z"
+
+ThreeArgs $first $second $third
+ThreeArgs $second -x $first $third
+ThreeArgs -x $first $second $third
+ThreeArgs $first -y $second $third
+ThreeArgs -y $second $first $third
+ThreeArgs $second -x $first -z $third
+ThreeArgs -x $first $second -z $third
+ThreeArgs $first -y $second -z $third
+ThreeArgs -y $second $first -z $third
+ThreeArgs $second -x $first -z $third
+ThreeArgs -x $first -z $third $second
+ThreeArgs $first -y $second -z $third
+ThreeArgs -y $second -z $third $first
+ThreeArgs $second -z $third -x $first
+ThreeArgs -x $first -z $third $second
+ThreeArgs $first -z $third -y $second
+ThreeArgs -y $second -z $third $first
+ThreeArgs -z $third -y $second $first
+ThreeArgs -z $third $second -x $first
+ThreeArgs -z $third -x $first $second
+ThreeArgs -z $third $first -y $second
+ThreeArgs -z $third -y $second $first