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

Question

This question already has answers here:

Closed 15 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.

• Nick Craver: Worth a read here if you haven't already as well: stackoverflow.com/questions/139199/…
• Jon Seigel: @Nick: Thanks, I didn't see that one. I'm gonna close this question as a duplicate.
• Bill the Lizard: Closed upon request of the author.

Answer 1

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.

• James Curran: I can't say for other databases, but backslash does not appear to be an escape character in MSSQL, and would simply be used literally. Your example would merely search for passwords of 'foo\' OR 1=1 --

Answer 2

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.

• erenon: It's vulnerable. The example doesn't use any parameter binding.