Merge pull request #180 from microsoft/powershell-ast-modernization-follow-up

PS: Fix PowerShell dataflow/taint-tracking failures

Author: MathiasVP

powershell/ql/lib/semmle/code/powershell/ast/internal/If.qll

@@ -53,5 +53,13 @@ class If extends Expr, TIf {
     )
   }
 
+  StmtBlock getABranch(boolean b) {
+    b = true and result = this.getAThen()
+    or
+    b = false and result = this.getElse()
+  }
+
+  StmtBlock getABranch() { result = this.getAThen() or result = this.getElse() }
+
   predicate hasElse() { exists(this.getElse()) }
 }

powershell/ql/lib/semmle/code/powershell/ast/internal/ScriptBlock.qll

@@ -79,6 +79,9 @@ class ScriptBlock extends Ast, TScriptBlock {
       result = this.getParameter(index)
     )
     or
+    i = ThisVar() and
+    result = this.getThisParameter()
+    or
     exists(int index |
       i = scriptBlockUsing(index) and
       result = this.getUsingStmt(index)
@@ -90,13 +93,14 @@ class ScriptBlock extends Ast, TScriptBlock {
     or
     any(Synthesis s).pipelineParameterHasIndex(this, i) and
     synthChild(getRawAst(this), PipelineParamVar(), result)
-    or
-    i = -1 and
-    synthChild(getRawAst(this), ThisVar(), result)
   }
 
+  Parameter getThisParameter() { synthChild(getRawAst(this), ThisVar(), result) }
+
   /**
    * Gets a parameter of this block.
+   *
+   * Note: This does not include the `this` parameter, but it does include pipeline parameters.
    */
   Parameter getAParameter() { result = this.getParameter(_) }
 

powershell/ql/lib/semmle/code/powershell/ast/internal/Synthesis.qll

@@ -84,6 +84,8 @@ class Synthesis extends TSynthesis {
 
   predicate functionName(FunctionBase f, string name) { none() }
 
+  predicate getAnAccess(VarAccessSynth va, Variable v) { none() }
+
   predicate memberName(Member m, string name) { none() }
 
   predicate typeName(Type t, string name) { none() }
@@ -116,13 +118,26 @@ Raw::Ast getRawAst(Ast r) { r = getResultAst(result) }
 
 private module ThisSynthesis {
   private class ThisSynthesis extends Synthesis {
+    private predicate thisAccess(Raw::Ast parent, ChildIndex i, Child child, Raw::Scope scope) {
+      scope = parent.getScope() and
+      parent.getChild(toRawChildIndex(i)).(Raw::VarAccess).getUserPath().toLowerCase() = "this" and
+      child = SynthChild(VarAccessSynthKind(TVariableSynth(scope, ThisVar())))
+    }
+
     override predicate child(Raw::Ast parent, ChildIndex i, Child child) {
       parent instanceof Raw::MethodScriptBlock and
       i = ThisVar() and
       child = SynthChild(VarSynthKind(ThisVarKind()))
       or
-      parent.getChild(toRawChildIndex(i)).(Raw::VarAccess).getUserPath().toLowerCase() = "this" and
-      child = SynthChild(VarAccessSynthKind(TVariableSynth(parent.getScope(), ThisVar())))
+      this.thisAccess(parent, i, child, _)
+    }
+
+    final override predicate getAnAccess(VarAccessSynth va, Variable v) {
+      exists(Raw::Ast parent, Raw::Scope scope, ChildIndex i |
+        this.thisAccess(parent, i, _, scope) and
+        v = TVariableSynth(scope, ThisVar()) and
+        va = TVarAccessSynth(parent, i)
+      )
     }
 
     override predicate variableSynthName(VariableSynth v, string name) {
@@ -731,18 +746,26 @@ private module IteratorAccessSynth {
       )
     }
 
+    final override predicate getAnAccess(VarAccessSynth va, Variable v) {
+      exists(Raw::Ast parent, ChildIndex i, Raw::VarAccess r |
+        this.expr(parent, i, r, _) and
+        va = TVarAccessSynth(parent, i) and
+        v = this.varAccess(r)
+      )
+    }
+
     override predicate exprStmtExpr(ExprStmt e, Expr expr) {
       exists(Raw::Ast p, Raw::VarAccess va, Raw::CmdExpr cmdExpr, ChildIndex i1, ChildIndex i2 |
         this.stmt(p, i1, _, _) and
         this.expr(cmdExpr, i2, va, _) and
         e = TExprStmtSynth(p, i1) and
-        expr = TVarAccessSynth(cmdExpr, i2, this.varAccess(va))
+        expr = TVarAccessSynth(cmdExpr, i2)
       )
     }
 
     final override Expr getResultAstImpl(Raw::Ast r) {
       exists(Raw::Ast parent, ChildIndex i | this.expr(parent, i, r, _) |
-        result = TVarAccessSynth(parent, i, this.varAccess(r))
+        result = TVarAccessSynth(parent, i)
       )
     }
 

powershell/ql/lib/semmle/code/powershell/ast/internal/TAst.qll

@@ -61,7 +61,7 @@ private predicate hasScopeAndName(VariableImpl variable, Scope::Range scope, str
   scope = variable.getDeclaringScopeImpl()
 }
 
-private predicate access(Raw::VarAccess va, VariableImpl v) {
+predicate access(Raw::VarAccess va, VariableImpl v) {
   exists(string name, Scope::Range scope |
     pragma[only_bind_into](name) = variableNameInScope(va, scope)
   |
@@ -150,11 +150,11 @@ private module Cached {
         )
     } or
     TVariableSynth(Raw::Ast scope, ChildIndex i) { mkSynthChild(VarSynthKind(_), scope, i) } or
-    TVarAccessReal(Raw::VarAccess va, Variable v) { access(va, v) } or
-    TVarAccessSynth(Raw::Ast parent, ChildIndex i, Variable v) {
-      mkSynthChild(VarAccessRealKind(v), parent, i)
+    TVarAccessReal(Raw::VarAccess va) { access(va, _) } or
+    TVarAccessSynth(Raw::Ast parent, ChildIndex i) {
+      mkSynthChild(VarAccessRealKind(_), parent, i)
       or
-      mkSynthChild(VarAccessSynthKind(v), parent, i)
+      mkSynthChild(VarAccessSynthKind(_), parent, i)
     } or
     TWhileStmt(Raw::WhileStmt w) or
     TTypeNameExpr(Raw::TypeNameExpr t) or
@@ -277,7 +277,7 @@ private module Cached {
     n = TTypeConstraint(result) or
     n = TUnaryExpr(result) or
     n = TUsingStmt(result) or
-    n = TVarAccessReal(result, _) or
+    n = TVarAccessReal(result) or
     n = TWhileStmt(result) or
     n = TFunctionDefinitionStmt(result) or
     n = TExpandableSubExpr(result) or
@@ -308,7 +308,7 @@ private module Cached {
     result = TFunctionSynth(parent, i) or
     result = TBoolLiteral(parent, i) or
     result = TNullLiteral(parent, i) or
-    result = TVarAccessSynth(parent, i, _) or
+    result = TVarAccessSynth(parent, i) or
     result = TEnvVariable(parent, i) or
     result = TTypeSynth(parent, i) or
     result = TAutomaticVariable(parent, i) or

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

@@ -95,25 +95,23 @@ module Private {
 
   class VarAccessReal extends VarAccessImpl, TVarAccessReal {
     Raw::VarAccess va;
-    Variable v;
 
-    VarAccessReal() { this = TVarAccessReal(va, v) }
+    VarAccessReal() { this = TVarAccessReal(va) }
 
-    final override Variable getVariableImpl() { result = v }
+    final override Variable getVariableImpl() { access(va, result) }
 
-    final override string toString() { result = v.getName() }
+    final override string toString() { result = va.getUserPath() }
   }
 
   class VarAccessSynth extends VarAccessImpl, TVarAccessSynth {
     Raw::Ast parent;
     ChildIndex i;
-    Variable v;
 
-    VarAccessSynth() { this = TVarAccessSynth(parent, i, v) }
+    VarAccessSynth() { this = TVarAccessSynth(parent, i) }
 
-    final override Variable getVariableImpl() { result = v }
+    final override Variable getVariableImpl() { any(Synthesis s).getAnAccess(this, result) }
 
-    final override string toString() { result = v.getName() }
+    final override string toString() { result = this.getVariableImpl().getName() }
 
     final override Location getLocation() { result = parent.getLocation() }
   }