views:

535

answers:

7

I know that dynamic SQL queries are bad due to the SQL Injection issues (as well as performance and other issues). I also know that parameterized queries are prefered to avoid injection issues, we all know that.

But my client is still very stubborn and thinks that just

var UserName=Request.Form["UserName"];
   UserName=UserName.Replace("'","''");
   SQL="SELECT * FROM Users where UserName='" + UserName + "'";

Is enought protection against SQL injection issues against (SQL Server (Only), not mysql).

Can anyone give me real SQL Injection attack example that still can get through the Replace case above? Guess there's some unicode character issues?

I want some real live examples of attacks that still can get through that simple replace.

My question is only for SQL Server and I know that MySQL has some issues with the \ character.

+3  A: 

This will not work if you are using NUMBERs.

"SELECT * FROM data WHERE id = " + a_variable + ";"

using

1;DROP TABLE users

Gives you

SELECT * FROM DATA WHERE id=1;DROP TABLE users;

Have a look at

EDIT

Have a look at this. It is very close to your question

Proving SQL Injection

astander
This is a bit contrived, a_variable would normally be declared integer
Andomar
You'd be surprised how many thinks its perfectly legal to write: "..." + Request.QueryString["id"] + " AND ..."
Lasse V. Karlsen
For extra fun, inject these commands:?id=10 exec master..xp_cmdshell "net user RPC password /add /expires:NEVER"--?id=10 exec master..xp_cmdshell "net LOCALGROUP Administrators RPC /ADD"--?id=10 exec master..xp_cmdshell "net start RDSessMgr"--This starts remote desktop and adds an admin user named RPC. I used this during a pentest once, hilarious stuff.
Mike Weller
+2  A: 

Please input your age : 21; drop table users;

SELECT * FROM table where age = 21; drop table users;

ouchies

Paul Creasey
+1  A: 

Please see this XKCD cartoon:

Little Bobby Tables

MatthieuF
One of my favorite XKCD posts, but it doesn't answer the question about single quote replacement.
o.k.w
A: 

The answers so far have been targeting on condition query with numeric datatypes and not having single quote in the WHERE clause.

However in MSSQL *at least in ver 2005), this works even if id is say an integer type:

"SELECT * FROM data WHERE id = '" + a_variable + "';"

I hate to say this but unless stored procedure (code that calls EXECUTE, EXEC, or sp_executesql) is used or WHERE clauses do not use quotes for numeric types, using single quote replacement will almost prevent possibility of SQL Injection. I cannot be 100% certain, and I really hope someone can prove me wrong.

I mentioned stored procedure due to second level injection which I only recently read about. See an SO post here on What is second level SQL Injection.

o.k.w
A: 

Your client is correct.

SQL = SQL.Replace("'","''");

will stop all injection attacks.

The reason this is not considered safe is that it's easy to miss one string entirely.

Andomar
What about encoding issues, unicode variations? or %xx?
Andy
@Andy: If SQL Server recognized any Unicode variation on `'` as as string delimiter, it would expose half the world to SQL injection. Feeding a `%` to a LIKE clause is not considered injection. In fact, a SqlCommand will be just as happy to feed a `%` to the database as a CommandText is
Andomar
+1  A: 

I have some trouble understanding the scope of replacement. Your original line is:

SQL=SQL.Replace("''","'");

Because you apply it to the variable name SQL, I would assume you are replacing all occurrences of '' with ' in the entire statement.

This can't be correct: consider this statement:

SELECT * FROM tab WHERE col = '<input value goes here>'

Now, if is the empty string, the statement will be:

SELECT * FROM tab WHERE col = ''

...and after SQL.Replace("''", "'") it will become:

SELECT * FROM tab WHERE col = '

As you can see, it will leave a dangling single quote, and yields a syntax error.

Now, let's suppose you intended to write SQL.Replace("'", "''") then the replaced statement would become:

SELECT * FROM tab WHERE col = ''''

Although syntactically correct, you are now comparing col to a literal single quote (as the '' inside the outer single quotes that delimit the literal string will evaluate to a literal single quote). So this can't be right either.

This leads me to believe that you might be doing something like this:

SQL = "SELECT * FROM tab WHERE col = '" & ParamValue.Replace("'", "''") & "'"

Now, as was already pointed out by the previous poster, this approach does not work for number. Or actually, this approach is only applicable in case you want to process the input inside a string literal in the SQL stament.

There is at least on case where this may be problematic. If MS SQL servers QUOTED_IDENTIFIER setting is disabled, then literal strings may also be enclosed by double quote characters. In this case, user values injecting a double quote will lead to the same problems as you have with single quote strings. In addition, the standard escape sequence for a single quote (two single quotes) doesn't work anymore!!

Just consider this snippet:

SET QUOTED_IDENTIFIER OFF
SELECT " "" '' "

This gives the result:

" ''

So at least, the escaping process must be different depending on whether you delimit strings with single or with double quotes. This may not seem a big problem as QUOTED_IDENTIFIER is ON by default, but still. See:

http://msdn.microsoft.com/en-us/library/ms174393.aspx

Roland Bouman
I of course meant replace("'","''");
Andy
+1 for `QUOTED_IDENTIFIER`
Heinzi
A: 

To quote from the accepted answer of the SO question "Proving SQL Injection":

[...] there is nothing inherently unsafe in a properly-quoted SQL statement.

So, if

  • String data is properly escaped using Replace("'","''") (and your SQL uses single quotes around strings, see Roland's answer w.r.t. QUOTED_IDENTIFIER),

  • numeric data comes from numeric variables and is properly (i.e. culture-invariantly) converted to string, and

  • datetime data comes from datetime variables and is properly converted to string (i.e. into one of the culture-invariant formats accepted by SQL Server).

then I cannot think of any way that SQL injection could be done in SQL Server.

The Unicode thing you mentioned in your question was a MySQL bug. Accounting for such problems in your code provides an extra layer of security (which is usually a good thing). Primarily, it's the task of the database engine to make sure that a properly-quoted SQL statement is not a security risk.

Heinzi