| Author |
Topic |
|
parrot
Posting Yak Master
132 Posts |
 Posted - 2013-01-03 : 15:54:05
|
| 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
38200 Posts |
|
|
parrot
Posting Yak Master
132 Posts |
Posted - 2013-01-03 : 16:08:12
|
| 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 |
 |
|
|
tkizer
Almighty SQL Goddess
38200 Posts |
Posted - 2013-01-03 : 16:18:01
|
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 UsingEnd FunctionTara KizerMicrosoft MVP for Windows Server System - SQL Serverhttp://weblogs.sqlteam.com/tarad/Subscribe to my blog |
|
|
|
parrot
Posting Yak Master
132 Posts |
Posted - 2013-01-03 : 16:23:36
|
| 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 |
|
|
|
tkizer
Almighty SQL Goddess
38200 Posts |
|
|
parrot
Posting Yak Master
132 Posts |
Posted - 2013-01-03 : 16:26:35
|
| 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? |
|
|
|
robvolk
Most Valuable Yak
15732 Posts |
Posted - 2013-01-03 : 16:37:36
|
| There's tons of ways:http://ferruh.mavituna.com/sql-injection-cheatsheet-oku/http://hphosts.blogspot.com/2008/09/injection-via-hex-encoded-sql.htmlI 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. |
|
|
|
parrot
Posting Yak Master
132 Posts |
Posted - 2013-01-03 : 16:48:48
|
| 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? |
|
|
|
parrot
Posting Yak Master
132 Posts |
Posted - 2013-01-03 : 16:55:21
|
| 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? |
|
|
|
robvolk
Most Valuable Yak
15732 Posts |
Posted - 2013-01-03 : 17:31:56
|
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. |
|
|
|
parrot
Posting Yak Master
132 Posts |
Posted - 2013-01-03 : 17:45:14
|
| 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. |
|
|
|
robvolk
Most Valuable Yak
15732 Posts |
Posted - 2013-01-03 : 18:16:25
|
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. |
|
|
|
parrot
Posting Yak Master
132 Posts |
Posted - 2013-01-03 : 18:30:20
|
| 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. |
|
|
|
robvolk
Most Valuable Yak
15732 Posts |
|
|
|
SQL injection problems