Fabulous Adventures In Coding
Eric Lippert is a principal developer on the C# compiler team. Learn more about Eric.
Joel points out today that SQL injection vulnerabilities are common and bad, bad, bad. He does a good job of describing the attack but doesn't really talk about how to mitigate it.
When I advise people on how to close security holes like this I always tell them that closing the original hole is probably not enough. You don't want to make the attackers do one "impossible" thing because you just might be mistaken about what is actually impossible. Make them do three or four impossible things, and odds are good that it really will be impossible to cause harm.
To start with, run all strings that come from users through a string checker looking for anything out of the ordinary. If you expect a string to contain only letters, numbers and spaces, then write a regular expression that verifies that and get in the habit of rejecting all input that doesn't conform. That should make it impossible for attackers to put special characters like quotation marks in there.
Assume that a clever attacker will subvert that. Assume that you've made a mistake and forgotten to put a check in somewhere. Look for every place in the code that uses that user-supplied string. Don't stop at SQL construction. Anything that gets passed to JScript's eval could be an injection attack. Anything that gets echoed back to the user could be a cross-site scripting attack. Anything that gets written to disk could be an attempt to write a script onto the server's disk to trick an admin into running it. Eliminate as many of these as you can.
But how do you eliminate them? A great way to mitigate the risk of a SQL injection attack is to use stored procedures. Stored procedures ensure that only the query that you want to run actually runs. But they have nice properties in addition to being more secure against injection. They can be updated in the database, so that when the database structure changes, you change the stored procedure rather than searching through your code for SQL statement construction. And stored procedures often run faster because the database can optimize itself for them.
How else can we make it impossible for an attacker to run Joel's proposed attack? The attack Joel describes deletes stuff from the database, which is pretty unsubtle. A more subtle attack would consist of an update query which ups the user's available credit, or changes the prices of products, or some such thing. The database code should make use of the principle of least privilege. If you only expect it to look up results from particular database tables, then create a database account that only has those permissions, and then use that account from the server code. Don't connect to the database as admin if you don't need admin privileges; that's just asking for someone to abuse those privileges.
Furthermore, don't keep secrets in source code. For instance, put the name of the database server, the name of the database account, and the password in a registry key or an ACL'd file. Assume that attackers will obtain your source code. Keep machine names, employee names, keep anything sensitive that an attacker could use out of the source code.
How did Joel find out that the server was vulnerable at all? When the server helpfully told him that there was malformed SQL, and furthermore, what part of it was. Not only is this potentially a cross-site scripting attack (because user-supplied data is echoed back) but it is like waving a big sign explaining exactly how to construct an attack! Detailed error messages that describe the internal state of the server should only be given to users who have been authenticated by the server and are known to be server developers. Ordinary users should get a generic "something bad happened" error that explains nothing.
For Joel's proposed attack to succeed, everything has to go wrong. The server has to fail to validate input, then use it in an insecure way, then connect to the database as an administrator. Regrettably, many server-side web apps leave themselves wide open to these sorts of attacks. Eliminate all of these problems, not just the string concatenation. Remember, think like an evil person, assume the worst, and make evildoers jump through as many hoops as you possibly can. Defense in depth!
" A great way to mitigate the risk of a SQL injection attack is to use stored procedures. "
Absolutely! Then call the stored procedures this way:
query="exec InsertRecord @value1=" & request.querystring("value1") & ...
Seriously though, versioning on stored procedures is a pain. Ad-hoc queries aren't *that* slow and you don't have to deal with the DBAs to get work done. For inserts I tend to stick with SPs though.
<quote> use stored procedures </quote>
SQL Injection is still possible if stored procedures are used:
1. If the stored procedure includes code to create and execute a dynamic sql statement using user inputs
2. If the developer uses dynamic sql to execute the stored procedure, as in
sql="exec mysproc '" & userinput & "'"
SQL Injection depends on the use of dynamic sql. Therefore, the best way to defeat it is to not use dynamic sql: use parameters to pass data to either a stored procedure or to a sql string containing parameter markers.
This does not relieve one of the requirement to validate all user input, rejecting suspect data rather than forcing it into the database.
Dave: there is always a balance between security and convenience. Deciding where that balance is requires a cost analysis of each, and the cost of low security is often quite high!
Bob: Yes, I should have been more clear. Call stored procedures with parameters, otherwise you're just trading one injection path for another.
Actually, I'm not sure the advice regarding the string checker is at all that good - as proposed, anyone needing accented characters (or even Kanji) is going to find their content gets stripped out. It's not every day that someone wants to report that a møøse bit their webpage on your DB-backed web app, but it could happen, y'know?
It might be smarter to simply evaluate which non-alphanumerics are bad and strip those out. I wonder how Unicode-savvy SQL interpreters are -- perhaps you could replace 'bad' characters with some visual equivalent that's not in the ASCII range of characters.
whups -- i should mention that dispite that nit, the article overall is very well thought out. I hope that people reading this learn how to put these ideas to work, then actually do it.
Robert: you're basically advocating for a blacklist. That's bad. You're right on the Unicode remark, but that doesn't apply to a numeric field, which was Eric's example. For a text field, you've got to rely on the other layers of protection, because it's a string and basically everything is valid in there, including ' delete * from foo.
Steve Friedl posted a step-by-step walkthrough of how he cracked a SQL Server once. In that case, the door was apparently wide open, but still, it's interesting reading:
my approach to minimize sql injection..
1. Use stored procedure
2. Avoid using dynamic sql in stored procedure
3. Grant only minumum required permission to the account executing the proc
4. validate all user input. use XMLSchema for input validation
Please add more...
What about simply using type system to do this (a la http://blog.moertel.com/articles/2006/10/18/a-type-based-solution-to-the-strings-problem)?
Using the type system to track semantic information about string contents is an excellent idea and a step in the right direction. No doubt about that. But based on the word "simply" in your suggestion, I suspect that you have missed the point of my posting. The point is that good defense requires more than one layer of protection. If an attack requires three things to be broken to work, FIX ALL OF THEM. Don't just fix the strings problem and hope for the best. Take a multi-layered approach to security.
Rob: I said to ensure that the string contains only what you _expect_. If you expect kanji, then write a regular expression filter that allows kanji through!
A few questions do still arise though.
First, what if you WANT to allow " * - ;, etc? The classic approach is to run them through an escapement algorithm that encodes them into alphanumeric entities. Those entities then do not have semantic import when injected into SQL.
Second, what do you do when you get a string that doesn't meet your filter? You can 1) escape it, 2) strip the offending characters, or 3) reject it entirely. Which is the right thing to do depends on whether you need to maintain fidelity of the string and how paranoid you are about people attacking you. If you think that there's a high likelihood that unusual characters are characteristic of attacks, then the sensible thing to do is to put your shields up at the first sign of trouble.
I first heard of these dangerous SQL injection attacks a few years ago and wondered why they were so important. Well, once I understood the nature of this problem it became obvious to me that such a thing would almost never happen to my code because I almost never incorporate user submitted strings in my code. I always compare user supplied data to an internal list and only if it matches my list do I execute some code that I wrote and the code only executes data that I provide. Usually nothing that the user provides ever gets embedded in a command. In the rare cases when you do have to accept some input from the user then it is of paramount importance to correctly use some form of carefully constructed regex that will only pass the data that you expect it to and no more. If people did this then they would almost never suffer from SQL injection attacks and it would become an obscure issue.
Yes, again, you are correct. And again, people seem to be missing my point.
The problem could be solved by allowing the injection attack but limiting the database permissions to only allow reading from certain tables. If everyone did that, it would become an "obscure issue". And if everyone used stored procedures it would be a non-issue.
My point is that there are any number of ways to fix this problem. DO ALL OF THEM. Don't just do enough to fix the problem and then hope for the best. Defense in depth!
Have you ever get any error message telling something's wrong with the SQL statement? If yes, you might consider attack the application in order to, say for example, increase your bank account balance.While SQL Injection is a well-known security vulnerab
"Anything that gets passed to JScript's eval could be an injection attack"
Crikey! I'd say that any script using Eval was a problem in itself. I've yet to come up with a scenario where it was _ever_ necessary (apart from an "execute any script" page, which is bad enough in itself).