Can my users inject my dynamic sql?

Question

I'm a desktop developer writing for internal users, so I'm not worried about malicious hackers, but I would like to know if there's anything they could enter when updating a value that would execute sql on the server.

The business defines their content schema and I have a CRUD application for them that doesn't have to be changed when their schema changes because the validation details are table-driven and the updates are with dynamic SQL. I have to support single quotes in their data entry, so when they enter them, I double them before the SQL is executed on the server. From what I've read, however, this shouldn't be enough to stop an injection.

So my question is, what text could they enter in a free-form text field that could change something on the server instead of being stored as a literal value?

Basically, I'm building an SQL statement at runtime that follows the pattern:

update table set field = value where pkField = pkVal

with this VB.NET code:

Friend Function updateVal(ByVal newVal As String) As Integer
    Dim params As Collection
    Dim SQL As String
    Dim ret As Integer

    SQL = _updateSQL(newVal)
    params = New Collection
    params.Add(SQLClientAccess.instance.sqlParam("@SQL", DbType.String, 0, SQL))
    Try
        ret = SQLClientAccess.instance.execSP("usp_execSQL", params)
    Catch ex As Exception
        Throw New Exception(ex.Message)
    End Try
    Return ret
End Function

Private Function _updateSQL(ByVal newVal As String) As String
    Dim SQL As String
    Dim useDelimiter As Boolean = (_formatType = DisplaySet.formatTypes.text)
    Dim position As Integer = InStr(newVal, "'")
    Do Until position = 0
        newVal = Left(newVal, position) + Mid(newVal, position)       ' double embedded single quotes '
        position = InStr(position + 2, newVal, "'")
    Loop
    If _formatType = DisplaySet.formatTypes.memo Then
        SQL = "declare @ptrval binary(16)"
        SQL = SQL & " select @ptrval = textptr(" & _fieldName & ")"
        SQL = SQL & " from " & _updateTableName & _PKWhereClauses
        SQL = SQL & " updatetext " & _updateTableName & "." & _fieldName & " @ptrval 0 null '" & newVal & "'"
    Else
        SQL = "Update " & _updateTableName & " set " & _fieldName & " = "
        If useDelimiter Then
            SQL = SQL & "'"
        End If
        SQL = SQL & newVal
        If useDelimiter Then
            SQL = SQL & "'"
        End If
        SQL = SQL & _PKWhereClauses
    End If
    Return SQL
End Function

when I update a text field to the value

Redmond'; drop table OrdersTable--

it generates:

Update caseFile set notes = 'Redmond''; drop table OrdersTable--' where guardianshipID = '001168-3'

and updates the value to the literal value they entered.

What else could they enter that would inject SQL?

Again, I'm not worried that someone wants to hack the server at their job, but would like to know how if they could accidentally paste text from somewhere else and break something.

Thanks.

Answer 1

Assuming you escape string literals (which from what you said you are doing), you should be safe. The only other thing I can think of is if you use a unicode-based character set to communicate with the database, make sure the strings you send are valid in that encoding.

Answer 2

As ugly as your doubling up code is (:p - Try String.Replace instead.) I'm pretty sure that will do the job.

Answer 3

The only safe assumption is that if you're not using parameterized queries (and you're not, exclusively, here, because you're concatenating the input string into your sql), then you're not safe.

Answer 4

Regardless of how you cleanse the user input increasing the attack surface is the real problem with what you're doing. If you look back at the history of SQL Injection you'll notice that new and even more creative ways to wreak havoc via them have emerged over time. While you may have avoided the known it's always what's lurking just around the corner that makes this type of code difficult to productionize. You'd be better to simply use a different approach.

Answer 5

You never never ever never want to build a SQL statement using user input that will be then directly executed. This leads to SQL injection attacks, as you've found. It would be trivial for someone to drop a table in your database, as you've described.

You want to use parameterized queries, where you build an SQL string using placeholders for the values, then pass the values in for those parameters.

Using VB you'd do something like:

'Define our sql query'
Dim sSQL As String = "SELECT FirstName, LastName, Title " & _
                     "FROM Employees " & _
                     "WHERE ((EmployeeID > ? AND HireDate > ?) AND Country = ?)"

'Populate Command Object'
Dim oCmd As New OledbCommand(sSQL, oCnn)

'Add up the parameter, associated it with its value'
oCmd.Parameters.Add("EmployeeID", sEmpId)
oCmd.Parameters.Add("HireDate", sHireDate)
oCmd.Parameters.Add("Country", sCountry)

(example taken from here) (also not I'm not a VB programmer so this might not be proper syntax, but it gets the point across)

Answer 6

You can also evaluate an alternative solution. Dynamic generation of SQL with parameters. Something like this:

// snippet just for get the idea
var parameters = new Dictionary<string, object>();
GetParametersFromUI(parameters);


if (parameters.ContainsKey("@id")) {
    whereBuilder.Append(" AND id = @id");
    cmd.Parameters.AddWithValue("@id", parameters["@id"]);
}
...