Cleaning requests to prevent SQL injection attack

Question

We have inherited an old website with almost 2000 different hand built SQL strings taking the variables directly from httprequests. The site is compromised by SQL injection attack regularly. Obviously this site should have been coded using SQL parameters to avoid this security nightmare, but due to the workload involved changing these we are looking for another way of 'cleaning' the incoming requests.

Main Clean Function :-

Public Function myRequest(ByRef Request As HttpRequest, ByVal param As String) As String
        Return CleanRequest(Request(param))
End Function

Public Function CleanRequest(ByVal requestString As String) As String
        Dim badChars() As String = {"select", "drop", ";", "--", "insert", "delete", "xp_", "update"}
        Dim newChars As String = requestString

        For i = 0 To UBound(badChars)
            newChars = Replace(newChars, badChars(i), "", 1, -1, vbTextCompare)
        Next

        CleanRequest = Replace(newChars, "'", "''")
End Function

Called as so :-

Dim details As DataSet

detailsSQL = "select * from mytable where tableid = '" & myRequest(Request, "tableid") & "'"
details = sql.sqlSelect(detailsSQL)

Note that the code is structured and named as it is for easy find & replace. With this code in place though the site continues to be regularly compromised. Can anyone recommend additions to the main 'clean' function that will help stop these injection attacks?

This will be unhelpful for any user living in [Staindrop](http://en.wikipedia.org/wiki/Staindrop). — Brian Hooper, Jan 21 '13 at 10:53
Interresting though on http://stackoverflow.com/questions/1800013/does-this-code-prevent-sql-injection — Larry, Jan 21 '13 at 12:26
Hi Simon, I have not parametrised the 'fix' because it is necessary to have a solution that I can 'find & replace' into the many situations where it occurs, as stated in the question. — AshRolls, Jan 22 '13 at 11:04

score 5 · Accepted Answer · edited May 23 '17 at 11:53

I think the only reliable way you will prevent SQL Injection would be to put the hard work in and convert all the requests to use SQL Parameters.

However, to answer your question your clean method seems to be missing the fundamental trigger for SQL Injection - unescaped characters. At the moment your site is still susceptible to various types of attacks, for example:

"password' or 1=1; --"
"password'; DROP Table Users; --"

At the very least, make sure you are escaping any bad characters. Be warned though, this isn't always enough, so I re-iterate if you want to completely eradicate SQL Injection - use SQL Parameters.

score 1 · Answer 2 · answered Jan 21 '13 at 10:56

1

The piece of SQL below is wrong. Please use SQL parameters : it will help you to get rid with the most obviouses kind of SQL injection attacks.

detailsSQL = "select * from mytable where tableid = '" & myRequest(Request, "tableid") & "'"
details = sql.sqlSelect(detailsSQL)

Replace this with :

detailsSQL = "select * from mytable where tableid = @tableid"
details = sql.sqlSelect(detailsSQL, new SqlParameter(@tableid, myRequest(Request, "tableid")))

Assuming

sql is a wrapper for SQLServer database queries
Your sqlSelect function is adapted to have SQL parameters as well
myRequest escapes entries properly as James suggested

Using parameters the right way will make no longer necessary to parse each entries for SQL statements.

answered Jan 21 '13 at 10:56

Larry

17,605
9
77
106

Thanks but you haven't fully read and understood the question. – AshRolls Mar 17 '15 at 11:26
I was reviewing my account and going through some old posts when I noticed this and thought it should be responded too for clarity in case anyone finds this thread through Google in future. – AshRolls Mar 18 '15 at 10:31
That sounds fair. Hope the accepted answer and links in comments helped. – Larry Mar 18 '15 at 10:47

Cleaning requests to prevent SQL injection attack

2 Answers2