ASP sanitising user input (SQL injection)

[Durzel]

EDIT: Have modified it - it was erroneously stripping off single - characters when it should only be wiping --, plus some other regexp tweaks

I've been working on some sites that haven't been properly validating user input and as a result all kinds of nastiness has been going on behind the scenes (e.g. UNION SELECT'ing to pull out admin details). I knocked together a function to sanitise user input and wanted to get some feedback (mainly as my regexp is hazy).

I designed it so that words like "SELECT", "DELETE", etc would be ok to pass so long as they weren't passed in combination with others to make up an SQL command (e.g. SELECT x FROM, DELETE FROM, etc)

Function SQLSafe(sTxt)
   'User input sanitizer to remove risk of SQL injection
   'by Darren Coleman ([email protected])
   'version 1.03 - 10/09/2008
   '  1.00 - Initial release
   '  1.01 - Modified regexp to wipe rest of string if SQL command sequence detected
   '  1.02 - Modified SQL sequence checks to handle multiple spaces between commands
   '  1.03 - Fixed broken "--" detection (was deleting all - characters), tweaked regexp slightly
   
   Dim uberSQLInjExp
   Set uberSQLInjExp = New RegExp
   
   'The following regexp will match HTML tags, sequence combinations of 
   'SQL commands as well as the more obvious control characters & SQL terminators
   With uberSQLInjExp
      .Pattern = 	"<[^>]+>|" & _
                  "SELECT((.|\s)*?)FROM((.|\s)*?)$|" & _
                  "UPDATE((.|\s)*?)SET((.|\s)*?)$|" & _
                  "INSERT[\s]+INTO((.|\s)*?)$|" & _
                  "DELETE[\s]+FROM((.|\s)*?)$|" & _
                  "(DROP|CREATE|ALTER|TRUNCATE)[\s]+TABLE[\s]+((.|\s)*?)$|" & _
                  "UNION[\s]+(ALL|SELECT){1}[\s]+((.|\s)*?)$|" & _
                  "[;|\r?\n|\r|\x00|\x1a]|[-]{2}"			
      .IgnoreCase = True
      .Global = True
   End With
   
   SQLSafe = uberSQLInjExp.Replace(sTxt,"")
   'Double-quote any single quotes in the text
   SQLSafe = Replace(SQLSafe,"'","''")
   
   Set uberSQLInjExp = Nothing
End Function

If anyone has any optimisations or suggestions for the above I'd be very grateful if they'd share their knowledge

 

[Durzel]

Cool I should point out that the one I've written was done from scratch so I don't know how optimal it is - I haven't really gone deep into regexp for a while so there might be issues with it. It's provided "as is"

It's not as advanced as the one you've pasted, it doesn't actually validate the input (string, numeric, date, etc) as I do that elsewhere. Just needed a one-size-fits-all encapsulated function that would deal with some of the attacks we've been seeing. My function strips out HTML tags as well which may or may not be desireable.

 

[Durzel]

I've made some more changes/improvements to this but wanted to find out if anyone actually was bothered about me posting them or not - thought I'd be doing fellow ASP programmers a favour but no one seems to care about SQL injection problems?

(plus I still need help optimising my regexp)

 

[Durzel]

Sorry for delay replying, below is the latest version.

I've added two more regexps for "common SQL attacks" (the DECLARE .. CHAR and CONVERT .. SP_PASSWORD checks). I am not sure whether it is a good idea to add regexps for specialist attacks though as whilst it means it filters them out properly I am a little worried about the performance hit of more and more regexp'ing. Interested to hear thoughts...

(I've commented out a section in the function which links in with another function I've written but haven't included below - basically a system which emails the support team if a potential hack is detected)

Function SQLSafe(sTxt)
	'User input sanitizer to remove risk of SQL injection
	'by Darren Coleman ([email protected])
	'  1.00 - Initial release (10/09/2008)
	'  1.01 - Modified regexp to wipe rest of string if SQL command sequence detected
	'  1.02 - Modified SQL sequence checks to handle multiple spaces between commands
	'  1.03 - Fixed broken "--" detection (was deleting all - characters), tweaked regexp slightly
	'  1.04 - Added abuse-mail feature, added DECLARE x VARCHAR regexp, reclassified ; non-malicious
	'  1.05 - Added AND..CONVERT..SP_PASSWORD regexp
	
	Dim uberSQLInjExp
	Set uberSQLInjExp = New RegExp
	
	'The following regexp will match HTML tags, sequence combinations of 
	'SQL commands as well as the more obvious control characters & SQL terminators
	With uberSQLInjExp
		.Pattern = "<[^>]+>|" & _
			"SELECT((.|\s)*?)FROM((.|\s)*?)$|" & _
			"UPDATE((.|\s)*?)SET((.|\s)*?)$|" & _
			"INSERT[\s]+INTO((.|\s)*?)$|" & _
			"DELETE[\s]+FROM((.|\s)*?)$|" & _
			"(DROP|CREATE|ALTER|TRUNCATE)[\s]+TABLE[\s]+((.|\s)*?)$|" & _
			"UNION[\s]+(ALL|SELECT){1}[\s]+((.|\s)*?)$|" & _
			"DECLARE((.|\s)*?)[\s]+(NVARCHAR|VARCHAR|CHAR){1}((.|\s)*?)$|" & _
			"AND[\s]+((.|\s)*?)CONVERT((.|\s)*?)SP_PASSWORD$|" & _
			"[\r?\n|\r|\x00|\x1a]|[-]{2}"			
		.IgnoreCase = True
		.Global = True
	End With
	
	SQLSafe = uberSQLInjExp.Replace(sTxt,"")
	
	'Sanitize input which probably isn't malicious but we don't want anyway
	SQLSafe = Replace(SQLSafe,"'","''")
	SQLSafe = Replace(SQLSafe,";","")
	
	'If SEND_ABUSE_MAIL And uberSQLInjExp.Test(sTxt) Then
	'	sendAbuseMail sTxt, SQLSafe
	'End If
	
	Set uberSQLInjExp = Nothing
End Function

 

[Durzel]

I've also toyed with the idea of changing ((.|\s)*?) (match whitespace or any character zero or more times) to (\s+.+\s+) (match one or more spaces, any character one or more times and one or more spaces)...

The idea being that the first one would match:

SELECTS anything FROM sometable
SELECT anything SFROM sometable

..which I guess could be valid input, whereas the second would only match:

SELECT anything FROM sometable

Any thoughts?

 

[Durzel]

Yeah, if we were talking PHP then it wouldn't be a concern as mysql_real_escape_string() does the job well.