Merge pull request #282 from microsoft/sql-injection-powershell-more-barriers

PS: Remove from FPs from the SQL injection query.

Author: chanel-y

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

@@ -73,7 +73,7 @@ module SqlInjection {
 
     override predicate allowImplicitRead(DataFlow::ContentSet cs) {
       cs.getAStoreContent().(DataFlow::Content::KnownKeyContent).getIndex().asString().toLowerCase() =
-        ["query", "inputfile"]
+        query()
     }
   }
 
@@ -106,4 +106,15 @@ module SqlInjection {
   }
 
   class TypeSanitizer extends Sanitizer instanceof SimpleTypeSanitizer { }
+
+  class ValidateAttributeSanitizer extends Sanitizer {
+    ValidateAttributeSanitizer() {
+      exists(Function f, Attribute a, Parameter p |
+        p = f.getAParameter() and
+        p.getAnAttribute() = a and
+        a.getAName() = ["ValidateScript", "ValidateSet", "ValidatePattern"] and
+        this.asParameter() = p
+      )
+    }
+  }
 }

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

@@ -4,12 +4,15 @@ edges
 | test.ps1:1:1:1:10 | userinput | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance |  |
 | 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: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 |  |
 | test.ps1:72:1:72:11 | QueryConn2 [element Query] | test.ps1:81:15:81:25 | QueryConn2 | provenance |  |
 | test.ps1:72:15:79:1 | ${...} [element Query] | test.ps1:72:1:72:11 | QueryConn2 [element Query] | provenance |  |
 | test.ps1:78:13:78:22 | userinput | test.ps1:72:15:79:1 | ${...} [element Query] | provenance |  |
+| test.ps1:121:9:121:56 | unvalidated | test.ps1:125:92:125:103 | unvalidated | provenance |  |
+| test.ps1:128:28:128:37 | userinput | test.ps1:121:9:121:56 | unvalidated | provenance |  |
 nodes
 | test.ps1:1:1:1:10 | userinput | semmle.label | userinput |
 | test.ps1:1:14:1:45 | Call to read-host | semmle.label | Call to read-host |
@@ -23,10 +26,14 @@ nodes
 | 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 |
+| 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 |
 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 |
 | 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 |
+| 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 |

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

@@ -111,4 +111,29 @@ TakesTypedParameters $userinput $userinput $userinput $userinput $userinput $use
 $query = "SELECT * FROM MyTable WHERE MyColumn = '$userinput'"
 Invoke-Sqlcmd -unknown $userinput -ServerInstance "MyServer" -Database "MyDatabase" -q "SELECT * FROM MyTable" # GOOD
 
-Invoke-Sqlcmd -ServerInstance "MyServer" -Database "MyDatabase" -InputFile $userinput # GOOD # this is not really what this query is about.
+Invoke-Sqlcmd -ServerInstance "MyServer" -Database "MyDatabase" -InputFile $userinput # GOOD # this is not really what this query is about.
+
+function With-Validation() {
+    param(
+        [ValidateSet("FirstName","LastName")]
+        [parameter(Mandatory=$true)][string]$validated,
+
+        [parameter(Mandatory=$true)][string]$unvalidated
+    )
+
+    Invoke-Sqlcmd -unknown $userinput -ServerInstance "MyServer" -Database "MyDatabase" -q $validated # GOOD
+    Invoke-Sqlcmd -unknown $userinput -ServerInstance "MyServer" -Database "MyDatabase" -q $unvalidated # BAD
+}
+
+With-Validation $userinput $userinput
+
+$QueryConn3 = @{
+    Database = "MyDB"
+    ServerInstance = "MyServer"
+    Username = "MyUserName"
+    Password = "MyPassword"
+    ConnectionTimeout = 0
+    inputfile = $userinput
+}
+
+Invoke-Sqlcmd @QueryConn3 # GOOD