Mr Shoubs
Mr Shoubs

Reputation: 15389

Prevent SQL Injection in Dynamic column names

I can't get away without writing some dynamic sql conditions in a part of my system (using Postgres).

My question is how best to avoid SQL Injection with the method I am currently using.

EDIT (Reasoning): There are many of columns in a number of tables (a number which grows (only) and is maintained elsewhere). I need a method of allowing the user to decide which (predefined) column they want to query (and if necessary apply string functions to). The query itself is far too complex for the user to write themselves, nor do they have access to the db. There are 1000's of users with varying requirements and I need to remain as flexible as possible - I shouldn't have to revisit the code unless the main query needs to change - Also, there is no way of knowing what conditions the user will need to use.

I have objects (received via web service) that generates a condition (the generation method is below - it isn't perfect yet) for some large sql queries.

The _FieldName is user editable (parameter name was, but it didn't need to be) and I am worried it could be an attack vector. I put double quotes (see quoted identifier) around the field name in an attempt to sanitize the string, this way it can never be a key word. I could also look up the field name against a list of fields, but it would be difficult to maintain on a timely basis.

Unfortunately the user must enter the condition criteria, I am sure there must be more I can add to the sanatize method? and does quoting the column name make it safe? (my limited testing seems to think so).

an example built condition would be "AND upper(brandloaded.make) like 'O%' and upper(brandloaded.make) not like 'OTHERBRAND'" ...

Any help or suggestions are appreciated.

Public Function GetCondition() As String
   Dim sb As New Text.StringBuilder

   'put quote around the table name in an attempt to prevent some sql injection
   'http://www.postgresql.org/docs/8.2/static/sql-syntax-lexical.html
   sb.AppendFormat(" {0} ""{1}"" ", _LogicOperator.ToString, _FieldName)

   Select Case _ConditionOperator
      Case ConditionOperatorOptions.Equals
          sb.Append(" = ")

      ...

   End Select

   sb.AppendFormat(" {0} ", Me.UniqueParameterName) 'for parameter

   Return Me.Sanitize(sb)

End Function

Private Function Sanitize(ByVal sb As Text.StringBuilder) As String

   'compare against a similar blacklist mentioned here: http://forums.asp.net/t/1254125.aspx

    sb.Replace(";", "")
    sb.Replace("'", "")
    sb.Replace("\", "")
    sb.Replace(Chr(8), "")

    Return sb.ToString

End Function

Public ReadOnly Property UniqueParameterName() As String
     Get
         Return String.Concat(":" _UniqueIdentifier)
     End Get
End Property

Upvotes: 7

Views: 4509

Answers (3)

user1664043
user1664043

Reputation: 705

One thing you could do is go ahead and create your dynamic query but put something like this as a prefix:

"IF EXISTS(SELECT * FROM sys.columns where object_id=OBJECT_ID('mytable') and name = @dynamicName)
    SELECT * FROM mytable WHERE [" + dynamicName + "] = 'Whatever your test is.'"

Yeah, it makes the query a little more expensive, but it is protected against injection.

Upvotes: 1

jmoreno
jmoreno

Reputation: 13561

SQL Injection happens because user entered data is used in a dynamic query, without paramerterizing it. Unfortunately, in your case the user entered data can not be paramertized, the solution is to not let the user enter that data.

A possible workaround would be to use a whitelist (not BLACKlist) of acceptable characters. But really, you should see about obtaining a list of fieldNames and verifying the users input (and then use YOUR version of the string, not the one that came from the user).

User entered data is always suspect, and should be avoided if at all possible.

Upvotes: 3

Andrew Morton
Andrew Morton

Reputation: 25013

You can get the column names from the database and compare to check the user has entered a valid column name.

Upvotes: 9

Related Questions