Prevent sql injection 3

[saminetemad]

Hello,
I have a sql string.

protected void Button1_Click(object sender, EventArgs e)
{

string strconnection;
strconnection = ConfigurationSettings.AppSettings["connectionstring"];
SqlConnection DBConnection = new SqlConnection(strconnection);
DBConnection.Open();
string sql = "";
sql = "UPDATE MoneyManagement SET ";
sql += "Name=" + "'" +TextBox1.Text + "'" + ",";
sql += "Explain=" + "'" + TextBox2.Text + "'" + " ";
sql += "WHERE ID=2 ";

SqlCommand cmd = new SqlCommand(sql, DBConnection);

cmd.ExecuteNonQuery();
DBConnection.Close();
}

Have you a secure way for send sql string to the database?I'm not going to define the parameters. I want to send data with sql string.

 

[mrdenny]

Defining parameters is the only reliable way to prevent SQL Injection attacks.

Denny
MVP
MCSA (2003) / MCDBA (SQL 2000)
MCTS (SQL 2005 / SQL 2005 BI / SQL 2008 DBA / SQL 2008 DBD / SQL 2008 BI / MWSS 3.0: Configuration / MOSS 2007: Configuration)
MCITP (SQL 2005 DBA / SQL 2008 DBA / SQL 2005 DBD / SQL 2008 DBD / SQL 2005 BI / SQL 2008 BI)

My Blog

 

[markros]

As Denny said, there is no other reliable way.

Besides, the change is just few lines of code:

 protected void Button1_Click(object sender, EventArgs e)
    {

        string strconnection;
        strconnection = ConfigurationSettings.AppSettings["connectionstring"];
        SqlConnection DBConnection = new SqlConnection(strconnection);
        DBConnection.Open();
        string sql = "";
        sql = @"UPDATE MoneyManagement 
                SET Name = @Name,
                    Explain = @Explain
                WHERE ID = 2"; // Are you sure 2 is not a parameter?

        SqlCommand cmd = new SqlCommand(sql, DBConnection);
        cmd.Parameters.AddWithValue("@Name",TextBox1.Text);//Better give a good name to Textbox1 like txtName
cmd.Parameters.AddWithValue("@Explain", TextBox2.Text);       
            cmd.ExecuteNonQuery();
            DBConnection.Close();
    }

This is the first approximation. The good ADO.NET practice also suggests to use Using operator or Try/Catch logic to catch possible exceptions.

PluralSight Learning Library

 

[ousoonerjoe]

markros and mrdenny are right. Parameters are the only real way to prevent it. While markos showed a great example, I would take it one step further and make the UPDATE a stored procedure and pass the parameters. It adds an extra layer of security and makes for an easy way of modifying the UPDATE statement without requiring a recompile.

--------------------------------------------------
“Crash programs fail because they are based on the theory that, with nine women pregnant, you can get a baby a month.” --Wernher von Braun
--------------------------------------------------

 

[jsteph]

I'm not real well versed in the various methods of sql injection, but from what little I've seen, it appears the double-dash is a requirement in, at least, the simpler attacks.

So for argument's sake, suppose you had a rule in your particular application that you wouldn't allow text fields to contain --. If you sanitized all text going into raw sql statements with, say, a Replace(TextBox1.Text,"--","- -"), what percentage of sql-injection attack mighty such a device prevent?
Thanks,
--Jim

 

[markros]

I'd say it will prevent 20% or less of attacks.

PluralSight Learning Library

 

[mrdenny]

I would agree with markros, at 20% or less.

Denny
MVP
MCSA (2003) / MCDBA (SQL 2000)
MCTS (SQL 2005 / SQL 2005 BI / SQL 2008 DBA / SQL 2008 DBD / SQL 2008 BI / MWSS 3.0: Configuration / MOSS 2007: Configuration)
MCITP (SQL 2005 DBA / SQL 2008 DBA / SQL 2005 DBD / SQL 2008 DBD / SQL 2005 BI / SQL 2008 BI)

My Blog

 

[ousoonerjoe]

Don't forget about the ";". The semicolon is the statement terminator. It can be used to terminate a statement and then inject the new commands.

For example...

TextBox1.Text = "''; [i]SQL injected statement[/i] --"

[COLOR=green]The code you pass to SQL via your code looks something like:[/color]
Dim SqlTxt As String
SqlTxt = "UPDATE Customer SET Comment = " & TextBox1.Text & " WHERE CustomerId = 1000;"

[COLOR=green]What the database will see is 2 seperate commands:[/color]
UPDATE Customer SET Comment = '';
[i]SQL injected statement[/i];

This is where a LOT of damage is done. The first statement just wiped out every value for Comment on every row. The second statement executed what the attacker really wanted to do. Both are valid statements to the SQL interpreter and thus no error is returned. Just looking for the "--" does not catch the error in time. The full table update fires, then the Injected statement fires, THEN the error hits. Unless you have transaction based execution with ROLLBACK on error (if the error does actually happen), the clean up could be potentially nasty.

The example is rather remedial, but I hope it illustrates a little more of a typical "first pass" SQL Injection attempt. Simply removing "--" or ";" is not always possible. Semicolons are used a lot in comments and as email address separators. It may take more typing and more time up front, but clean up afterwards will be MUCH worse. Worse case scenario, they create a backdoor and are able to create their own login account, data dump all sensitive data at will and you'd never know they were there.

--------------------------------------------------
“Crash programs fail because they are based on the theory that, with nine women pregnant, you can get a baby a month.” --Wernher von Braun
--------------------------------------------------

 

[jsteph]

Sooner,
In my example that statement wouldn't that have failed because the -- was removed? I will have to study up on injection methods to see what sort get by without the inline comment.

My environment is simple (as well as internal--mine are all intra-net apps) and we have generally relied on the (assumed) ignorance of our users, we also assume a level of trust. However we are expanding as a business and the apps will be more and more used by newer locations we've acquired and the user base--which is still non-technical--is not as familiar to us and I'm looking for the simplest sanitatizing methods given that we are allowing raw sql.

I understand that it's not 100% possible to prevent this with raw sql but I'm so shorthanded now that I absolutely will not have the time to rebuild these apps using parameterized database operations. In fact I'm at the point where if someone here knows enough to get an injection attack done, I'd probably poach them from the accounting department and have them work for me...
--Jim

 

[ousoonerjoe]

LOL.... Been there.

Just remember, that the number one threat to security is from inside the company. Bigger the company, the bigger that threat becomes.

My example isn't the best, I know... it was something just slapped together real quick for purpose of this discussion. Remember that by default, statements are committed as soon as they are executed unless within a TRANSACTION block with ROLLBACK on error. Your example probably would have failed, but only after the injection had committed to the database.

Mind you, I'm no expert at this and there are still a few things I'm not completely sure how they work, and am always open to corrections and explanations, but in some of the research I've come across (a lot from the guys here at Tek-Tips), the semicolon was just as vital, because it basically causes an Order of Operation and takes advantage of the implicit commit state when not in a transaction block.

--------------------------------------------------
“Crash programs fail because they are based on the theory that, with nine women pregnant, you can get a baby a month.” --Wernher von Braun
--------------------------------------------------

 

[ousoonerjoe]

I knew i had started a topic about this a while back...

thread183-1509615

--------------------------------------------------
“Crash programs fail because they are based on the theory that, with nine women pregnant, you can get a baby a month.” --Wernher von Braun
--------------------------------------------------

 

[gmmastros]

The biggest offender with SQL injection is the single-quote (in my opinion). Let's look at the previous example to see what would happen.

TextBox1.Text = "''; SQL injected statement --"

The code you pass to SQL via your code looks something like:
Dim SqlTxt As String
SqlTxt = "UPDATE Customer SET Comment = " & TextBox1.Text & " WHERE CustomerId = 1000;"

What the database will see is 2 seperate commands:
UPDATE Customer SET Comment = '';
SQL injected statement;

Now, let's change the sqlTxt line like this:

TextBox1.Text = "''; SQL injected statement --"

The code you pass to SQL via your code looks something like:
Dim SqlTxt As String
SqlTxt = "UPDATE Customer SET Comment = " & [!]Replace([/!]TextBox1.Text[!], "'", "''")[/!] & " WHERE CustomerId = 1000;"

What the database will see is:
UPDATE Customer 
SET    Comment = '[!]''''; SQL injected statement --[/!]'
WHERE  CustomerId = 1000;

Note that the when SQL sees consecutive single-quotes, it is treated as a single quote of DATA, not a string delimiter. The first single-quote is seen as a string delimiter, the next 2 are seen as a single-quote of data (not a delimiter). The next 2 would also be seen as data, not a delimiter.

So, in this case, the data in the comment column of the customer table for just the one customerid = 1000 would be:

[tt](single-quote)(single-quote); SQL injected statement --[/tt]



-George
Microsoft SQL Server MVP
My Blogs
SQLCop
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[mrdenny]

Putting on my sales guy hat for a moment...

I've got a book on SQL Security coming out where I talk about all the bad things people can do via SQL Injection (including viruses that get into your network and cause all sorts of problems). The book should be available in about a month or so, and is currently available from Amazon on pre-order.

http://www.amazon.com/Securing-SQL-...6251/ref=sr_1_1?ie=UTF8&qid=1294260564&sr=8-1

Denny
MVP
MCSA (2003) / MCDBA (SQL 2000)
MCTS (SQL 2005 / SQL 2005 BI / SQL 2008 DBA / SQL 2008 DBD / SQL 2008 BI / MWSS 3.0: Configuration / MOSS 2007: Configuration)
MCITP (SQL 2005 DBA / SQL 2008 DBA / SQL 2005 DBD / SQL 2008 DBD / SQL 2005 BI / SQL 2008 BI)

My Blog

 

[gmmastros]

Denny,

Do you know if there will be a kindle version? (I just got one for Christmas.)

-George
Microsoft SQL Server MVP
My Blogs
SQLCop
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[jsteph]

Sooner, George,
Ok, I can see the statement *prior* to the injection getting executed and commited (and I get a quote-character in the comment field), but with the "--" broken up to "- -" (as in my simplistic sanitation method), would you then get an error on the actual injection statment? It sees a close-quote with no open-quote and the statement fails, correct?
--Jim

 

[gmmastros]

In my example, where you replace a single-quote with 2 single-quotes, the sql would NOT get executed at all. In fact, you would see the entire string that the hacker used in the data.

-George
Microsoft SQL Server MVP
My Blogs
SQLCop
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[ousoonerjoe]

jsteph...

The semicolon acts as an End of Statement character. The injected code will end with "; - - WHERE...". The answer is no. The error occurs on the statement directly AFTER the injected code. Injected Code still fires successfully.

--------------------------------------------------
“Crash programs fail because they are based on the theory that, with nine women pregnant, you can get a baby a month.” --Wernher von Braun
--------------------------------------------------

 

[mrdenny]

George,
I've asked my editor (you aren't the first person to ask me). When I get an answer I'll let you know.

Denny

Denny
MVP
MCSA (2003) / MCDBA (SQL 2000)
MCTS (SQL 2005 / SQL 2005 BI / SQL 2008 DBA / SQL 2008 DBD / SQL 2008 BI / MWSS 3.0: Configuration / MOSS 2007: Configuration)
MCITP (SQL 2005 DBA / SQL 2008 DBA / SQL 2005 DBD / SQL 2008 DBD / SQL 2005 BI / SQL 2008 BI)

My Blog

 

[jsteph]

Sooner,
Thanks...my tests act differently, but I may be doing things different:
When I say we've got 'Raw SQL' here, I'm not giving the users a command line or even using Exec(@someSQLString).

Below is typical of the ASP apps we've got out there(assuming variables dim'd, etc):

strUser = Request("Username")
strSomeValue = Request("SomeValue")
strUser = replace(strUser,"--","- -") 'crude sql inj. sanitization
strSomeValue = replace(strSomeValue,"--","- -")
strSomeValue = replace(strSomeValue,"'","''") 'escape single quote

Set CMD = server.createobject("adodb.command")
cmd.commandtype = adCmdText
cmd.activeconnection = Conn 'existing
cmd.commandtext = "Update tblUsers Set Somefield = '" & strSomeValue & "' Where username = '" & strUser & "'"
cmd.execute
if err.Number <> 0 Then '(on error resume next is on here to suppress error info from displaying on page)
    'below is a text-file logwriting function
    WriteToLog "Error executing statement for user " & struser & " value: " & strSomeValue
end if

In the example above, I've tried every which way described in previous posts and I always get an error--nothing executes, (which is good). I even hardcoded two pefectly formed UPDATE statements, both followed by semicolons, and then a - -' (as would be the case with the attack example in previous posts) and assigned it directly to into the Cmd.CommandText, and still I check the value in the table I'm updating and none of the statements executed--I just get the error about the stray dash "incorrect syntax".

I've never explicitley set or cleared any 'autocommit' option--to the best of my knowledge the Server (Sql server 2005) is set up with the default MS settings.

I'm not trying to be difficult here--I just want to know if I've missed something in this. I know there are other ways of attack but the crux of what I'm getting at here is that it appears to me that if there's and error *anywhere* in the command text, the entire statement fails.
--Jim

 

[ousoonerjoe]

Interesting. I know at one time I tried exactly what you have, and it still worked. It did not act as an "all or nothing" execution. It committed the lines it could and only reported the lines that failed. Mind you, it's been a couple years since then. If i get a chance, I'll try to put together an example of how it can still work.

--------------------------------------------------
“Crash programs fail because they are based on the theory that, with nine women pregnant, you can get a baby a month.” --Wernher von Braun
--------------------------------------------------