Powershell vs SQL injection attacks.

I recently was maintaining someone else's PHP code base when we realized the codebase was vulnerable to SQL injection attacks... If you're not familiar with SQL injection attacks it's where a field, most likely a string, that gets passed to the code has a specially crafted string at the end to execute code that the programmer did not intend to have executed. There are several ways to combat this, such as strictly setting the variable type and other constraints, and or parameterizing the SQL query.

This got me thinking about my massive library of Powershell scripts, some of which do go access SQL in what I now know is an insecure method. In PHP there is a method of parameterizing the dynamic SQL with a question mark, like so:

$query = "SELECT * FROM [table] WHERE field1= ? AND field2= ?";
$params=array($variable1, $variable2);
$res = sqlsrv_query($conn, $query, $params);

Parameterizing the SQL in PHP protects it from SQL injection because the SQL server can't execute a parameter even if it is a malformed string.

So a quick Google search for how to do that in Powershell resulted in... not much...

There were some other modules out there that did SQL parameterization, but none of them did it in such a dynamic/generic way as I was able to do in PHP to prevent the vulnerability.

My old method for calling SQL from Powershell was this:

function invoke-sql{
    param( [string] $dataSource,
            [string] $database,
            [string] $sqlCommand
          )
        $connectionString = "Data Source=$dataSource; " +
                    "Integrated Security=SSPI; " +
                    "Initial Catalog=$database"

        $connection = new-object system.data.SqlClient.SQLConnection($connectionString)
        $command = new-object system.data.sqlclient.sqlcommand($sqlCommand,$connection)
        $connection.Open()

        $adapter = New-Object System.Data.sqlclient.sqlDataAdapter $command
        $dataset = New-Object System.Data.DataSet
        $adapter.Fill($dataSet) | Out-Null

        $connection.Close()
        $dataSet.Tables
    }

Which, if you're building a dynamic query from some input is absolutely vulnerable to SQL injection unless you take other methods to constrain those inputs, and even if you do, the best practice is still to use parameterization. If you are not using inputs to dynamically build your SQL, that function should be perfectly secure. After scanning all the places I use my invoke-sql function I only found 2 scripts where SQL injection potentially could be an issue.

Since I know parameterization of SQL is possible in Powershell, we need to focus on the system.data.sqlclient.sqlcommand object and how that works. A quick lookup on Microsoft's documentation shows that it does work with parameters, but they have to be named which is not very conducive to a function that could have none or many parameters.

The syntax for adding a parameter by name looks like this:

$command.Parameters.AddWithValue("@foo",$value)

So, therefore, my goal was to interpret the SQL and the parameters passed to create the array of parameters in a way that the built-in SQL objects can work with, and also I need to change the SQL query so that each '?' is a distinctly named variable.

It's not just about finding all the question marks in the string, but because we are going to be manipulating the SQL query string and inserting characters we will need to make those changes from the end backward. We can use Regex to find the matches and sort to order them the way we want them:

$mymatches=$([regex]::matches($sqlcommand,'\?'))|sort index -desc

Then we can loop through the query string replacing each question mark with a named variable

        $counter=$mymatches.length
        foreach ($match in $mymatches){
            $sqlcommand=$sqlcommand.substring(0,$match.index) +"@a$counter"+ $sqlcommand.substring($match.index+1)
            $counter--
        }

So each '?' gets replaced with '@a' and a number.

We need to then increase the counter up by one since it should be at 0 at the moment, and then we can create the command parameters:

        $counter=1
        $command = new-object system.data.sqlclient.sqlcommand($sqlCommand,$connection)

        foreach ($p in $parameters){
            $command.Parameters.AddWithValue("@a$counter",$p)#|out-null
            $counter++
        }

In the end, there are some instances where you may not want or need to build dynamic SQL, so my overall invoke-SQL command should still work without the parameters being passed in, which also makes it backward compatible until I can go correct those two scripts to utilize the new secure functionality. After a few more tweaks here is the new full function allowing parameterized SQL with question marks:

function invoke-sql{<#
    .SYNOPSIS
    Runs a SQL query

    .DESCRIPTION
    Connection variables and a SQL string, optionally an array of parameters are passed to the function, which then connects to the designated SQL instance and performs the query.
    Because a dynamically generated SQL statement is NOT secure and vulnerable to SQL injection attacks, unless otherwise handled, use the parameterized option for passing variables.

    .PARAMETER Datasource
    A string of the datasource, AKA the server name. Required. Defaults to the first position

    .PARAMETER Database
    A string of the database name. Required. Defaults to the second position

    .PARAMETER SQLCommand
    A string of the query to run. Required. Defaults to the third position. When used as a parameterized query the paramters should be replaced with question marks.

    .PARAMETER Parameters
    An array of variables that the function will parameterize for the SQL server. Optional. Defaults to fourth position

    .EXAMPLE
    $sqlcommand="Select * from table"
    invoke-sql localhost $database $sqlcommand

    .EXAMPLE
    $sqlcommand= "Select * from table where field=? or field =?"
    $params=@("value1","value2")
    invoke-sql localhost $database $sqlcommand $params

    #>
    [CmdletBinding()]
    param( 
        [parameter(position=0, mandatory=$true)]
        [string]$datasource,
        [parameter(position=1, mandatory=$true)]
        [string]$database,
        [parameter(position=2, mandatory=$true)]
        [string] $sqlCommand,
        [parameter(position=3)]
        [array]$parameters
    )
    $connectionString = "Data Source=$dataSource; " +
                "Integrated Security=SSPI; " +
                "Initial Catalog=$database"

    $connection = new-object system.data.SqlClient.SQLConnection($connectionString)

    if($null -eq $parameters){
        $command = new-object system.data.sqlclient.sqlcommand($sqlCommand,$connection)
     }
     else {
        #the user has sent a parameterized query. Search the query for the character ? and replace with a @a# where # corresponds to which ? in the query it is.
        $mymatches=$([regex]::matches($sqlcommand,'\?'))|sort index -desc #we want to start at the end of the string so as we adjust the string count it doesn't break things
        if($parameters.Length -ne $mymatches.length){throw "Parameter counts do not match."} #error check, make sure what the query expects for parameters is the same number of params entered
        $counter=$mymatches.length
        foreach ($match in $mymatches){
            $sqlcommand=$sqlcommand.substring(0,$match.index) +"@a$counter"+ $sqlcommand.substring($match.index+1)
            $counter--
        }
        $counter=1 #counter should be reset to 1 to fill in the parameters
        $command = new-object system.data.sqlclient.sqlcommand($sqlCommand,$connection)

        foreach ($p in $parameters){
            $command.Parameters.AddWithValue("@a$counter",$p)|out-null
            $counter++
        }
    }
    $connection.Open()
    $adapter = New-Object System.Data.sqlclient.sqlDataAdapter $command
    $dataset = New-Object System.Data.DataSet
    $adapter.Fill($dataSet) | Out-Null

    $connection.Close()
    $dataSet.tables
}
<#
#--test vs sql injection--#
invoke-sql $datasource $database "select "1;drop TABLE [table_name1]"
invoke-sql $datasource $database "select ?" @("1;drop TABLE [table_name2]
#--end test vs sql injection--#
#>