2


I've got a project with a lot of SQL-queries compiled in *.DLLs files.
Yesterday, I've received a new bug: username (which generated automatically) with single quote causes an error.
The reason is queries like this one:

    string.Format("SELECT TimeZone from yaf_User WHERE [Name]='{0}'", UserName);


Can someone to suggest any kind of trick or hack to fix it?

Update: I don't know why the developers use this horrible way to generate SQL-query, but for now I should fix it. The client will not understand why I should to rewrite a lot of code for fix.

Egor Romanko
  • 70
  • 1
  • 7
  • Please edit the question to show the string after the variable substitution. – Gordon Linoff Jan 20 '14 at 15:09
  • 1
    possible duplicate of [How do parameterized queries help against SQL injection?](http://stackoverflow.com/questions/5468425/how-do-parameterized-queries-help-against-sql-injection) – Alex K. Jan 20 '14 at 15:09
  • If you really can't change that queries to use parameters (!!!) at least escape quotes – Adriano Repetti Jan 20 '14 at 15:11
  • 1
    Hackers SQL Injection paradise. Not just one vulnerable query, but DLL's full of them!! – Mitch Wheat Jan 20 '14 at 15:11
  • If this was presented to me I'd contact the client and inform them of some serious security vulnerabilities in their code. You cannot make a judgment call on how important their customers' security is to them but equally you can't fix it for free. Give them the facts and let them decide - document everything. – Liath Jan 20 '14 at 15:19
  • I'd send them a link to http://xkcd.com/327/ – Ian Nelson Jan 20 '14 at 15:56

3 Answers3

7

What you are doing is dangerous because it lends itself to SQL injection attacks (as a side effect it also causes the issue you're seeing.

The solution is to use parameterised queries - this also avoids SQL injection attacks.

SqlCommand cmd = new SqlCommand("SELECT TimeZone from yaf_User WHERE [Name]=@UserName"), conn);
cmd.Parameters.Add(new SqlParameter("Username", theUsername));

Your only alternative is to escape the single quote. However this is a fix to your code but your solution would remain insecure. I cannot stress how important it is that you resolve this issue - as it stands I could wipe out entire tables of data in your system by logging in with a malicious username.

Liath
  • 9,913
  • 9
  • 51
  • 81
  • 1
    In this case I'll should to rewrite all assemblies I have :( – Egor Romanko Jan 20 '14 at 15:11
  • 1
    It's highly recommended. If not, you leave yourself open to sql injection, like the OP said... – ganders Jan 20 '14 at 15:14
  • Thanks for your suggestion. I'll contact the client and describe this situation. I guess I can provide the one kind of 'hotfix': to remove all single quotes. This is terribly bad way, I know :( – Egor Romanko Jan 20 '14 at 15:31
  • Yep, you'll have to re-write all the assemblies. When banks discover that their safes are easy to break into, they get new safes everywhere, not just the one that got broken into. – Aaron Bertrand Jan 20 '14 at 15:40
1

This is for Chris. Escaping characters may work. If the person is clever, the are some ways around it.

For instance.

-- Use Adventure works
use adventureworks2012
go

Say, I know you are replace a single quote with two, your chosen solution on the answere line. Enter the following

Bothell'; GRANT CONTROL TO [adw_user];PRINT' at a text box.

This boils down to this @fld variable.

-- Declare the vars
declare @sql nvarchar(max);
declare @fld varchar(128) = 'Bothell''; GRANT CONTROL TO [adw_user];PRINT''';
print @fld

-- Perform some injection
set @sql = 'select * from [Person].[Address] where City = ' + 
    char(39) +  @fld + char(39);
print @sql
exec sp_executesql @sql

There you have SQL Injection.

select * from [Person].[Address] where City = 'Bothell'; 
GRANT CONTROL TO [adw_user];PRINT''

(26 row(s) affected)

http://www.w3schools.com/sql/sql_injection.asp

Quote from W3Schools - The only proven way to protect a web site from SQL injection attacks, is to use SQL parameters.

A very good read. Check out link to truncation attacks. In short, parameterization makes sure the input is treated as a literal, not code.

http://blogs.msdn.com/b/raulga/archive/2007/01/04/dynamic-sql-sql-injection.aspx

CRAFTY DBA
  • 14,351
  • 4
  • 26
  • 30
0

Single quotes are the reason why databases get hacked. It's the concept behind SQL Injection. Taking advantage of the fact that Strings use quotes. Manipulating the value to be inserted by adding additional quotes and messing up the whole SQL statement. You can use parametized queries to avoid SQL injection but takes a few more lines of codes or use this instead.

sql="SELECT TimeZone from yaf_User WHERE [Name]='" + VariableName.replace("'","''") + "'";
chris_techno25
  • 2,401
  • 5
  • 20
  • 32
  • What if the user enters the following on the front end? "105; DROP TABLE yaf_User". Good example of SQL Injection. – CRAFTY DBA Jan 20 '14 at 15:41
  • @CRAFTYDBA, Hi thanks for that thought-provoking question. I like that. I tried inserting the DROP Table injection. It works if I don't put .replace("'","''"). It really drops the table, but it doesn't work if I have .replace("'","''") just as stated in my answer. It just inserts the DROP statement literally into the database. Can you tell me why my solution doesn't work for you? I'd love to know my method's weakness. Thank you. – chris_techno25 Jan 21 '14 at 01:25