How can this SQL query code be broken/exploited by user input? [duplicate]

This question already has answers here: Closed 12 years ago. 开发者_运维问答

Possible Duplicate:

Can I protect against SQL Injection by escaping single-quote and surrounding user input with single-quotes?

We have a legacy app that doesn't do queries using positional parameters, and there's SQL everywhere. It was decided (before I started here) that since user input can contain apostrophes, every string input should be manually escaped for those apostrophes.

Here is the essential original code (not written by me), translated into C# for easier consumption:

private string _Escape(string input)
{
    return input.Replace("'", "''");
}

private bool _IsValidLogin(string userName, string password)
{
    string sql =
        string.Format
        (
            @"SELECT COUNT(*) FROM UserAccounts
                WHERE UserName = '{0}' AND Password = '{1}'",
            _Escape(userName),
            _Escape(password)
        );
    // ...
}

This really seems like it can be broken in some way, but I'm at a loss as to how it could be exploited by user input. Assume user input is unfiltered until it hits _IsValidLogin, and forget that passwords appear to be stored in plain text.

The solution to shore it up for good is obvious -- use positional parameters -- but I need some ammunition to demonstrate to management why/how this code is insecure so time/$ can be allocated for it to get fixed.

Note: I'm assuming this can be broken, but that may not actually be the case. I'm not a SQL superstar.

Note 2: I've expressed this question as database-agnostic, but if you can exploit this code for a certain engine, I welcome your contribution.

---

It could be exlpoited by backslashes.

password = foo\' OR 1=1 --

becomes:

password = foo\'' OR 1=1 --

the query:

"SELECT COUNT(*) FROM UserAccounts
                WHERE UserName = '{0}' AND Password = 'foo\'' OR 1=1 --'"

-- Is the comment mark in this example.

The solution assumes the program only filters (duplicates) apostrophes.

---

Well, I can't see a way it's vulnerable. So, let's argue a different reason why it should be changed --- it's rather ineffiecent. In MSSQL (and, I think, most other high end SQL servers), queries are parsed, and execution plan is devised, and then the query and plan are stored. If an exact copy of the query is requested again, the saved execution plan is used. Parameter don't affect this, so if you use parameters, it will reuse the plans; if you embed the text, it never will.