Merge pull request #287 from microsoft/add-zipslip-query-ps

PS: Add ZipSlip query

Author: MathiasVP

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

@@ -249,6 +249,15 @@ module API {
       result = this.getContent(contents.getAReadContent())
     }
 
+    /**
+     * Gets a representative for the instanceof field of the given `name`.
+     */
+    pragma[inline]
+    Node getField(string name) {
+      // This predicate is currently not 'inline_late' because 'name' can be an input or output
+      Impl::fieldEdge(this.getAnEpsilonSuccessor(), name, result)
+    }
+
     /**
      * Gets a representative for an arbitrary element of this collection.
      */
@@ -615,6 +624,15 @@ module API {
       contentEdge(pred, any(DataFlow::ContentSet set | set.isAnyElement()).getAReadContent(), succ)
     }
 
+    cached
+    predicate fieldEdge(Node pred, string name, Node succ) {
+      exists(DataFlow::ContentSet set, DataFlow::Content::FieldContent fc |
+        fc.getLowerCaseName() = name and
+        set.isSingleton(fc) and
+        contentEdge(pred, set.getAReadContent(), succ)
+      )
+    }
+
     cached
     predicate parameterEdge(Node pred, DataFlowDispatch::ParameterPosition paramPos, Node succ) {
       exists(DataFlowPrivate::ParameterNodeImpl parameter, DataFlow::CallableNode callable |

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

@@ -72,8 +72,25 @@ abstract private class ChildMapping extends Ast {
    */
   abstract predicate relevantChild(Ast child);
 
+  /**
+   * Holds if `child` appears before its parent in the control-flow graph.
+   * This always holds for expressions, and _almost_ never for statements.
+   */
+  abstract predicate precedesParent(Ast child);
+
   pragma[nomagic]
-  abstract predicate reachesBasicBlock(Ast child, CfgNode cfn, BasicBlock bb);
+  final predicate reachesBasicBlock(Ast child, CfgNode cfn, BasicBlock bb) {
+    this.relevantChild(child) and
+    cfn.getAstNode() = this and
+    bb.getANode() = cfn
+    or
+    exists(BasicBlock mid |
+      this.reachesBasicBlock(child, cfn, mid) and
+      not mid.getANode().getAstNode() = child
+    |
+      if this.precedesParent(child) then bb = mid.getAPredecessor() else bb = mid.getASuccessor()
+    )
+  }
 
   /**
    * Holds if there is a control-flow path from `cfn` to `cfnChild`, where `cfn`
@@ -93,18 +110,7 @@ abstract private class ChildMapping extends Ast {
  * A class for mapping parent-child AST nodes to parent-child CFG nodes.
  */
 abstract private class ExprChildMapping extends Expr, ChildMapping {
-  pragma[nomagic]
-  override predicate reachesBasicBlock(Ast child, CfgNode cfn, BasicBlock bb) {
-    this.relevantChild(child) and
-    cfn.getAstNode() = this and
-    bb.getANode() = cfn
-    or
-    exists(BasicBlock mid |
-      this.reachesBasicBlock(child, cfn, mid) and
-      bb = mid.getAPredecessor() and
-      not mid.getANode().getAstNode() = child
-    )
-  }
+  final override predicate precedesParent(Ast child) { this.relevantChild(child) }
 }
 
 /**
@@ -113,18 +119,7 @@ abstract private class ExprChildMapping extends Expr, ChildMapping {
 abstract private class NonExprChildMapping extends ChildMapping {
   NonExprChildMapping() { not this instanceof Expr }
 
-  pragma[nomagic]
-  override predicate reachesBasicBlock(Ast child, CfgNode cfn, BasicBlock bb) {
-    this.relevantChild(child) and
-    cfn.getAstNode() = this and
-    bb.getANode() = cfn
-    or
-    exists(BasicBlock mid |
-      this.reachesBasicBlock(child, cfn, mid) and
-      bb = mid.getASuccessor() and
-      not mid.getANode().getAstNode() = child
-    )
-  }
+  override predicate precedesParent(Ast child) { none() } // this is not final because it is overriden by ForEachStmt
 }
 
 private class AttributeBaseChildMapping extends NonExprChildMapping, AttributeBase {
@@ -1208,7 +1203,7 @@ module StmtNodes {
     StmtCfgNode getBody() { s.hasCfgChild(s.getBody(), this, result) }
   }
 
-  private class LoopStmtChildMapping extends NonExprChildMapping, LoopStmt {
+  abstract private class LoopStmtChildMapping extends ChildMapping, LoopStmt {
     override predicate relevantChild(Ast child) { child = this.getBody() }
   }
 
@@ -1222,7 +1217,9 @@ module StmtNodes {
     StmtCfgNode getBody() { s.hasCfgChild(s.getBody(), this, result) }
   }
 
-  private class DoUntilStmtChildMapping extends LoopStmtChildMapping, DoUntilStmt {
+  private class DoUntilStmtChildMapping extends LoopStmtChildMapping, NonExprChildMapping,
+    DoUntilStmt
+  {
     override predicate relevantChild(Ast child) {
       child = this.getCondition() or super.relevantChild(child)
     }
@@ -1238,7 +1235,9 @@ module StmtNodes {
     ExprCfgNode getCondition() { s.hasCfgChild(s.getCondition(), this, result) }
   }
 
-  private class DoWhileStmtChildMapping extends LoopStmtChildMapping, DoWhileStmt {
+  private class DoWhileStmtChildMapping extends LoopStmtChildMapping, NonExprChildMapping,
+    DoWhileStmt
+  {
     override predicate relevantChild(Ast child) {
       child = this.getCondition() or super.relevantChild(child)
     }
@@ -1300,10 +1299,14 @@ module StmtNodes {
     ExprCfgNode getHashTableExpr() { s.hasCfgChild(s.getHashTableExpr(), this, result) }
   }
 
-  private class ForEachStmtChildMapping extends LoopStmtChildMapping, ForEachStmt {
+  private class ForEachStmtChildMapping extends LoopStmtChildMapping, NonExprChildMapping,
+    ForEachStmt
+  {
     override predicate relevantChild(Ast child) {
       child = this.getVarAccess() or child = this.getIterableExpr() or super.relevantChild(child)
     }
+
+    override predicate precedesParent(Ast child) { child = this.getIterableExpr() }
   }
 
   class ForEachStmtCfgNode extends LoopStmtCfgNode {
@@ -1313,12 +1316,12 @@ module StmtNodes {
 
     override ForEachStmt getStmt() { result = s }
 
-    ExprCfgNode getVarAccess() { s.hasCfgChild(s.getVarAccess(), this, result) }
+    ExprNodes::VarAccessCfgNode getVarAccess() { s.hasCfgChild(s.getVarAccess(), this, result) }
 
     ExprCfgNode getIterableExpr() { s.hasCfgChild(s.getIterableExpr(), this, result) }
   }
 
-  private class ForStmtChildMapping extends LoopStmtChildMapping, ForStmt {
+  private class ForStmtChildMapping extends LoopStmtChildMapping, NonExprChildMapping, ForStmt {
     override predicate relevantChild(Ast child) {
       child = this.getInitializer() or
       child = this.getCondition() or
@@ -1476,7 +1479,7 @@ module StmtNodes {
     override UsingStmt getStmt() { result = s }
   }
 
-  private class WhileStmtChildMapping extends LoopStmtChildMapping, WhileStmt {
+  private class WhileStmtChildMapping extends LoopStmtChildMapping, NonExprChildMapping, WhileStmt {
     override predicate relevantChild(Ast child) {
       child = this.getCondition() or
       super.relevantChild(child)

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

@@ -1025,13 +1025,15 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
  * Holds if there is a read step of content `c` from `node1` to `node2`.
  */
 predicate readStep(Node node1, ContentSet c, Node node2) {
+  // Qualifier -> Member read
   exists(CfgNodes::ExprNodes::MemberExprReadAccessCfgNode var, Content::FieldContent fc |
     node2.asExpr() = var and
     node1.asExpr() = var.getQualifier() and
     fc.getLowerCaseName() = var.getLowerCaseMemberName() and
     c.isSingleton(fc)
   )
   or
+  // Qualifier -> Index read
   exists(CfgNodes::ExprNodes::IndexExprReadAccessCfgNode var, CfgNodes::ExprCfgNode e |
     node2.asExpr() = var and
     node1.asExpr() = var.getBase() and
@@ -1046,32 +1048,48 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
     c.isAnyElement()
   )
   or
+  // Implicit read before a return
   exists(CfgNode cfgNode |
     node1 = TPreReturnNodeImpl(cfgNode, true) and
     node2 = TImplicitWrapNode(cfgNode, true) and
     c.isSingleton(any(Content::KnownElementContent ec | exists(ec.getIndex().asInt())))
   )
   or
+  // Implicit read before a process block
   c.isAnyPositional() and
   exists(CfgNodes::ProcessBlockCfgNode processBlock |
     processBlock.getPipelineParameterAccess() = node1.asExpr() and
     node2 = TProcessNode(processBlock)
   )
   or
+  // Implicit read of a positional before a property-by-name process iteration
   c.isAnyPositional() and
   exists(CfgNodes::ProcessBlockCfgNode pb, CfgNodes::ExprNodes::VarReadAccessCfgNode va |
     va = pb.getAPipelineByPropertyNameParameterAccess() and
     node1.asExpr() = va and
     node2 = TProcessPropertyByNameNode(va.getVariable(), false)
   )
   or
+  // Implicit read of a property before a property-by-name process iteration
   exists(PipelineByPropertyNameParameter p, Content::KnownElementContent ec |
     c.isKnownOrUnknownElement(ec) and
     ec.getIndex().asString() = p.getLowerCaseName() and
     node1 = TProcessPropertyByNameNode(p, false) and
     node2 = TProcessPropertyByNameNode(p, true)
   )
   or
+  // Read from a collection into a `foreach` loop
+  exists(
+    CfgNodes::StmtNodes::ForEachStmtCfgNode forEach, Content::KnownElementContent ec, BasicBlock bb,
+    int i
+  |
+    c.isKnownOrUnknownElement(ec) and
+    node1.asExpr() = forEach.getIterableExpr() and
+    bb.getNode(i) = forEach.getVarAccess() and
+    node2.asDefinition().definesAt(_, bb, i)
+  )
+  or
+  // Summary read steps
   FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
     node2.(FlowSummaryNode).getSummaryNode())
 }

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

@@ -1,6 +1,7 @@
 private import powershell
 private import DataFlowDispatch
 private import DataFlowPrivate
+private import semmle.code.powershell.dataflow.Ssa
 private import semmle.code.powershell.typetracking.internal.TypeTrackingImpl
 private import semmle.code.powershell.ApiGraphs
 private import semmle.code.powershell.Cfg
@@ -13,6 +14,9 @@ class Node extends TNode {
   /** Gets the expression corresponding to this node, if any. */
   CfgNodes::ExprCfgNode asExpr() { result = this.(ExprNode).getExprNode() }
 
+  /** Gets the definition corresponding to this node, if any. */
+  Ssa::Definition asDefinition() { result = this.(SsaDefinitionNodeImpl).getDefinition() }
+
   ScriptBlock asCallable() { result = this.(CallableNode).asCallableAstNode() }
 
   /** Gets the parameter corresponding to this node, if any. */
@@ -477,14 +481,10 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
  *
  * For example, `[Foo]::new()` or `New-Object Foo`.
  */
-class ObjectCreationNode extends ExprNode {
-  CfgNodes::ExprNodes::ObjectCreationCfgNode objectCreation;
-
-  ObjectCreationNode() { this.getExprNode() = objectCreation }
+class ObjectCreationNode extends CallNode {
+  override CfgNodes::ExprNodes::ObjectCreationCfgNode call;
 
-  final CfgNodes::ExprNodes::ObjectCreationCfgNode getObjectCreationNode() {
-    result = objectCreation
-  }
+  final CfgNodes::ExprNodes::ObjectCreationCfgNode getObjectCreationNode() { result = call }
 
   /**
    * Gets the node corresponding to the expression that decides which type
@@ -493,7 +493,7 @@ class ObjectCreationNode extends ExprNode {
    * For example, in `[Foo]::new()`, this would be `Foo`, and in
    * `New-Object Foo`, this would be `Foo`.
    */
-  Node getConstructedTypeNode() { result.asExpr() = objectCreation.getConstructedTypeExpr() }
+  Node getConstructedTypeNode() { result.asExpr() = call.getConstructedTypeExpr() }
 
   bindingset[result]
   pragma[inline_late]

powershell/ql/lib/semmle/code/powershell/frameworks/Microsoft.PowerShell.model.yml

@@ -44,6 +44,9 @@ extensions:
       - ["microsoft.powershell.utility!", "Method[format-wide]", "Argument[-inputobject,pipeline]", "ReturnValue", "taint"]
       - ["microsoft.powershell.utility!", "Method[get-unique]", "Argument[-inputobject,pipeline]", "ReturnValue", "taint"]
       - ["microsoft.powershell.utility!", "Method[join-string]", "Argument[-inputobject,pipeline]", "ReturnValue", "taint"]
+      - ["microsoft.powershell.management!", "Method[join-path]", "Argument[-path,0]", "ReturnValue", "taint"]
+      - ["microsoft.powershell.management!", "Method[join-path]", "Argument[-childpath,1]", "ReturnValue", "taint"]
+      - ["microsoft.powershell.management!", "Method[join-path]", "Argument[-additionalchildpath,2]", "ReturnValue", "taint"]
 
   - addsTo:
       pack: microsoft/powershell-all

powershell/ql/lib/semmle/code/powershell/frameworks/System.IO.model.yml

@@ -30,4 +30,10 @@ extensions:
       - ["system.io.filestream", "Instance", "file"]
       - ["system.io.filestream", "Instance", "file-write"]
       - ["system.io.streamwriter", "Instance", "file-write"]
-      - ["system.io.streamwriter", "Instance", "file-write"]
+      - ["system.io.streamwriter", "Instance", "file-write"]
+
+  - addsTo:
+      pack: microsoft/powershell-all
+      extensible: summaryModel
+    data:
+      - ["system.io.path!", "Method[getfullpath]", "Argument[0]", "ReturnValue", "taint"]