Please start any new threads on our new
site at https://forums.sqlteam.com. We've got lots of great SQL Server
experts to answer whatever question you can come up with.
| Author |
Topic |
|
BitShift
Yak Posting Veteran
98 Posts |
 Posted - 2007-01-11 : 17:26:33
|
| My application uses stored procedures exclusively when accepting input from the user. Does this alone, protect me from sql injection ? I was reading the following,http://www.devcity.net/Articles/250/1/article.aspxIm now using regular expressions to validate all user input, but the article got me to thinking about what kind of other potential problems you could face if relying on stored procedures alone.Anyone got another article that lists other potential trouble areas besides procesures that use 'execute' ? |
|
|
snSQL
Master Smack Fu Yak Hacker
1837 Posts |
Posted - 2007-01-11 : 17:44:25
|
| No - if you pass a string to a stored procedure and then concatenate that string into a dynamic SQL string and execute it (with EXEC[UTE] or sp_executesql), you could still have a problem with SQL injection.If you do not use any dynamic SQL in your stored procedures then you are OK. If you do then either do not use any user entered strings in your dynamic SQL, or if you must use user entered strings, then check those strings very carefully. |
 |
|
|
Kristen
Test
22859 Posts |
Posted - 2007-01-12 : 00:45:39
|
| "Im now using regular expressions to validate all user input"Seems a bit overkill ...If you dostrSQL = "SELECT * FROM MyTable WHERE Col1 = '" & document.form[FieldName].value & "'"then you will have trouble.If you do:strSQL = "SELECT * FROM MyTable WHERE Col1 = '" & SafeSQL(document.form[FieldName].value) & "'"and SafeSQL doubles up any embedded ' then you will be safe - I don't see that RegEx tests are required.If you are using Stored procedures you need to use parameters passing - rather than:strSQL = "EXEC MySProc @Param1 = '" & document.form[FieldName].value & "'"because that leaves you in the same boat as the first dynamic SQL example above.And after that you need to be careful if your Sprocs do any dynamic SQL execution, as snSQL describes.Kristen |
|
|
|
jsmith8858
Dr. Cross Join
7423 Posts |
Posted - 2007-01-12 : 08:59:07
|
| parameters are safe only if you use them correctly. Never, ever concatenate parameters with sql statements and execute them, either at your client or in T-SQL. Always, always explicitly place parameters in your SQL statemetns, and then assign those parameters values. At the client, in ADO.NET for example, you use the Paramters() property of a SqlCommand to do this; in T-SQL, you would use either not use dynamic SQL at all and just reference the parameters in your SQL, or if you do need dynamic SQL for some reason, you use sp_executeSQL which provides an easy and safe way to assign parameter values without needing any concatenation.Please see:http://weblogs.sqlteam.com/jeffs/archive/2006/07/21/10728.aspxandhttp://weblogs.sqlteam.com/jeffs/archive/2006/04/21/9651.aspxfor some thoughts and examples. read the comments, too.If you use parameters properly, you will *NEVER* suffer from a SQL injection attack even if you never validate any input. the only possible errors you will get will be datatype conversion errors if your parameter requires an INT or a Datetime and you try to pass an invalid string. The validation you need to do is to be sure that the values themselves are valid; i.e., if firstname is required, or that date-of-birth is a valid date in the format you expect, or that a particular number is positive or not equal to zero. I find it kind of funny, actually, the amount of work people do to avoid sql injection when either a) they are not vulnerable because they are using parameters properly , or b) they could simply do things the correct way in the first place and not worry about it at all.- Jeff |
|
|
|
mr_mist
Grunnio
1870 Posts |
Posted - 2007-01-12 : 10:36:14
|
| Personally I think it's worth validating all form input first anyway, because otherwise you are potentially wasting a round-trip to the DB server.-------Moo. :) |
|
|
|
jsmith8858
Dr. Cross Join
7423 Posts |
Posted - 2007-01-12 : 10:47:58
|
| Agreed. validate it for content and to verify that things required are there and have valid values, but not for sql injection. i.e., Do not escape ' characters in names like "O'Neil" with double '' and so on ... if you use parameters correctly, you'll be storing "O''Neil" in your database, which you don't want!- Jeff |
|
|
|
snSQL
Master Smack Fu Yak Hacker
1837 Posts |
Posted - 2007-01-12 : 11:53:23
|
quote:
parameters are safe only if you use them correctly. Never, ever concatenate parameters with sql statements and execute them, either at your client or in T-SQL.
quote:
If you use parameters properly, you will *NEVER* suffer from a SQL injection attack even if you never validate any input.
You kind of contradict yourself here, I know what you're saying - that proper use of parameters means never concatenating them into dynamic SQL, but realistically people will want to do that from time to time. I AM NOT ADVOCATING THE USE OF DYNAMIC SQL, but if someone wants to use dynamic SQL, rather than saying that is improper use of parameters, lets be realistic and say - IF YOU USE DYNAMIC SQL THEN CHECK/VALIDATE THE PARAMETER VALUES EXTREMELY CAREFULLY. |
|
|
|
jsmith8858
Dr. Cross Join
7423 Posts |
Posted - 2007-01-12 : 13:13:59
|
| snSQL -- no, that is not true. did you read the link I provided? you can use dynamic SQL as much as you want, and if you use it *properly* and use parameters *properly* you never have to worry about sql injection. ever. You don't have to check or escape any input for anything other than data types and business rules; you should never write a single line of code in any app that escapes ' into '' or things like that; it is never necessary.As for my statements, there's no contradiction at all; in fact, I am repeating the same thing twice. From your quotes, point #1 is to simply never concatenate parameters into a SQL statement and execute it. point #2 is simply re-iterating that if you use parameters properly, you will be fine. I am actually the same thing, said different ways. If you are using parameters wrong, don't write code to validate and escape your input. Fix your code. It is WRONG. there's never an excuse for writing crappy code and then compounding the issue by writing even MORE crappy code to compensate. Do it right. it's actually EASIER to do it right in the first place, as I demonstrate in my blog post on the topic.- Jeff |
|
|
|
snSQL
Master Smack Fu Yak Hacker
1837 Posts |
Posted - 2007-01-12 : 15:54:43
|
| True enough - if you use sp_executesql with parameters, even varchar parameters, instead of concatenation and EXEC[UTE] then you are safe. |
|
|
|
snSQL
Master Smack Fu Yak Hacker
1837 Posts |
Posted - 2007-01-26 : 15:37:15
|
quote:
you can use dynamic SQL as much as you want, and if you use it *properly* and use parameters *properly* you never have to worry about sql injection. ever.
Jeff, I thought about this some more. Here's a case where you cannot use parameters *properly*.create table #test (col1 int)insert #test values (1)insert #test values (2)select * from #testdeclare @sql nvarchar(2000)declare @param nvarchar(20)set @param = N'col1'--This dynamic SQL, which does not use parameters, worksset @sql = N'UPDATE #test SET ' + @param + N' = 100'exec sp_executesql @sqlselect * from #test--This dynamic SQL, which uses parameters, does not workset @sql = N'UPDATE #test SET @param = 200'exec sp_executesql @sql, N'@param nvarchar(20)', @paramselect * from #testdrop table #test I'm sure you'll argue that you should not create dynamic SQL like this, but the fact is that if you need to do this you will have to check the column name yourself and then concatenate it into the dynamic SQL, you cannot pass the column name as a parameter.Hopefully it's never necessary to use user input in a case like this, but I think it's worth pointing out. |
|
|
|
jezemine
Master Smack Fu Yak Hacker
2886 Posts |
|
|
russell
Pyro-ma-ni-yak
5072 Posts |
Posted - 2007-01-26 : 23:30:53
|
quote:
Originally posted by jsmith8858
you should never write a single line of code in any app that escapes ' into '' or things like that; it is never necessary
If you mean T-SQL code, then I agree. But if you mean client application code, then no, can't agree. There are other security issues to consider as well. If I can break your application by simple entering a single quote into a form, querystring or by sending request headers with home-baked cookies, then I have at least a 50-50 chance of hacking in. On the subject of cookies, this is one that developers seem to almost always ignore when writing validation routines. It is important to remember that real hackers aren't using web browsers to hack your network. They are writing scripts, and have ready made compiled applications. There is only one firm rule for any internet-facing application: if it's user input, validate it. There aren't any exceptions to this one. This goes for web apps, compiled apps, desktop apps...everything. |
|
|
|
|
|
|
|
|
do stored procedures = safe from sql injection ?