SQL Injection even when escaping quote

Question

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.

Answer 1

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.

Answer 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.