We are having another discussion here at work about using parametrized sql queries in our code. We have two sides in the discussion: Me and some others that say we should always use parameters to safeguard against sql injections and the other guys that don't think it is necessary. Instead they want to replace single apostrophes with two apostrophes in all strings to avoid sql injections. Our databases are all running Sql Server 2005 or 2008 and our code base is running on .NET framework 2.0.
Let me give you a simple example in C#:
I want us to use this:
string sql = "SELECT * FROM Users WHERE Name=@name";
SqlCommand getUser = new SqlCommand(sql, connection);
getUser.Parameters.AddWithValue("@name", userName);
//... blabla - do something here, this is safe
While the other guys want to do this:
string sql = "SELECT * FROM Users WHERE Name=" + SafeDBString(name);
SqlCommand getUser = new SqlCommand(sql, connection);
//... blabla - are we safe now?
Where the SafeDBString function is defined as follows:
string SafeDBString(string inputValue)
{
return "'" + inputValue.Replace("'", "''") + "'";
}
Now, as long as we use SafeDBString on all string values in our queries we should be safe. Right?
There are two reasons to use the SafeDBString function. First, it is the way it has been done since the stone ages, and second, it is easier to debug the sql statements since you see the excact query that is run on the database.
So then. My question is whether it really is enough to use the SafeDBString function to avoid sql injection attacks. I have been trying to find examples of code that breaks this safety measure, but I can't find any examples of it.
Is there anybody out there that can break this? How would you do it?
EDIT:
To summarize the replies so far:
- Nobody has found a way to get around the SafeDBString on Sql Server 2005 or 2008 yet. That is good, I think?
- Several replies pointed out that you get a performance gain when using parametrized queries. The reason is that the query plans can be reused.
- We also agree that using parametrized queries give more readable code that is easier to maintain
- Further it is easier to always use parameters than to use various versions of SafeDBString, string to number conversions and string to date conversions.
- Using parameters you get automatic type conversion, something that is especially useful when we are working with dates or decimal numbers.
- And finally: Don't try to do security yourself as JulianR wrote. The database vendors spend lots of time and money on security. There is no way we can do better and no reason we should try to do their job.
So while nobody was able to break the simple security of the SafeDBString function I got lots of other good arguments. Thanks!
Answer
I think the correct answer is:
Don't try to do security yourself. Use whatever trusted, industry standard library there is available for what you're trying to do, rather than trying to do it yourself. Whatever assumptions you make about security, might be incorrect. As secure as your own approach may look (and it looks shaky at best), there's a risk you're overlooking something and do you really want to take that chance when it comes to security?
Use parameters.
No comments:
Post a Comment