3

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

I just want to know, If I replace every ' with '' in user inputs, for instance string.Replace("'","''"), and validate numbers (make sure that they are numbers, and do not contain any other character), is SQL Injection still possible? How?

I'm using dynamic SQL queries, using SqlCommand. Something like this:

cmd.CommandText = "SELECT * FROM myTable WHERE ID = " + theID.ToString();

or

cmd.CommandText = "UPDATE myTable SET title='" + title.Replace("'","''") + "' WHERE ID = " + theID.ToString();

Input integers are automatically validated (checked whether they are a real number) in ASP.NET MVC.

Community
  • 1
  • 1
Mahdi Ghiasi
  • 14,873
  • 19
  • 71
  • 119
  • 2
    Do yourself (and your DB server) a favor: use parameterized queries... – Thomas Levesque Sep 02 '12 at 10:43
  • 7
    Do you have some kind of philosophical or religious objection to using parameterised queries, or other suggested proven methods to avoid SQL injection? – podiluska Sep 02 '12 at 10:45
  • 1
    If I want to create a new project, I should use parameterised queries. But in this case, there is an existing program, which does the things that I said in the question to prevent injection. And changing all of queries need a lot of time. **I just want to know if the security of this methods are enough or not...** And why -1?!? – Mahdi Ghiasi Sep 02 '12 at 10:50
  • 1
    @Mahdi the only safe answer we can give is "no, they aren't enough". There are readily available evil SQL scripts available on black-hat sites designed to side-step your validation such as quote-doubling. Frankly it isn't worth the risk, especially when it is so easy to parameterize. – Marc Gravell Sep 02 '12 at 12:00
  • @podiluska: I'm staring down a case in my own codebase where it only works if I pass a string literal. If I parameterize it at all the SQL Optimizer picks a plan that's so bad it will never finish. – Joshua Nov 20 '21 at 00:49

1 Answers1

11

If this is a legacy project that is coded this way then, whilst not optimal, I'm not currently aware of any way that it can be susceptible to SQL injection as long as every string is treated in that manner and the queries are just simple ones as you have shown.

I cannot state any more certainty than that however. Without using parametrised queries there is always the possibility that there is some vulnerability that you have not yet considered.

Manually escaping the quotes yourself is error prone and can sometimes fail in ways that are difficult to anticipate in advance. For example with the following table

CREATE TABLE myTable(title VARCHAR(100))
INSERT INTO myTable VALUES('Foo')

And stored procedure using dynamic SQL built up with string concatenation 

CREATE PROC UpdateMyTable
@newtitle NVARCHAR(100)
AS
/*
Double up any single quotes
*/
SET @newtitle = REPLACE(@newtitle, '''','''''')

DECLARE @UpdateStatement VARCHAR(MAX)

SET @UpdateStatement = 'UPDATE myTable SET title='''  + @newtitle + ''''

EXEC(@UpdateStatement)

You can try the following

Normal update

EXEC UpdateMyTable N'Foo'
SELECT * FROM myTable /*Returns "Foo"*/

SQL Injection attempt foiled

EXEC UpdateMyTable N''';DROP TABLE myTable--'
SELECT * FROM myTable  /*Returns "';DROP TABLE myTable--"*/

SQL Injection attempt succeeds and drops the table

EXEC UpdateMyTable N'ʼ;DROP TABLE myTable--'
SELECT * FROM myTable  /*Returns "Invalid object name 'myTable'."*/

The issue here is that the third query passes U+02BC instead of the standard apostrophe and then the string is assigned to a varchar(max) after the sanitation occurs which silently converts this to a regular apostrophe.

Until I read the answer here that issue would never have occurred to me.

Community
  • 1
  • 1
Martin Smith
  • 438,706
  • 87
  • 741
  • 845