Merge pull request #114 from microsoft/powershell-flow-into-this

MathiasVP · web-flow · commit 7e66dc3481d3 · 2024-10-08T16:38:08.000+02:00
PS: Support flow through `this`
diff --git a/powershell/ql/lib/powershell.qll b/powershell/ql/lib/powershell.qll
@@ -79,4 +79,5 @@ import semmle.code.powershell.IndexExpr
 import semmle.code.powershell.HashTable
 import semmle.code.powershell.SplitExpr
 import semmle.code.powershell.CommentEntity
-import semmle.code.powershell.Variable
+import semmle.code.powershell.Variable
+import semmle.code.powershell.internal.Internal::Public
diff --git a/powershell/ql/lib/semmle/code/powershell/Command.qll b/powershell/ql/lib/semmle/code/powershell/Command.qll
@@ -65,30 +65,3 @@ class Cmd extends @command, CmdBase {
 
   Redirection getARedirection() { result = this.getRedirection(_) }
 }
-
-/**
- * An argument to a command.
- *
- * The argument may be named or positional.
- */
-class Argument extends Expr {
-  Cmd cmd;
-
-  Argument() { cmd.getAnArgument() = this }
-
-  Cmd getCmd() { result = cmd }
-
-  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(_) }
-}
diff --git a/powershell/ql/lib/semmle/code/powershell/Function.qll b/powershell/ql/lib/semmle/code/powershell/Function.qll
@@ -33,16 +33,28 @@ abstract private class AbstractFunction extends Ast {
   /** Gets the number of function parameters. */
   final int getNumberOfFunctionParameters() { result = count(this.getAFunctionParameter()) }
 
-  /** Gets the number of parameters (both function and block). */
+  /**
+   * Gets the number of parameters (both function and block).
+   * Note: This excludes the `this` parameter.
+   */
   final int getNumberOfParameters() { result = count(this.getAParameter()) }
 
-  /** Gets the i'th parameter of this function, if any. */
+  /**
+   * Gets the i'th parameter of this function, if any.
+   *
+   * This does not include the `this` parameter.
+   */
   final Parameter getParameter(int i) {
     result = this.getFunctionParameter(i)
     or
     result = this.getBody().getParamBlock().getParameter(i)
   }
 
+  final Parameter getThisParameter() {
+    result.isThis() and
+    result.getFunction() = this
+  }
+
   /** Gets any parameter of this function. */
   final Parameter getAParameter() { result = this.getParameter(_) }
 
diff --git a/powershell/ql/lib/semmle/code/powershell/IndexExpr.qll b/powershell/ql/lib/semmle/code/powershell/IndexExpr.qll
@@ -1,5 +1,5 @@
 import powershell
-private import internal.ExplicitWrite
+private import internal.ExplicitWrite::Private
 
 class IndexExpr extends @index_expression, Expr {
   override string toString() { result = "...[...]" }
diff --git a/powershell/ql/lib/semmle/code/powershell/Variable.qll b/powershell/ql/lib/semmle/code/powershell/Variable.qll
@@ -1,6 +1,6 @@
 private import powershell
 private import semmle.code.powershell.controlflow.internal.Scope
-private import internal.Internal as Internal
+private import internal.Parameter::Private as Internal
 
 private predicate isFunctionParameterImpl(Internal::Parameter p, Function f, int i) {
   function_definition_parameter(f, i, p)
@@ -42,7 +42,8 @@ private newtype TParameterImpl =
   TInternalParameter(Internal::Parameter p) or
   TUnderscore(Scope scope) {
     exists(VarAccess va | va.getUserPath() = "_" and scope = va.getEnclosingScope())
-  }
+  } or
+  TThisParameter(Scope scope) { exists(scope.getEnclosingFunction().getDeclaringType()) }
 
 private class ParameterImpl extends TParameterImpl {
   abstract Location getLocation();
@@ -109,9 +110,22 @@ private class Underscore extends ParameterImpl, TUnderscore {
   final override Scope getEnclosingScope() { result = scope }
 }
 
+private class ThisParameter extends ParameterImpl, TThisParameter {
+  Scope scope;
+
+  ThisParameter() { this = TThisParameter(scope) }
+
+  override Location getLocation() { result = scope.getLocation() }
+
+  override string getName() { result = "this" }
+
+  final override Scope getEnclosingScope() { result = scope }
+}
+
 private newtype TVariable =
   TLocalVariable(string name, Scope scope) {
     not isParameterImpl(name, scope) and
+    not name = "this" and // This is modeled as a parameter
     exists(VarAccess va | va.getUserPath() = name and scope = va.getEnclosingScope())
   } or
   TParameter(ParameterImpl p)
@@ -186,8 +200,11 @@ class Parameter extends AbstractLocalScopeVariable, TParameter {
 
   predicate hasDefaultValue() { exists(this.getDefaultValue()) }
 
+  /** Holds if this is the `this` parameter. */
+  predicate isThis() { p instanceof ThisParameter }
+
   /**
-   * Gets the index of this parameter.
+   * Gets the index of this parameter, if any.
    *
    * The parameter may be in a parameter block or a function parameter.
    */
diff --git a/powershell/ql/lib/semmle/code/powershell/VariableExpression.qll b/powershell/ql/lib/semmle/code/powershell/VariableExpression.qll
@@ -1,5 +1,5 @@
 import powershell
-private import internal.ExplicitWrite
+private import internal.ExplicitWrite::Private
 
 private predicate isParameterName(@variable_expression ve) { parameter(_, ve, _, _) }
 
diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll
@@ -217,7 +217,11 @@ module ExprNodes {
     /** Gets the name of this argument, if any. */
     string getName() { result = e.getName() }
 
-    StmtNodes::CmdCfgNode getCmd() { result.getAnArgument() = this }
+    /** Holds if `this` is a qualifier to a call. */
+    predicate isQualifier() { e.isQualifier() }
+
+    /** Gets the call for which this is an argument. */
+    CallCfgNode getCall() { result.getAnArgument() = this or result.getQualifier() = this }
   }
 
   private class InvokeMemberChildMapping extends ExprChildMapping, InvokeMemberExpr {
diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Scope.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Scope.qll
@@ -36,4 +36,11 @@ class Scope extends Ast, @script_block {
    * This may be both function paramters and parameter block parameters.
    */
   Parameter getAParameter() { result = this.getParameter(_) }
+
+  Parameter getThisParameter() {
+    exists(Function func |
+      func.getBody() = this and
+      result = func.getThisParameter()
+    )
+  }
 }
diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowDispatch.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowDispatch.qll
@@ -37,7 +37,7 @@ abstract class LibraryCallable extends string {
   LibraryCallable() { any() }
 
   /** Gets a call to this library callable. */
-  Cmd getACall() { none() }
+  Call getACall() { none() }
 }
 
 /**
@@ -222,7 +222,8 @@ private module Cached {
 
   cached
   newtype TArgumentPosition =
-    TKeywordArgumentPosition(string name) { name = any(CmdParameter p).getName() } or
+    TThisArgumentPosition() or
+    TKeywordArgumentPosition(string name) { name = any(Argument p).getName() } or
     TPositionalArgumentPosition(int pos, NamedSet ns) {
       exists(CfgNodes::CallCfgNode call |
         call = ns.getABindingCall() and
@@ -232,7 +233,8 @@ private module Cached {
 
   cached
   newtype TParameterPosition =
-    TKeywordParameter(string name) { name = any(CmdParameter p).getName() } or
+    TThisParameterPosition() or
+    TKeywordParameter(string name) { name = any(Argument p).getName() } or
     TPositionalParameter(int pos, NamedSet ns) {
       exists(CfgNodes::CallCfgNode call |
         call = ns.getABindingCall() and
@@ -245,6 +247,9 @@ import Cached
 
 /** A parameter position. */
 class ParameterPosition extends TParameterPosition {
+  /** Holds if this position represents a `this` parameter. */
+  predicate isThis() { this = TThisParameterPosition() }
+
   /**
    * Holds if this position represents a positional parameter at position `pos`
    * with function is called with exactly the named parameters from the set `ns`
@@ -256,6 +261,8 @@ class ParameterPosition extends TParameterPosition {
 
   /** Gets a textual representation of this position. */
   string toString() {
+    this.isThis() and result = "this"
+    or
     exists(int pos, NamedSet ns |
       this.isPositional(pos, ns) and result = "pos(" + pos + ", " + ns.toString() + ")"
     )
@@ -266,13 +273,18 @@ class ParameterPosition extends TParameterPosition {
 
 /** An argument position. */
 class ArgumentPosition extends TArgumentPosition {
+  /** Holds if this position represents a `this` argument. */
+  predicate isThis() { this = TThisArgumentPosition() }
+
   /** Holds if this position represents a positional argument at position `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() {
+    this.isThis() and result = "this"
+    or
     exists(int pos, NamedSet ns |
       this.isPositional(pos, ns) and result = "pos(" + pos + ", " + ns.toString() + ")"
     )
@@ -284,6 +296,8 @@ class ArgumentPosition extends TArgumentPosition {
 /** Holds if arguments at position `apos` match parameters at position `ppos`. */
 pragma[nomagic]
 predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
+  ppos.isThis() and apos.isThis()
+  or
   exists(string name |
     ppos.isKeyword(name) and
     apos.isKeyword(name)
diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll
@@ -407,6 +407,9 @@ private module ParameterNodes {
     override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
       parameter.getDeclaringScope() = c.asCfgScope() and
       (
+        pos.isThis() and
+        parameter.isThis()
+        or
         pos.isKeyword(parameter.getName())
         or
         // Given a function f with parameters x, y we map
@@ -432,7 +435,9 @@ private module ParameterNodes {
       )
     }
 
-    override CfgScope getCfgScope() { result.getAParameter() = parameter }
+    override CfgScope getCfgScope() {
+      result.getAParameter() = parameter or result.getThisParameter() = parameter
+    }
 
     override Location getLocationImpl() { result = parameter.getLocation() }
 
@@ -458,7 +463,7 @@ module ArgumentNodes {
     ExplicitArgumentNode() { this.asExpr() = arg }
 
     override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
-      arg.getCmd() = call.asCall() and
+      arg.getCall() = call.asCall() and
       (
         pos.isKeyword(arg.getName())
         or
@@ -467,6 +472,9 @@ module ArgumentNodes {
           ns.getAnExactBindingCall() = call.asCall() and
           pos.isPositional(i, ns)
         )
+        or
+        arg.isQualifier() and
+        pos.isThis()
       )
     }
   }
diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/SsaImpl.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/SsaImpl.qll
@@ -69,8 +69,14 @@ predicate uninitializedWrite(Cfg::EntryBasicBlock bb, int i, LocalVariable v) {
  * function f($a, $b) { ... }
  * ```
  * the inverted index of `$a` is 1, and the inverted index of `$b` is 0.
+ *
+ * The inverted index of `$this` is always always the number of
+ * parameters (excluding `this`).
  */
 private int getInvertedIndex(Parameter p) {
+  p.isThis() and
+  result = p.getFunction().getNumberOfParameters()
+  or
   exists(int i |
     p.getIndex() = i or
     p.hasParameterBlock(_, i)
diff --git a/powershell/ql/lib/semmle/code/powershell/internal/Argument.qll b/powershell/ql/lib/semmle/code/powershell/internal/Argument.qll
@@ -0,0 +1,71 @@
+private import powershell
+
+module Private {
+  /**
+   * An argument to a call.
+   *
+   * The argument may be named or positional.
+   */
+  abstract class AbstractArgument extends Expr {
+    Ast call;
+
+    /** Gets the call that this is an argumnt of. */
+    final Ast getCall() { result = call }
+
+    /** Gets the position if this is a positional argument. */
+    abstract int getPosition();
+
+    /** Gets the name if this is a keyword argument. */
+    abstract string getName();
+
+    /** Holds if this is a qualifier of a call */
+    abstract predicate isQualifier();
+  }
+
+  class CmdArgument extends AbstractArgument {
+    override Cmd call;
+
+    CmdArgument() { call.getAnArgument() = this }
+
+    override int getPosition() { call.getPositionalArgument(result) = this }
+
+    override string getName() { call.getNamedArgument(result) = this }
+
+    final override predicate isQualifier() { none() }
+  }
+
+  class InvokeArgument extends AbstractArgument {
+    override InvokeMemberExpr call;
+
+    InvokeArgument() { call.getAnArgument() = this or call.getQualifier() = this }
+
+    override int getPosition() { call.getArgument(result) = this }
+
+    override string getName() { none() }
+
+    final override predicate isQualifier() { call.getQualifier() = this }
+  }
+}
+
+private import Private
+
+module Public {
+  final class Argument = AbstractArgument;
+
+  /** A positional argument to a command. */
+  class PositionalArgument extends Argument {
+    PositionalArgument() {
+      not this instanceof NamedArgument and not this instanceof QualifierArgument
+    }
+  }
+
+  /** A named argument to a command. */
+  class NamedArgument extends Argument {
+    NamedArgument() { exists(this.getName()) }
+  }
+
+  /** An argument that is a qualifier to a method. */
+  class QualifierArgument extends Argument {
+    QualifierArgument() { this.isQualifier() }
+  }
+}
diff --git a/powershell/ql/lib/semmle/code/powershell/internal/ExplicitWrite.qll b/powershell/ql/lib/semmle/code/powershell/internal/ExplicitWrite.qll
@@ -1,15 +1,19 @@
 private import powershell
 
-/**
- * Holds if `e` is written to by `assign`.
- *
- * Note there may be more than one `e` for which `isExplicitWrite(e, assign)`
- * holds if the left-hand side is an array literal.
- */
-predicate isExplicitWrite(Expr e, AssignStmt assign) {
-  e = assign.getLeftHandSide()
-  or
-  e = any(ConvertExpr convert | isExplicitWrite(convert, assign)).getExpr()
-  or
-  e = any(ArrayLiteral array | isExplicitWrite(array, assign)).getAnElement()
+module Private {
+  /**
+   * Holds if `e` is written to by `assign`.
+   *
+   * Note there may be more than one `e` for which `isExplicitWrite(e, assign)`
+   * holds if the left-hand side is an array literal.
+   */
+  predicate isExplicitWrite(Expr e, AssignStmt assign) {
+    e = assign.getLeftHandSide()
+    or
+    e = any(ConvertExpr convert | isExplicitWrite(convert, assign)).getExpr()
+    or
+    e = any(ArrayLiteral array | isExplicitWrite(array, assign)).getAnElement()
+  }
 }
+
+module Public { }
diff --git a/powershell/ql/lib/semmle/code/powershell/internal/Internal.qll b/powershell/ql/lib/semmle/code/powershell/internal/Internal.qll
diff --git a/powershell/ql/lib/semmle/code/powershell/internal/Parameter.qll b/powershell/ql/lib/semmle/code/powershell/internal/Parameter.qll
diff --git a/powershell/ql/test/library-tests/controlflow/graph/Cfg.expected b/powershell/ql/test/library-tests/controlflow/graph/Cfg.expected
diff --git a/powershell/ql/test/library-tests/controlflow/graph/Cfg.ql b/powershell/ql/test/library-tests/controlflow/graph/Cfg.ql
diff --git a/powershell/ql/test/library-tests/dataflow/fields/test.expected b/powershell/ql/test/library-tests/dataflow/fields/test.expected
diff --git a/powershell/ql/test/library-tests/dataflow/fields/test.ps1 b/powershell/ql/test/library-tests/dataflow/fields/test.ps1

Original file line number	Diff line number	Diff line change
`@@ -407,6 +407,9 @@ private module ParameterNodes {`
`407`	`407`	`override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {`
`408`	`408`	`parameter.getDeclaringScope() = c.asCfgScope() and`
`409`	`409`	`(`
	`410`	`+ pos.isThis() and`
	`411`	`+ parameter.isThis()`
	`412`	`+ or`
`410`	`413`	`pos.isKeyword(parameter.getName())`
`411`	`414`	`or`
`412`	`415`	`// Given a function f with parameters x, y we map`
`@@ -432,7 +435,9 @@ private module ParameterNodes {`
`432`	`435`	`)`
`433`	`436`	`}`
`434`	`437`
`435`		`- override CfgScope getCfgScope() { result.getAParameter() = parameter }`
	`438`	`+ override CfgScope getCfgScope() {`
	`439`	`+ result.getAParameter() = parameter or result.getThisParameter() = parameter`
	`440`	`+ }`
`436`	`441`
`437`	`442`	`override Location getLocationImpl() { result = parameter.getLocation() }`
`438`	`443`
`@@ -458,7 +463,7 @@ module ArgumentNodes {`
`458`	`463`	`ExplicitArgumentNode() { this.asExpr() = arg }`
`459`	`464`
`460`	`465`	`override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {`
`461`		`- arg.getCmd() = call.asCall() and`
	`466`	`+ arg.getCall() = call.asCall() and`
`462`	`467`	`(`
`463`	`468`	`pos.isKeyword(arg.getName())`
`464`	`469`	`or`
`@@ -467,6 +472,9 @@ module ArgumentNodes {`
`467`	`472`	`ns.getAnExactBindingCall() = call.asCall() and`
`468`	`473`	`pos.isPositional(i, ns)`
`469`	`474`	`)`
	`475`	`+ or`
	`476`	`+ arg.isQualifier() and`
	`477`	`+ pos.isThis()`
`470`	`478`	`)`
`471`	`479`	`}`
`472`	`480`	`}`