Take the 2-minute tour ×
Information Security Stack Exchange is a question and answer site for Information security professionals. It's 100% free, no registration required.

There's a Classic ASP application at my job that is (I believe) highly vulnerable to SQL injection. I want to prove to management that this code isn't secure, but all I'm able to do is insert "SQLINJ" log records in the database:

Public function preventSQLInjection(lstr)

    BlackList = Array("--", ";", "'", "/*", "*/", "@@", "@",_
                      "alter ", "begin ", "create ", "cursor ",_
                      "declare ", "delete ", "drop ", " end", "exec ",_
                      "execute ", "fetch ", "insert ", "kill ", "open ",_
                      "select ", "sysobjects", "syscolumns",_
                      "table ", "update ", "=")

    preventSQLInjection = lstr

        For Each s in BlackList      
        If ( InStr (lstr, s) <> 0 ) Then
            execSqlQuery "INSERT INTO AccesLogs (datetime,ip,action,comments) VALUES ('" & formatDate(date) & " " & formatTime(time) & "','" & request.ServerVariables("REMOTE_ADDR") & "','SQLINJ','" & replace(lstr,"'","''") & "')",connectionstring
            preventSQLInjection = "DONOTUSEIT"
            exit for
        End If  
    Next

end function

From my understanding the easiest attack would be on the AccessLogs table itself, because of replace(lstr,"'","''") which "sanitizes" the SQL-Injection attempt and logs it.


I KNOW that this code should be ditched and all executable SQL should be executed with actual commands and parameters - I hate concatenated strings with a passion.

I want to demonstrate management that the page is very vulnerable, like, sysobjects is blacklisted, but not sys.objects so if I can inject executable SQL it must be possible to get then entire database's schema without sweating.

How can this function be defeated from, say, the username and password fields on the login.asp page? Or am I making this all up and this code is perfectly secure?

share|improve this question
    
It seems to me that any aproach to sanitizing SQL commands that is based on searching for particular strings, without fully parsing the commands, cannot possibly work. At the very least, every time one of the blacklisted terms occurs innocently in a text field you will get a false positive. –  ddyer Feb 10 at 20:21
    
Indeed, the log is full of "attacks" of users entering their email address instead of their username! –  retailcoder Feb 10 at 20:24
2  
@retailcoder Something tells me that when you find a command that works, the only result will be that they will add that command to the blacklist. Some people are just immune to reason. –  Philipp Feb 10 at 20:47
    
Are you using any LIKE queries? He's not escaping % –  Yolanda Ruiz Feb 10 at 21:04
1  
Hmmm... some of these commands are followed by spaces. Are there other whitespaces which SQL accepts as token delimeter? –  Philipp Feb 10 at 22:17

2 Answers 2

up vote 4 down vote accepted

I want to prove to management that this code isn't secure

You could spend the rest of your (perhaps short) career doing so, over and over as they add to the blacklist. I suggest trying to educate them instead.

Read the OWASP SQL Injection Cheat Sheet and related sheets referenced therein, and then be able to present that to the decision makers and answer questions rationally - OWASP is a very reputable source.

Also read Why We Shouldn't Roll Our Own - in particular, the Phil Zimmerman example is an example, from a very well known cryptographer, of why "well, I can't defeat it" does not mean "it" is secure.

If you must try to provide an example, try playing around with various encodings; hexadecimal is clearly allowed, so pull up your ASCII Table for use with 0x and CHAR(), and perhaps also play around with HTML escape characters or Unicode (UCS-2, for SQL Server).

See if you can use 'set quoted_identifier off' as well.

As kiBytes said, try mixed case - the most common SQL Server collations are all case insensitive.

If anything works, you'll need to go back to educating them that just because you found N holes doesn't mean there aren't X holes remaining, some of which you will not find before attackers do.

share|improve this answer

I can't test it, but the documentation tells that the "InStr" function is case sensitive, so I believe you can use any token but the symbols in capital letters:

share|improve this answer
    
indeed, SELECT NAME, TYPE_DESC FROM sys.objects for a name doesn't trigger a SQLINJ log entry... but doesn't make it executable SQL. That's a very good start though! –  retailcoder Feb 10 at 20:35
2  
Since you have access to the code, try it in the other way, consider a valid SQL string query and then try to build it with the injection =) –  kiBytes Feb 10 at 20:40

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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