2

I am lazy and would rather parameterize once (to get an object_id for a name) and use Invoke-SqlCmd for the rest of the work.

Given a series of functions meant to work with an arbitrary server object, is the below test enough to confirm that CmdletBinding an input parameter as Int32 prevents injection by way of the user-supplied $object_id?

function foo {
    [CmdletBinding()]Param(
        [Int32]$object_id 
    )
    $query = "select type_desc from sys.objects where object_id = $object_id;"
    Invoke-Sqlcmd -ServerInstance "localhost" -Database "tempdb" -Query $query
}

foo 3
foo "0 union all select name from sys.syslogins where sid = 0x01"
foo $null
foo ([math]::Pow(2,31)+1)
foo @(1,2)
  • If you implement your query as a stored proc which expectsan integer as a parameter.. it will fail if it gets anything else. – Sir Swears-a-lot May 12 '18 at 10:14
  • The queries are intended to function against arbitrary environments. I don’t want to require deployment of all the required code as a series of stored procs to a given database prior to execution. Just double-checking my assumptions about type casting in my PS cmdlets – Peter Vandivier May 12 '18 at 10:44
5

You can, but there's problems with doing that.

Primarily, it's still not a great idea because the pattern isn't extendable. It only works for certain data types. As soon as you have a string parameter, you're back where you were with injection. It also means that you have to care about how certain data types (byte arrays, GUIDs, datetimes) get cast to strings.

You also have to be aware that null-valued parameters will get silently modified to 0 since [int32] is a non-nullable type. You'd have to specify [nullable[int32]] to prevent that behavior. Even then, since null values that are cast to a string will result in an empty string, you get a much more descriptive error with a parameter.

Would you rather debug this:

Invoke-Sqlcmd : Incorrect syntax near ';'.

Or this:

Exception calling "Fill" with "1" argument(s): "The parameterized query '(@object_id int)select type_desc from sys.objects where object_i' expects the parameter '@object_id', which was not supplied."

Finally, if you're going to use the query a lot it might be harder for the system to reuse the query plan.

It's really not that difficult to specify once you're used to it:

function foo {
    [CmdletBinding()]
    Param(
        $object_id 
    )

    $Query = 'select type_desc from sys.objects where object_id = @object_id;'
    $ConnectionString = 'Data Source={0};Initial Catalog={1};Integrated Security=SSPI' -f 'localhost', 'tempdb'

    $SqlConnection = New-Object -TypeName System.Data.SqlClient.SqlConnection -ArgumentList $ConnectionString

    $SqlCommand = New-Object -TypeName System.Data.SqlClient.SqlCommand -ArgumentList $Query, $SqlConnection
    $SqlCommand.Parameters.Add('@object_id', [System.Data.SqlDbType]::Int).Value = $object_id

    $SqlDataAdapter = New-Object -TypeName System.Data.SqlClient.SqlDataAdapter -ArgumentList $SqlCommand

    $DataTable = New-Object -TypeName System.Data.DataTable -ArgumentList 'objects'

    [void]$SqlDataAdapter.Fill($DataTable)
    $SqlConnection.Dispose()

    , $DataTable
}

[If you're fine with outputting DataRows only, you can drop the comma from the last line. The comma forces the function to output the DataTable whole. Without the comma, you'll get a stream of DataRows.]

It looks complicated, but you can reuse this pattern over and over again. I essentially copied the above code from my own scripts.

If your concern is code deployment, then I'd argue that this method is still better. The above code requires only the .Net Framework. Indeed, the code even works on PowerShell Core 6.0 (granted, you have to call (foo <value>).Rows to get equivalent output because Core hasn't been taught how to display DataTables at the command line). Using Invoke-Sqlcmd adds a dependency on the SqlServer PowerShell module being installed. Sure, it's a first party module, but it still doesn't ship with Windows by default. You definitely lose some benefits of encapsulation being done by others without Invoke-Sqlcmd, but there's still a trade-off.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.