SQL injection proof

Question

I am trying to simply prove here that this simple function is not good enough to prevent every sql injection in the world:

Function CleanForSQL(ByVal input As String) As String Return input.Replace("'", "''") End Function 

Here is a typical insert statement from one of our applications:

 Database.DBUpdate("UPDATE tblFilledForms SET Text1 = '" + CleanForSQL(txtNote.Text) + "' WHERE FilledFormID = " + DGVNotes.SelectedRows(0).Cells("FilledFormID").Value.ToString) 

I know this is not safe, due to a google search and other questions on StackOverflow.com. There is one question that I found in which all functions, such as the one that I presented above, are irrelevant and meaningless.

So, based on the message I'm connected to, simply by typing

'Chr (8); update tblMaint SET Value1 = 2 WHERE ValueID = 2 -

in txtNote should be enough to clear every value in text1 throughout the tblFilledForms table and then update the second row of the tblmaint table to be correct?

What SHOULD happen here is that VB will interpret this as

UPDATE tblFilledForms SET Text1 = '' 'Chr (8); update tblMaint SET Value1 = 2 WHERE ValueID = 2-- 'WHERE FilledFormID = 5120327

and send it to SQL, which will execute chr (8), to erase the third, which will create

UPDATE tblFilledForms SET Text1 = ''; update tblMaint SET Value1 = 2 WHERE ValueID = 2-- 'WHERE FilledFormID = 5120327

for correct execution in the database?

Then I copied Chr (8) from the clipboard and replaced Chr (8) in the text box with the contents of the clipboard and still failed. It puts the entire line directly in the field without any problems.

So what am I doing wrong here? or what else can i do to break it?

Technologies and background: I use MS SQL Server 2005 and VB.NET 2005. The Text1 field in the database is the Varchar field (600) (don't ask me why its not MAX, its meaningless, I know) The table has certain triggers that would prevent a massive update such as this, and throw some errors if the injection really worked correctly.

PS. I know that parameterized queries are a way to go here, and I'm not looking for answers, for example, "well, I don’t know why this does not work, but parameterized queries are the way." I am looking for an opportunity to prove that our software is broken and that we need to rewrite it using the best principles.

For anyone reading this question, to find out how best to filter your text fields, the answer DOES NOT! Use the options! they are much better, safer and easier!

Answer 1

Chr (8) is part of the quoted literal string, as is the update statement, so SQL Server is not going to interpret it as a function call. In this example, Text1 will be set to a literal value:

 'Chr(8); update tblMaint SET Value1 = 2 WHERE ValueID = 2-- 

(yes, including this single quote)

So, in this example, your code is protected. Most of the hang on SQL injection is due to the accidental failure of checking and quoting values, there is nothing inherently unsafe in a correctly formulated SQL statement.

Answer 2

Your CleanForSQL method only processes rows. What happens when you do not use a string, but instead of INT? In this case there will be no end of the tick to close, so the injection will happen anyway. Consider this example ...

 Database.DBUpdate("UPDATE tblFilledForms SET Int1 = " + CleanForSQL(txtNote.Text) + " WHERE FilledFormID = " + DGVNotes.SelectedRows(0).Cells("FilledFormID").Value.ToString) 

in this case, simply enter the following:

0; update tblMaint SET Value1 = 2 WHERE ValueID = 2--

Answer 3

Scott Ivey has a classic case that can break him, the lack of quotes that protect numerical input. (+ 1'ed that)

Depending on the language and where the string is "cleared" and the database used, your immediate risk is that the language allows you to escape the string. At this point, one quote that you are trying to avoid is not right.

\ '; DROP yourTable; - => \ ''; DROP yourTable; -

This goes into your sql string as

 UPDATE tblFilledForms SET Text1 = '" + \''; DROP yourTable;-- + ' etc. 

What then:

 UPDATE tblFilledForms SET Text1 = '\''; DROP yourTable;-- ' etc. 

'\' 'is accepted as a literal string of one quote, if your database supports escaped characters - bingo is your compromised one.

You should also remember that protection should be effective, even if in the above example of the update instruction it was not possible to protect the parameter in the where clause, was it because DGVNotes.SelectedRows (0) .Cells ("FilledFormID"). Value.ToString) can never be entered by the user? Will this be done throughout the life of the application, etc.?

Answer 4

You are not doing anything wrong. This is how SQL Server parses strings. The first quote opens a line, after which you followed it with a hidden quote, followed by Chr (8).

As an exercise, what happens if you run this in SQL Server: SELECT '''Hello' ? In this case, the same parsing rules apply.

Answer 5

I think your problem is that Chr(8) not running, you need to find another way to get the main quote sign.