SQL Server Forums
Profile | Register | Active Topics | Members | Search | Forum FAQ
 
Register Now and get your question answered!
Username:
Password:
Save Password
Forgot your Password?

 All Forums
 SQL Server 2005 Forums
 Transact-SQL (2005)
 SQL injection problems
 New Topic  Reply to Topic
 Printer Friendly
Author Previous Topic Topic Next Topic  

parrot
Posting Yak Master

USA
132 Posts

Posted - 01/03/2013 :  15:54:05  Show Profile  Reply with Quote
I thought I had sufficient code in place to prevent SQL injection. For the past year everything was ok. Now my tables are being infected with advertising code being appended to certain fields. I want to make sure my code for preventing sql injection is correct. Below is an example of how I process input fields from a web site.

strSQL += "INSERT INTO mytable ([Category], [Subcategory], [inputfield])"
strSQL += " VALUES ('" + category + "', '" + subcategory + "', ?)";

OleDbCommand myCommand = new OleDbCommand(strSQL, OleDbConn1);
myCommand.Connection = OleDbConn1;

myCommand.Parameters.Add("@inputfield", OleDbType.VarChar, inputfield.Text.Length);
myCommand.Parameters["@inputfield"].Value = inputfield.Text;

try
{
myCommand.ExecuteNonQuery();
}
Note that category and subcategory are hardcoded in program tables whereas inputfield is entered in the web page. Is this sufficient protection for sql injection?
Dave

tkizer
Almighty SQL Goddess

USA
35934 Posts

Posted - 01/03/2013 :  16:00:24  Show Profile  Visit tkizer's Homepage  Reply with Quote
You are concatenating your SQL, which is exactly the problem. Use parameterized queries instead.

Tara Kizer
Microsoft MVP for Windows Server System - SQL Server
http://weblogs.sqlteam.com/tarad/

Subscribe to my blog
Go to Top of Page

parrot
Posting Yak Master

USA
132 Posts

Posted - 01/03/2013 :  16:08:12  Show Profile  Reply with Quote
Tara,
Thanks for your feedback. Can you give me a quick link to an example of a parameterized query? I know I have used those also but I want to make sure I am using it correctly.
Dave
Go to Top of Page

tkizer
Almighty SQL Goddess

USA
35934 Posts

Posted - 01/03/2013 :  16:18:01  Show Profile  Visit tkizer's Homepage  Reply with Quote
I much prefer, well actually demand, that all data access be through stored procedures (for various reasons). Here is how to do it without stored procedures though:


Public Function GetBarFooByBaz(ByVal Baz As String) As String
    Dim sql As String = "SELECT foo FROM bar WHERE baz= @Baz"

    Using cn As New SqlConnection("Your connection string here"), _
        cmd As New SqlCommand(sql, cn)

        cmd.Parameters.Add("@Baz", SqlDbTypes.VarChar, 50).Value = Baz
        Return cmd.ExecuteScalar().ToString()
    End Using
End Function



Tara Kizer
Microsoft MVP for Windows Server System - SQL Server
http://weblogs.sqlteam.com/tarad/

Subscribe to my blog
Go to Top of Page

parrot
Posting Yak Master

USA
132 Posts

Posted - 01/03/2013 :  16:23:36  Show Profile  Reply with Quote
I am wondering why I was led to believe that using the ? would make it immune to sql injection. What is the purpose of using the ? in my example? Thanks for the code. Is it possible to see an example in C#?
Dave
Go to Top of Page

tkizer
Almighty SQL Goddess

USA
35934 Posts

Posted - 01/03/2013 :  16:25:36  Show Profile  Visit tkizer's Homepage  Reply with Quote
http://www.dotnetperls.com/sqlparameter

Tara Kizer
Microsoft MVP for Windows Server System - SQL Server
http://weblogs.sqlteam.com/tarad/

Subscribe to my blog
Go to Top of Page

parrot
Posting Yak Master

USA
132 Posts

Posted - 01/03/2013 :  16:26:35  Show Profile  Reply with Quote
As an additional bit of information on my sql injection defense. I do edit out all input fields having an * or < or ; plus many other characgers yet I still have html code appended to my data field. How can it get by my edits?
Go to Top of Page

robvolk
Most Valuable Yak

USA
15635 Posts

Posted - 01/03/2013 :  16:37:36  Show Profile  Visit robvolk's Homepage  Reply with Quote
There's tons of ways:

http://ferruh.mavituna.com/sql-injection-cheatsheet-oku/
http://hphosts.blogspot.com/2008/09/injection-via-hex-encoded-sql.html

I just found out about hex-encoded injection a few months ago. IMHO, stored procedures are the only way you have a chance at preventing SQL injection. Any dynamic/constructed/ad-hoc SQL at any stage (including inside a stored procedure) has an opportunity to be injected.
Go to Top of Page

parrot
Posting Yak Master

USA
132 Posts

Posted - 01/03/2013 :  16:48:48  Show Profile  Reply with Quote
This makes me want to just say screw it and drop my website after being on the internet for 12 years. I have over 30 programs with 100's of sql commands I would have to go over and make into stored procedures. What gets me is my website is such an insignificant website for a small town - who in the hell would want to hack into it?
Go to Top of Page

parrot
Posting Yak Master

USA
132 Posts

Posted - 01/03/2013 :  16:55:21  Show Profile  Reply with Quote
One more time. What is the difference between my first example above using myCommand.Parameters.Add("@inputfield", OleDbType.VarChar, inputfield.Text.Length); versus using command.Parameters.Add(new SqlParameter("inputfield", inputfied));

Is SQLParameters different from using OleDbCommand?
Go to Top of Page

robvolk
Most Valuable Yak

USA
15635 Posts

Posted - 01/03/2013 :  17:31:56  Show Profile  Visit robvolk's Homepage  Reply with Quote
It's not someone targeting you personally, it's a script or bot that's hitting every site it can. And don't take this the wrong way, but stored procedures have been the recommended method for more than 12 years. It's absolutely worth your time to refactor the site to use them exclusively. Pace yourself, do 1 or 2 a day until they're all done.

I don't know enough about OleDbCommand vs. other methods, I'm not familiar enough with them, but I don't think either one will fully protect against SQL injection unless you do something like:
strSQL = "INSERT INTO mytable ([Category], [Subcategory], [inputfield]) VALUES(@category,@subcategory,@inputfield)"

OleDbCommand myCommand = new OleDbCommand(strSQL, OleDbConn1);
myCommand.Connection = OleDbConn1;

myCommand.Parameters.Add("@category", OleDbType.VarChar, 20);
myCommand.Parameters.Add("@subcategory", OleDbType.VarChar, 20);
myCommand.Parameters.Add("@inputfield", OleDbType.VarChar, inputfield.Text.Length);
myCommand.Parameters["@category"].Value = category;
myCommand.Parameters["@subcategory"].Value = subcategory;
myCommand.Parameters["@inputfield"].Value = inputfield.Text;
Warning: code not tested. You'll need to modify to match category and subcategory types and lengths.

The key to preventing injection is to form valid SQL commands without concatenating strings. You'll notice what I did with the strSQL variable, there's no concatenation, and there's no way to affect the syntax of the SQL statement.

While you can do this without using stored procedures, doing so would greatly reduce the chance of accidental injection. If you have to rewrite them all anyway, take the extra step.
Go to Top of Page

parrot
Posting Yak Master

USA
132 Posts

Posted - 01/03/2013 :  17:45:14  Show Profile  Reply with Quote
I appreciate your feedback and you are right that I should have used stored procedures initially but that is water over the dam now. I guess I really don't know what you mean by concatenating strings. In my example I used a ? instead of @inputfield. Else, the syntax is the same using myCommand.Parameters["@inputfield"].Value = inputfield.Text; Can you tell me what is different besides the ? verus the @inputfield? The first two fields in my commands were not input fields and that is why I didn't parametize them.
Go to Top of Page

robvolk
Most Valuable Yak

USA
15635 Posts

Posted - 01/03/2013 :  18:16:25  Show Profile  Visit robvolk's Homepage  Reply with Quote
Using ? makes it a positional, rather than named, parameter. I prefer named parameters since it's too easy to modify a SQL statement and move something out of order, which won't matter if everything is named.

Even though you are using Command objects and Parameters, your current code is, in essence, dynamic SQL:
strSQL += "INSERT INTO mytable ([Category], [Subcategory], [inputfield])"
strSQL += " VALUES ('" + category + "', '" + subcategory + "', ?)";
It concatenates strings in such a way that SQL can be injected. Probably the best example is the classic:

http://xkcd.com/327/

I'm not sure how ? would be interpreted as a variable, but I know that using my version:
strSQL = "INSERT INTO mytable ([Category], [Subcategory], [inputfield]) VALUES(@category,@subcategory,@inputfield)"
Is not susceptible to injection, as the statement/syntax is closed, and the value passed to the variable is encapsulated as a variable.
Go to Top of Page

parrot
Posting Yak Master

USA
132 Posts

Posted - 01/03/2013 :  18:30:20  Show Profile  Reply with Quote
Thanks for the clarification. I am thinking of replacing the positional ? with the @inputfield qualifier as a temporary fix as I know I can do that probably within a week and work on updating to stored procedures in the next month or two on my test database. I need to get my website back into operation ASAP and really can't wait a month to finish the stored procedures. I am a 71 year old self-employed software programmer with 48 years of coding experience and am trying to keep up with the latest but it is not easy. The only thing I can say is I love programming and want to keep on until I die and along the way I will have to eat humble pie.
Go to Top of Page

robvolk
Most Valuable Yak

USA
15635 Posts

Posted - 01/03/2013 :  19:57:08  Show Profile  Visit robvolk's Homepage  Reply with Quote
If it makes you feel any better, you're not alone:

http://groups.google.com/forum/?fromgroups=#!topic/rubyonrails-security/DCNTNp_qjFM
http://threatpost.com/en_us/blogs/sql-injection-flaw-haunts-all-ruby-rails-versions-010313
Go to Top of Page
  Previous Topic Topic Next Topic  
 New Topic  Reply to Topic
 Printer Friendly
Jump To:
SQL Server Forums © 2000-2009 SQLTeam Publishing, LLC Go To Top Of Page
This page was generated in 0.09 seconds. Powered By: Snitz Forums 2000