From e1efae576a66d06a64df3cbd17b34466539f574d Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 11 Sep 2025 18:32:33 +0100 Subject: [PATCH 1/3] PS: Add another SQL injection FP. --- .../test/query-tests/security/cwe-089/SqlInjection.expected | 3 +++ powershell/ql/test/query-tests/security/cwe-089/test.ps1 | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) 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 b99d45072abb..0b680bf5bea8 100644 --- a/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected +++ b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected @@ -5,6 +5,7 @@ edges | test.ps1:1:1:1:10 | userinput | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | | | test.ps1:1:1:1:10 | userinput | test.ps1:78:13:78:22 | userinput | provenance | | | test.ps1:1:1:1:10 | userinput | test.ps1:128:28:128:37 | userinput | provenance | | +| test.ps1:1:1:1:10 | userinput | test.ps1:141:15:141:24 | userinput | provenance | | | test.ps1:1:14:1:45 | Call to read-host | test.ps1:1:1:1:10 | userinput | provenance | Src:MaD:0 | | test.ps1:4:1:4:6 | query | test.ps1:5:72:5:77 | query | provenance | | | test.ps1:8:1:8:6 | query | test.ps1:9:72:9:77 | query | provenance | | @@ -29,6 +30,7 @@ nodes | test.ps1:121:9:121:56 | unvalidated | semmle.label | unvalidated | | test.ps1:125:92:125:103 | unvalidated | semmle.label | unvalidated | | test.ps1:128:28:128:37 | userinput | semmle.label | userinput | +| test.ps1:141:15:141:24 | userinput | semmle.label | userinput | 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 | read from stdin | @@ -37,3 +39,4 @@ subpaths | 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 | | test.ps1:125:92:125:103 | unvalidated | test.ps1:1:14:1:45 | Call to read-host | test.ps1:125:92:125:103 | unvalidated | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | +| test.ps1:141:15:141:24 | userinput | test.ps1:1:14:1:45 | Call to read-host | test.ps1:141:15:141:24 | userinput | 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 b5c4527153e5..58adcd24eaf9 100644 --- a/powershell/ql/test/query-tests/security/cwe-089/test.ps1 +++ b/powershell/ql/test/query-tests/security/cwe-089/test.ps1 @@ -136,4 +136,6 @@ $QueryConn3 = @{ inputfile = $userinput } -Invoke-Sqlcmd @QueryConn3 # GOOD \ No newline at end of file +Invoke-Sqlcmd @QueryConn3 # GOOD + +&sqlcmd -e -S $userinput -U "Login" -P "MyPassword" -d "MyDBName" -i "input_file.sql" # GOOD [FALSE POSITIVE] \ No newline at end of file From e566145602e180f9b8417d6f668b3d487650212a Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 11 Sep 2025 19:20:59 +0100 Subject: [PATCH 2/3] PS: Use the same sinks for '&sqlcmd' as we do for 'Invoke-SqlCmd'. --- .../security/SqlInjectionCustomizations.qll | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll b/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll index 1ed256440c81..ac63a3267871 100644 --- a/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll +++ b/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll @@ -52,20 +52,27 @@ module SqlInjection { private string inputfile() { result = ["inputfile", "i"] } + bindingset[call] + pragma[inline_late] + private predicate sqlCmdSinkCommon(DataFlow::CallNode call, DataFlow::Node s) { + s = call.getNamedArgument(query()) + or + // If the input is not provided as a query parameter or an input file + // parameter then it's the first argument. + not call.hasNamedArgument(query()) and + not call.hasNamedArgument(inputfile()) and + s = 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. + s = unique( | | call.getAnArgument()) + } + class InvokeSqlCmdSink extends Sink { InvokeSqlCmdSink() { - exists(DataFlow::CallNode call | call.matchesName("Invoke-Sqlcmd") | - this = call.getNamedArgument(query()) - or - // If the input is not provided as a query parameter or an input file - // parameter then it's the first argument. - 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()) + exists(DataFlow::CallNode call | + call.matchesName("Invoke-Sqlcmd") and + sqlCmdSinkCommon(call, this) ) } @@ -94,11 +101,17 @@ module SqlInjection { override string getSinkType() { result = "write to " + memberName } } + /** + * A call of the form `&sqlcmd`. + * + * We don't know if this is actually a call to an SQL execution procedure, but it is + * very common to define a `sqlcmd` variable and point it to an SQL execution program. + */ class SqlCmdSink extends Sink { SqlCmdSink() { exists(DataFlow::CallOperatorNode call | call.getCommand().asExpr().getValue().stringMatches("sqlcmd") and - call.getAnArgument() = this + sqlCmdSinkCommon(call, this) ) } From c8eb73450cf3966126f9727b908f4b3b8d74d7e6 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 11 Sep 2025 19:21:23 +0100 Subject: [PATCH 3/3] PS: Accept test changes. --- .../ql/test/query-tests/security/cwe-089/SqlInjection.expected | 3 --- powershell/ql/test/query-tests/security/cwe-089/test.ps1 | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) 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 0b680bf5bea8..b99d45072abb 100644 --- a/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected +++ b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected @@ -5,7 +5,6 @@ edges | test.ps1:1:1:1:10 | userinput | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | | | test.ps1:1:1:1:10 | userinput | test.ps1:78:13:78:22 | userinput | provenance | | | test.ps1:1:1:1:10 | userinput | test.ps1:128:28:128:37 | userinput | provenance | | -| test.ps1:1:1:1:10 | userinput | test.ps1:141:15:141:24 | userinput | provenance | | | test.ps1:1:14:1:45 | Call to read-host | test.ps1:1:1:1:10 | userinput | provenance | Src:MaD:0 | | test.ps1:4:1:4:6 | query | test.ps1:5:72:5:77 | query | provenance | | | test.ps1:8:1:8:6 | query | test.ps1:9:72:9:77 | query | provenance | | @@ -30,7 +29,6 @@ nodes | test.ps1:121:9:121:56 | unvalidated | semmle.label | unvalidated | | test.ps1:125:92:125:103 | unvalidated | semmle.label | unvalidated | | test.ps1:128:28:128:37 | userinput | semmle.label | userinput | -| test.ps1:141:15:141:24 | userinput | semmle.label | userinput | 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 | read from stdin | @@ -39,4 +37,3 @@ subpaths | 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 | | test.ps1:125:92:125:103 | unvalidated | test.ps1:1:14:1:45 | Call to read-host | test.ps1:125:92:125:103 | unvalidated | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | read from stdin | -| test.ps1:141:15:141:24 | userinput | test.ps1:1:14:1:45 | Call to read-host | test.ps1:141:15:141:24 | userinput | 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 58adcd24eaf9..e97ba8d3676c 100644 --- a/powershell/ql/test/query-tests/security/cwe-089/test.ps1 +++ b/powershell/ql/test/query-tests/security/cwe-089/test.ps1 @@ -138,4 +138,4 @@ $QueryConn3 = @{ Invoke-Sqlcmd @QueryConn3 # GOOD -&sqlcmd -e -S $userinput -U "Login" -P "MyPassword" -d "MyDBName" -i "input_file.sql" # GOOD [FALSE POSITIVE] \ No newline at end of file +&sqlcmd -e -S $userinput -U "Login" -P "MyPassword" -d "MyDBName" -i "input_file.sql" # GOOD \ No newline at end of file