2

I'm trying to think of a way to get around the following, because it looks like someone did, and I want to fix it. But, I would really like to understand how the attack works before fixing it with something like OWASP Recommendation

    Set conn = Server.CreateObject("ADODB.Connection")
    conn.open xDb_Conn_Str
    sSql = "SELECT * FROM [User]"
    sSql = sSql & " WHERE [Username] = '" & CleanSql(sUserId) & "'"
    Set rs = conn.Execute(sSql)

CleanSql - 

Function CleanSql(str)

    Dim sWrk

    sWrk = Trim(str&"")
    sWrk = Replace(sWrk, "'", "''") ' Adjust for Single Quote

    sWrk = Replace(sWrk, "[", "[[]") ' Adjust for Open Square Bracket

    CleanSql = sWrk

End Function

Single quote is obviously escaped in this.

Right after this it will do a check if it finds the user to validate the password with the following:

If UCase(rs("Password")) = UCase(sPassWd) Then
    DoStuff()

Any help is appreciated.

BRogers
  • 3,534
  • 4
  • 23
  • 32
  • 1
    Cannot you use prepared statements with bind variables? What DB are you using? http://stackoverflow.com/questions/149848/classic-asp-sql-injection-protection?rq=1 – Thilo Aug 28 '15 at 05:35
  • "it looks like someone did (get around the password check using SQL injection". Are you sure? What is the evidence? Couldn't they just have sniffed, guessed or otherwise obtained a password or session cookie? – Thilo Aug 28 '15 at 05:38
  • DB is sql - I can use that. I am taking over the application – BRogers Aug 28 '15 at 05:38
  • Things have been added to posts and such. That is how I'm sure @Thilo – BRogers Aug 28 '15 at 05:38
  • You can add things to website posts without bypassing the password checking logic. Stolen admin password. Cross-site scripting. Legit but angry user. Missing session validation code. Definitely move to prepared statements, but your hacker probably got in somewhere else. – Thilo Aug 28 '15 at 05:41
  • Thanks, that's what I'm thinking too. Appreciate the feedback. My bet is on XSS right now. – BRogers Aug 28 '15 at 05:46

4 Answers4

9

Since it sounds like you're already aware of the benefits of prepared/parameterized statements, I won't preach. You seem to just be curious how your existing application could have been breached.

A simple \' ; drop table users -- could beat your quote-doubling. Your CleanSql() function would turn it into:

\'' ; drop table users --

Your SQL statement would become:

... WHERE [Username] = '\'' ; drop table users --'

And since '\'' is a valid value (an escaped single quote), your where clause is effectively ended. ; starts a new command and -- effectively comments out the closing quote. drop table could be anything... update users set password=... or insert into users values () or anything the attacker wants to run.

Bond
  • 16,071
  • 6
  • 30
  • 53
  • 1
    I'm going to mess around with this. Great answer to my question, probably to be marked as the solution. Thank you – BRogers Aug 28 '15 at 06:11
  • I tried `\' ; select Username from dbo.users where userid = 29 --` because I know the password and it still didn't work. Now I'm curious because your explanation looks correct. – BRogers Aug 28 '15 at 06:13
  • Excellent work 007. So they could actually have done a `SELECT * FROM [Users] WHERE [Username] = '\'' UNION SELECT 'ourname', 'ourpassword' FROM [Users]` in this case – Thilo Aug 28 '15 at 06:16
  • 1
    This is just one example of how to bypass this type of string concatenation. Your database may not support escaping single quotes in this manner (`\'`), for example. But some do. If it wasn't done this way, it was likely something similar. There are many ways of representing characters: escaping, encoding, different character sets, etc. – Bond Aug 28 '15 at 06:16
2

You should Always use Prepared statement, as escaping in in most case insufficient, because there is always special case you would forget about.

Dim cmdPrep1 As New ADODB.Command
Set cmdPrep1.ActiveConnection = conn

cmdPrep1.CommandText = "SELECT * FROM [User]  WHERE [Username] = ?"
cmdPrep1.CommandType = adCmdText
cmdPrep1.Prepared = True

Set prm1 = cmdPrep1.CreateParameter("Username", adChar, adParamInput, Len(sUserId), sUserId)
cmdPrep1.Parameters.Append prm1

Set rs = cmdPrep1.Execute()

If the prepared statement are unavailable for your language or database library, strongly consider switching.
Consult this wikipedia article to see why prepared statement are safer, but the gist of it is that parameters are passed separately from the query, thus avoiding embedding utrusted string in the query.

dvhh
  • 4,724
  • 27
  • 33
  • That's great, I know how to protect against it for the most part... I know methods that are generally recommended. I'm wondering if this was really the access point and how someone would get around something like the above. Thank you for your post. – BRogers Aug 28 '15 at 05:39
  • Prepared statement are the first recommendation in the [OWASP prevention list](https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet) – dvhh Aug 28 '15 at 05:48
1

Simply because this won't work for when the ESCAPE keyword is used.

Also, your CleanSql function will not work for unquoted values (e.g. numeric values).

That is why it is advised to use parameterised queries - these treat the values as data rather than as parts of the query.

Sometimes you cannot use parameterisation, for example if you dynamically alter an ORDER BY or if you want to dynamically change the table that the data is read from. In these cases you should use whitelisting to ensure the dynamic data is allowed by your application. The other situation where you can't use parameterisation is when there's an IN clause. This is where your CleanSql function would be useful - note you don't need the bracket escaping - this is only for LIKE queries where the escape character is not specified.

Community
  • 1
  • 1
SilverlightFox
  • 32,436
  • 11
  • 76
  • 145
0

A simple way: var = Replace(Request.Form("form"),"'","\'")

ricarela
  • 30
  • 3