My colleague Brian just posted about an error page he encountered on a public ecommerce site, and the clues it gave him about how the coding of that site was wrong in a lot of ways. He gave some good tips on fighting SQL Injection attacks, but I think he wasn’t as complete as he could have been on his recommendations. So I’m going to use this opportunity to clear up some common misperceptions about how best to defend against SQL injection attacks.
I know, you’re thinking "Why another post on SQL injection"? But hey, think of it like the airline safety demo. Just because you know how to buckle your seatbelt and use an oxygen mask doesn’t mean the person sitting next to you isn’t learning about it for the first time. 🙂
Perception – I put a dropdown (or listbox) on my web form, which means when the user submits the form, the value for that field will be one of the items in the list, so I don’t need to check it.
Reality – When you have a dropdown or list box on a web form, the value chosen is sent back in the name/value pair as part of the POST data. But that doesn’t mean those are the only possibilities for the value, it just means those are the choices you provided for the user to select from on that form. Remember, the web is stateless. A postback doesn’t connect directly with the form that had the inputs on it that were submitted, resulting in the postback. A hacker could put any value they want in the name/value pair of the POST data (or, for that matter, the query string) and submit it. Granted, ASP.NET’s event model might choke because you come at it from the control and event perspective and not the name/value pairs. Then again, it might not. And other server side languages might let it pass through just fine. So don’t assume that the only values you’re going to get back are from the list you send out on the page.
Perception – I’ll just write code that replaces all the single quotes in the user’s input with two single quotes in a row, and I’ll be fine. Also, I’ll look for bad words like DROP TABLE
Reality – A lot of people are under the impression that using regular expressions to clean the input (i.e. strip extra quotes, block specific words through blacklisting, etc) means you can now take the cleansed input and continue with the same technique of concatenating string constants with user input to produce the SQL statement you want. Unfortunately, there are still ways around this, and clever hackers can send in a string that your algorithms will helpfully "clean up" back into malicious SQL code.
Perception – SQL injection attack is a SQL Server specific problem, and doesn’t affect other database products.
Reality – Come on, no one really thinks *that*, do they?
Perception – I’ll move all of my dynamic SQL as a stored procedure will in and of itself remove the SQL injection vulnerability.
Reality – It’s just as easy to introduce SQL injection vulnerabilities with a stored procedure if you call them incorrectly from your code. Suppose you convert the dynamic SQL shown in Brian’s post, for example) into a sproc called getBarcode that returns the barcode when sent a unique ID. If you then call the sproc like this, you’re asking for trouble:
SqlCommand myCommand = new SqlCommand("EXEC getBarcode ‘" + scrambledID + "’", myConn);
You’re still concatenating, so malicious SQL statements can still piggyback at the end of the parameter. Your sproc will run, and then the extra SQL statements sent in will run too.
Almost as bad (but less frequently done) is to use the EXEC statement inside the stored procedure itself. Seriously, I’ve worked with folks who thought this was the only way to accomplish what they wanted. Their sprocs were pages and pages of IF and CASE statements, evaluating what permutation of parameters was sent in, and building SQL on the fly. So even if you call this by using SQL Parameters, you can still pass through a string that might contain malicious SQL statements into your sproc where they could be EXECuted.
The bottom line is that the one way to most effectively guard against SQL injection attacks is to parameterize your SQL statements, be they sprocs or dynamic SQL. Never, never, never concatenate to build your SQL string. In addition, I recommend reading this article from MSDN magazine on SQL injection. Now, return your tray tables to their full upright and locked position for takeoff. 🙂