diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPrivate.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPrivate.qll index 695905192432..2a61ba2cf210 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/TaintTrackingPrivate.qll +++ b/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 diff --git a/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll b/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll index a7588ce2d371..c3ae436f5477 100644 --- a/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll +++ b/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,7 +39,7 @@ 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 { @@ -40,12 +47,24 @@ module SqlInjection { 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 { diff --git a/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionQuery.qll b/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionQuery.qll index 752b691c54aa..6738f0cb59cc 100644 --- a/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionQuery.qll +++ b/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) + } } /** diff --git a/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected index 7e7a54f4ab19..86a4223d4baa 100644 --- a/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected +++ b/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 | diff --git a/powershell/ql/test/query-tests/security/cwe-089/test.ps1 b/powershell/ql/test/query-tests/security/cwe-089/test.ps1 index f7b8f400978d..265c7110427b 100644 --- a/powershell/ql/test/query-tests/security/cwe-089/test.ps1 +++ b/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() \ No newline at end of file +$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 \ No newline at end of file