10

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?

Anders
  • 65,052
  • 24
  • 180
  • 218
Mathieu Guindon
  • 332
  • 2
  • 12
  • 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 '14 at 20:21
  • Indeed, the log is full of "attacks" of users entering their email address instead of their username! – Mathieu Guindon Feb 10 '14 at 20:24
  • 3
    @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 '14 at 20:47
  • Are you using any LIKE queries? He's not escaping `%` – Yolanda Ruiz Feb 10 '14 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 '14 at 22:17
  • See the answer by Rob Kraft at https://stackoverflow.com/questions/15537368/how-can-sanitation-that-escapes-single-quotes-be-defeated-by-sql-injection-in-sq and see if that help you – David Waters Nov 19 '17 at 18:49

3 Answers3

5

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.

Anti-weakpasswords
  • 9,850
  • 2
  • 24
  • 52
3

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:

kiBytes
  • 3,470
  • 16
  • 26
  • 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! – Mathieu Guindon Feb 10 '14 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 '14 at 20:40
1

use this code

<script>eval('al' + 'ert(1)')</script>

<script>eval(String.fromCharCode(97, 108, 101, 114, 116, 40, 49, 41))</script>
<meta http-equiv="refresh" content="0;url=javascript:alert(1);">
schroeder
  • 125,553
  • 55
  • 289
  • 326
  • 5
    Just posting code is not useful. Like any other coding exercise, can you document or comment your code so that others can understand it or understand how it answers the question? – schroeder Oct 15 '19 at 06:56