Merge pull request #141 from microsoft/powershell-cmd-injection-fewer-sinks

PS: Improve sinks in `powershell/command-injection`

Author: MathiasVP

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

@@ -326,19 +326,25 @@ module API {
 
   /** A node representing a module/class object with epsilon edges to its descendents. */
   private class ModuleNode extends Node, Impl::MkModule {
-    /** Gets the module represented by this API node. */
-    string getModule() { this = Impl::MkModule(result) }
+    string qualifiedModule;
+    int n;
 
-    override string toString() { result = "Module(" + this.getModule() + ")" }
+    ModuleNode() { this = Impl::MkModule(qualifiedModule, n) }
 
-    TypeNode getType(string name) { result.getType() = this.getModule() + "." + name } // TODO: Check that name exists in module
-  }
+    ModuleNode getNext() { result = Impl::MkModule(qualifiedModule, n + 1) }
+
+    ModuleNode getPred() { result.getNext() = this }
+
+    string getComponent() { result = qualifiedModule.splitAt(".", n) }
 
-  private class TypeNode extends Node, Impl::MkType {
-    /** Gets the type represented by this API node. */
-    string getType() { this = Impl::MkType(result) }
+    string getModule() {
+      not exists(this.getPred()) and
+      result = this.getComponent()
+      or
+      result = this.getPred().getModule() + "." + this.getComponent()
+    }
 
-    override string toString() { result = "Type(" + this.getType() + ")" }
+    override string toString() { result = "Module(" + this.getModule() + ")" }
   }
 
   /** A node representing instances of a module/class with epsilon edges to its ancestors. */
@@ -413,13 +419,7 @@ module API {
    * Gets the node that represents the module with qualified
    * name `qualifiedModule`.
    */
-  ModuleNode mod(string qualifiedModule) { result = Impl::MkModule(qualifiedModule) }
-
-  /**
-   * Gets the node that represents the type with qualified
-   * name `qualifiedType`.
-   */
-  TypeNode type(string qualifiedType) { result = Impl::MkType(qualifiedType) }
+  ModuleNode mod(string qualifiedModule, int n) { result = Impl::MkModule(qualifiedModule, n) }
 
   /**
    * Gets an unqualified call at the top-level with the given method name.
@@ -466,26 +466,43 @@ module API {
 
   cached
   private module Impl {
+    private predicate isGacModule(string s) {
+      s =
+        [
+          "System.Management.Automation",
+          "Microsoft.Management.Infrastructure",
+          "Microsoft.PowerShell.Security",
+          "Microsoft.PowerShell.Commands.Management",
+          "Microsoft.PowerShell.Commands.Utility"
+        ]
+    }
+
+    private predicate isModule(string s, int n) {
+      (
+        any(UsingStmt using).getName() = s
+        or
+        any(Cmd cmd).getNamespaceQualifier() = s
+        or
+        any(TypeNameExpr tn).getName() = s
+        or
+        any(ModuleManifest manifest).getModuleName() = s
+        or
+        isGacModule(s)
+      ) and
+      exists(s.splitAt(".", n))
+    }
+
     cached
     newtype TApiNode =
       /** The root of the API graph. */
       MkRoot() or
       /** The method accessed at `call`, synthetically treated as a separate object. */
       MkMethodAccessNode(DataFlow::CallNode call) or
-      MkModule(string qualifiedModule) {
-        any(UsingStmt using).getName() = qualifiedModule
-        or
-        any(Cmd cmd).getNamespaceQualifier() = qualifiedModule
-        or
-        any(TypeNameExpr tn).getName() = qualifiedModule
-        or
-        any(ModuleManifest manifest).getModuleName() = qualifiedModule
-      } or
-      MkType(string qualifiedType) { any(ConstantValue cv).asString() = qualifiedType } or // TODO
+      MkModule(string qualifiedModule, int n) { isModule(qualifiedModule, n) } or
       /** Instances of `mod` with epsilon edges to its ancestors. */
-      MkInstanceUp(string qualifiedType) { exists(MkType(qualifiedType)) } or
+      MkInstanceUp(string qualifiedType) { exists(MkModule(qualifiedType, _)) } or
       /** Instances of `mod` with epsilon edges to its descendents, and to its upward node. */
-      MkInstanceDown(string qualifiedType) { exists(MkType(qualifiedType)) } or
+      MkInstanceDown(string qualifiedType) { exists(MkModule(qualifiedType, _)) } or
       /** Intermediate node for following forward data flow. */
       MkForwardNode(DataFlow::LocalSourceNode node, TypeTracker t) { isReachable(node, t) } or
       /** Intermediate node for following backward data flow. */
@@ -525,14 +542,6 @@ module API {
       )
     }
 
-    cached
-    predicate typeEdge(Node pred, string name, Node succ) {
-      exists(ModuleNode mod |
-        pred = mod and
-        succ = mod.getType(name)
-      )
-    }
-
     cached
     predicate memberEdge(Node pred, string name, Node succ) {
       exists(MemberExpr member | succ = getForwardStartNode(getNodeFromExpr(member)) |
@@ -546,8 +555,9 @@ module API {
       exists(DataFlow::CallNode call | succ = MkMethodAccessNode(call) and name = call.getName() |
         pred = getForwardEndNode(getALocalSourceStrict(call.getQualifier()))
         or
-        exists(string qualifiedModule, ModuleManifest manifest |
-          pred = mod(qualifiedModule) and
+        exists(string qualifiedModule, ModuleManifest manifest, int n |
+          pred = mod(qualifiedModule, n) and
+          not exists(mod(qualifiedModule, n + 1)) and
           manifest.getModuleName() = qualifiedModule
         |
           manifest.getACmdLetToExport() = name
@@ -647,8 +657,15 @@ module API {
 
     cached
     predicate instanceEdge(Node pred, Node succ) {
-      // An instance of a type
-      exists(string qualifiedType | pred = MkType(qualifiedType) |
+      exists(string qualifiedType, int n |
+        pred = MkModule(qualifiedType, n) and
+        not exists(MkModule(qualifiedType, n + 1))
+      |
+        exists(DataFlow::TypeNameNode typeName |
+          typeName.getTypeName() = qualifiedType and
+          succ = getForwardStartNode(typeName)
+        )
+        or
         exists(DataFlow::ObjectCreationNode objCreation |
           objCreation.getConstructedTypeName() = qualifiedType and
           succ = getForwardStartNode(objCreation)
@@ -659,15 +676,6 @@ module API {
           succ = getForwardStartNode(p)
         )
       )
-      or
-      // A use of a module (or static type?)
-      // TODO: Consider implicit module qualiifers and use instance on all of them
-      exists(string qualifiedType, DataFlow::TypeNameNode typeName |
-        pred = MkModule(qualifiedType) and
-        typeName.getTypeName() = qualifiedType
-      |
-        succ = getForwardStartNode(typeName)
-      )
     }
 
     cached

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

@@ -32,7 +32,17 @@ class Cmd extends @command, CmdBase {
   predicate isQualified() { parseCommandName(this, any(string s | s != ""), _) }
 
   /** Gets the namespace qualifier of this command, if any. */
-  string getNamespaceQualifier() { parseCommandName(this, result, _) }
+  string getNamespaceQualifier() {
+    result != "" and
+    parseCommandName(this, result, _)
+    or
+    // Implicit import because it's in a module manifest
+    parseCommandName(this, "", _) and
+    exists(ModuleManifest manifest |
+      manifest.getACmdLetToExport() = this.getCommandName() and
+      result = manifest.getModuleName()
+    )
+  }
 
   /** Gets the (possibly qualified) name of this command. */
   string getQualifiedCommandName() { command(this, result, _, _, _) }

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

@@ -2,3 +2,6 @@
  * Helper file that imports all framework modeling.
  */
 
+import semmle.code.powershell.frameworks.SystemManagementAutomationRunspaces.Runspaces
+import semmle.code.powershell.frameworks.SystemManagementAutomationPowerShell.PowerShell
+import semmle.code.powershell.frameworks.SystemManagementAutomationEngineIntrinsics.EngineIntrinsics

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

@@ -610,7 +610,13 @@ module StmtNodes {
 
     final override ExprCfgNode getCommand() { s.hasCfgChild(s.getCommand(), this, result) }
 
-    final override string getName() { result = s.getCmdName().getValue().getValue() }
+    final override string getName() { result = s.getCommandName() }
+
+    /** Holds if the command is qualified. */
+    predicate isQualified() { s.isQualified() }
+
+    /** Gets the namespace qualifier of this command, if any. */
+    string getNamespaceQualifier() { result = s.getNamespaceQualifier() }
   }
 
   /** A control-flow node that wraps a call to operator `&` */

powershell/ql/lib/semmle/code/powershell/dataflow/FlowSummary.qll

@@ -13,6 +13,9 @@ private import internal.DataFlowPrivate
 private module Summaries {
   private import semmle.code.powershell.Frameworks
   private import semmle.code.powershell.frameworks.data.ModelsAsData
+  import RunspaceFactory
+  import PowerShell
+  import EngineIntrinsics
 }
 
 /** A callable with a flow summary, identified by a unique string. */

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

@@ -202,6 +202,7 @@ private module Cached {
       isProcessPropertyByNameNode(iter, _)
     } or
     TScriptBlockNode(ScriptBlock scriptBlock) or
+    TTypePathNode(int n, CfgNode cfg) { isTypePathNode(_, n, cfg) } or
     TForbiddenRecursionGuard() {
       none() and
       // We want to prune irrelevant models before materialising data flow nodes, so types contributed
@@ -288,10 +289,24 @@ private module Cached {
     TypeTrackingInput::withoutContentStepImpl(_, n, _)
   }
 
+  private predicate isAutomaticVariable(Node n) {
+    n.asExpr().(CfgNodes::ExprNodes::VarReadAccessCfgNode).getVariable().getName() =
+      [
+        "args", "ConsoleFileName", "EnabledExperimentalFeatures", "Error", "Event", "EventArgs",
+        "EventSubscriber", "ExecutionContext", "HOME", "Host", "input", "IsCoreCLR", "IsLinux",
+        "IsMacOS", "IsWindows", "LASTEXITCODE", "MyInvocation", "NestedPromptLevel", "PID",
+        "PROFILE", "PSBoundParameters", "PSCmdlet", "PSCommandPath", "PSCulture", "PSDebugContext",
+        "PSEdition", "PSHOME", "PSItem", "PSScriptRoot", "PSSenderInfo", "PSUICulture",
+        "PSVersionTable", "PWD", "Sender", "ShellId", "StackTrace"
+      ]
+  }
+
   cached
   predicate isLocalSourceNode(Node n) {
     n instanceof ParameterNode
     or
+    isAutomaticVariable(n)
+    or
     // Expressions that can't be reached from another entry definition or expression
     (
       n instanceof ExprNode
@@ -1148,6 +1163,64 @@ class ScriptBlockNode extends TScriptBlockNode, NodeImpl {
   override predicate nodeIsHidden() { any() }
 }
 
+private predicate isTypePathNode(string type, int n, CfgNode cfg) {
+  exists(CfgNodes::ExprNodes::TypeNameCfgNode typeName, string s |
+    cfg = typeName and
+    type = typeName.getTypeName() and
+    s = type.splitAt(".", n)
+  )
+  or
+  exists(CfgNodes::StmtNodes::CmdCfgNode cmd, string s |
+    cfg = cmd.getCommand() and
+    type = cmd.getNamespaceQualifier() and
+    s = type.splitAt(".", n)
+  )
+}
+
+/**
+ * A dataflow node that represents a component of a type or module path.
+ *
+ * For example, `System`, `System.Management`, `System.Management.Automation`,
+ * and `System.Management.Automation.PowerShell` in the type
+ * name `[System.Management.Automation.PowerShell]`.
+ */
+class TypePathNodeImpl extends TTypePathNode, NodeImpl {
+  int n;
+  CfgNode cfg;
+
+  TypePathNodeImpl() { this = TTypePathNode(n, cfg) }
+
+  string getType() { isTypePathNode(result, n, cfg) }
+
+  predicate isComplete() { not exists(this.getNext()) }
+
+  int getIndex() { result = n }
+
+  string getComponent() { result = this.getType().splitAt(".", n) }
+
+  override CfgScope getCfgScope() { result = cfg.getScope() }
+
+  override Location getLocationImpl() { result = cfg.getLocation() }
+
+  override string toStringImpl() {
+    not exists(this.getPrev()) and
+    result = this.getComponent()
+    or
+    result = this.getPrev() + "." + this.getComponent()
+  }
+
+  override predicate nodeIsHidden() { any() }
+
+  TypePathNodeImpl getNext() { result = TTypePathNode(n + 1, cfg) }
+
+  TypePathNodeImpl getPrev() { result.getNext() = this }
+
+  TypePathNodeImpl getConstant(string s) {
+    s = result.getComponent() and
+    result = this.getNext()
+  }
+}
+
 /** A node that performs a type cast. */
 class CastNode extends Node {
   CastNode() { none() }

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

@@ -193,6 +193,21 @@ class PostUpdateNode extends Node {
   Node getPreUpdateNode() { result = pre }
 }
 
+/**
+ * A dataflow node that represents a component of a type or module path.
+ *
+ * For example, `System`, `System.Management`, `System.Management.Automation`,
+ * and `System.Management.Automation.PowerShell` in the type
+ * name `[System.Management.Automation.PowerShell]`.
+ */
+class TypePathNode extends Node instanceof TypePathNodeImpl {
+  string getComponent() { result = super.getComponent() }
+
+  TypePathNode getConstant(string s) { result = super.getConstant(s) }
+
+  API::Node track() { result = API::mod(super.getType(), super.getIndex()) }
+}
+
 cached
 private module Cached {
   cached

powershell/ql/lib/semmle/code/powershell/frameworks/MicrosoftPowershellUtility/model.yml

@@ -3,4 +3,4 @@ extensions:
       pack: microsoft-sdl/powershell-all
       extensible: sourceModel
     data:
-      - ["Microsoft.PowerShell.Utility", "Method[Read-Host].ReturnValue", "stdin"]
+      - ["Microsoft.PowerShell.Utility!", "Method[Read-Host].ReturnValue", "stdin"]

powershell/ql/lib/semmle/code/powershell/frameworks/MicrosoftWin32Registry/model.yml

@@ -3,4 +3,4 @@ extensions:
       pack: microsoft-sdl/powershell-all
       extensible: sourceModel
     data:
-      - ["Microsoft.Win32.Registry", "Method[GetValue]", "windows-registry"]
+      - ["Microsoft.Win32.Registry!", "Method[GetValue]", "windows-registry"]

powershell/ql/lib/semmle/code/powershell/frameworks/MicrosoftWin32RegistryKey/model.yml

@@ -3,6 +3,6 @@ extensions:
       pack: microsoft-sdl/powershell-all
       extensible: sourceModel
     data:
-      - ["Microsoft.Win32.RegistryKey", "Instance.Method[GetValue].ReturnValue", "windows-registry"]
-      - ["Microsoft.Win32.RegistryKey", "Instance.Method[GetValueNames].ReturnValue", "windows-registry"]
-      - ["Microsoft.Win32.RegistryKey", "Instance.Method[GetSubKeyNames].ReturnValue", "windows-registry"]
+      - ["Microsoft.Win32.RegistryKey", "Method[GetValue].ReturnValue", "windows-registry"]
+      - ["Microsoft.Win32.RegistryKey", "Method[GetValueNames].ReturnValue", "windows-registry"]
+      - ["Microsoft.Win32.RegistryKey", "Method[GetSubKeyNames].ReturnValue", "windows-registry"]