PS: Fix FPs on powershell/microsoft/public/sql-injection

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

@@ -18,7 +18,7 @@ predicate defaultTaintSanitizer(DataFlow::Node node) { none() }
 bindingset[node]
 predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) {
   node instanceof ArgumentNode and
-  c.isAnyElement()
+  c.isAnyPositional()
 }
 
 cached

powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll

@@ -22,7 +22,14 @@ module SqlInjection {
    * A data flow sink for SQL-injection vulnerabilities.
    */
   abstract class Sink extends DataFlow::Node {
+    /** Gets a description of this sink. */
     abstract string getSinkType();
+
+    /**
+     * Holds if this sink should allow for an implicit read of `cs` when
+     * reached.
+     */
+    predicate allowImplicitRead(DataFlow::ContentSet cs) { none() }
   }
 
   /**
@@ -32,20 +39,32 @@ module SqlInjection {
 
   /** A source of user input, considered as a flow source for command injection. */
   class FlowSourceAsSource extends Source instanceof SourceNode {
-    override string getSourceType() { result = "user-provided value" }
+    override string getSourceType() { result = SourceNode.super.getSourceType() }
   }
 
   class InvokeSqlCmdSink extends Sink {
     InvokeSqlCmdSink() {
       exists(DataFlow::CallNode call | call.matchesName("Invoke-Sqlcmd") |
         this = call.getNamedArgument("query")
         or
+        this = call.getNamedArgument("inputfile")
+        or
         not call.hasNamedArgument("query") and
+        not call.hasNamedArgument("inputfile") and
         this = call.getArgument(0)
+        or
+        // TODO: Here we really should pick a splat argument, but we don't yet extract whether an
+        // argument is a splat argument.
+        this = unique( | | call.getAnArgument())
       )
     }
 
     override string getSinkType() { result = "call to Invoke-Sqlcmd" }
+
+    override predicate allowImplicitRead(DataFlow::ContentSet cs) {
+      cs.getAStoreContent().(DataFlow::Content::KnownKeyContent).getIndex().asString().toLowerCase() =
+        ["query", "inputfile"]
+    }
   }
 
   class ConnectionStringWriteSink extends Sink {

powershell/ql/lib/semmle/code/powershell/security/SqlInjectionQuery.qll

@@ -18,6 +18,10 @@ private module Config implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
 
   predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+
+  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
+    node.(Sink).allowImplicitRead(cs)
+  }
 }
 
 /**

powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected

@@ -3,15 +3,22 @@ edges
 | test.ps1:1:14:1:45 | Call to read-host | test.ps1:9:72:9:77 | query | provenance | Src:MaD:0  |
 | test.ps1:1:14:1:45 | Call to read-host | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | Src:MaD:0  |
 | test.ps1:1:14:1:45 | Call to read-host | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | Src:MaD:0  |
+| test.ps1:1:14:1:45 | Call to read-host | test.ps1:78:13:78:22 | userinput | provenance | Src:MaD:0  |
+| test.ps1:72:15:79:1 | ${...} [element Query] | test.ps1:81:15:81:25 | QueryConn2 | provenance |  |
+| test.ps1:78:13:78:22 | userinput | test.ps1:72:15:79:1 | ${...} [element Query] | provenance |  |
 nodes
 | test.ps1:1:14:1:45 | Call to read-host | semmle.label | Call to read-host |
 | test.ps1:5:72:5:77 | query | semmle.label | query |
 | test.ps1:9:72:9:77 | query | semmle.label | query |
 | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | semmle.label | SELECT * FROM MyTable WHERE MyColumn = '$userinput' |
 | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | semmle.label | SELECT * FROM MyTable WHERE MyColumn = '$userinput' |
+| test.ps1:72:15:79:1 | ${...} [element Query] | semmle.label | ${...} [element Query] |
+| test.ps1:78:13:78:22 | userinput | semmle.label | userinput |
+| test.ps1:81:15:81:25 | QueryConn2 | semmle.label | QueryConn2 |
 subpaths
 #select
-| test.ps1:5:72:5:77 | query | test.ps1:1:14:1:45 | Call to read-host | test.ps1:5:72:5:77 | query | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | user-provided value |
-| test.ps1:9:72:9:77 | query | test.ps1:1:14:1:45 | Call to read-host | test.ps1:9:72:9:77 | query | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | user-provided value |
-| test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:1:14:1:45 | Call to read-host | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | user-provided value |
-| test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:1:14:1:45 | Call to read-host | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | user-provided value |
+| test.ps1:5:72:5:77 | query | test.ps1:1:14:1:45 | Call to read-host | test.ps1:5:72:5:77 | query | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin |
+| test.ps1:9:72:9:77 | query | test.ps1:1:14:1:45 | Call to read-host | test.ps1:9:72:9:77 | query | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin |
+| test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:1:14:1:45 | Call to read-host | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin |
+| test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:1:14:1:45 | Call to read-host | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin |
+| test.ps1:81:15:81:25 | QueryConn2 | test.ps1:1:14:1:45 | Call to read-host | test.ps1:81:15:81:25 | QueryConn2 | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin |

powershell/ql/test/query-tests/security/cwe-089/test.ps1

@@ -53,4 +53,29 @@ $parameter = $command.Parameters.Add("?", [System.Data.OleDb.OleDbType]::VarChar
 $parameter.Value = $userinput # GOOD
 $reader = $command.ExecuteReader()
 $reader.Close()
-$connection.Close()
+$connection.Close()
+
+$server = $Env:SERVER_INSTANCE
+Invoke-Sqlcmd -ServerInstance $server -Database "MyDatabase" -InputFile "Foo/Bar/query.sql" # GOOD
+
+$QueryConn = @{
+    Database = "MyDB"
+    ServerInstance = $server
+    Username = "MyUserName"
+    Password = "MyPassword"
+    ConnectionTimeout = 0
+    Query = ""
+}
+
+Invoke-Sqlcmd @QueryConn # GOOD
+
+$QueryConn2 = @{
+    Database = "MyDB"
+    ServerInstance = "MyServer"
+    Username = "MyUserName"
+    Password = "MyPassword"
+    ConnectionTimeout = 0
+    Query = $userinput
+}
+
+Invoke-Sqlcmd @QueryConn2 # BAD