How do I mitigate a SQL injection vuln?


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!

Comments (22)

  1. Dave says:

    " 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.

  2. Bob Barrows says:

    <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.

  3. EricLippert says:

    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.

  4. roberthahn says:

    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.

  5. roberthahn says:

    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.

  6. bleroy says:

    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.

  7. mike says:

    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:

    http://www.unixwiz.net/techtips/sql-injection.html

  8. Binu Purayil says:

    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…

  9. EricLippert says:

    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.

  10. EricLippert says:

    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.

  11. O(1) says:

    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.

  12. EricLippert says:

    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!

  13. 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

  14. Mike says:

    "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).

  15. Gabe says:

    I never understood why so many people write code that’s so vulnerable. I just always write a function like this:

    function SQLString(val)

    ….if val is null then

    ……..return "NULL"

    ….else

    ……..return "’" + str(val).replace("’", "”") + "’" ‘replace single apostrophe with two apostrophes

    end function

    Just so long as all user inputs (even dates and numbers) are passed through this function, no injection attack is possible with SQL Server. Why doesn’t everybody do this?

    And I should add that filtering out bad characters is generally a bad idea. There’s nothing worse than not being able to use a secure password on a site because some fool that doesn’t know how to prevent code injection decided that they need to make all non-alphanumeric characters illegal.

    Figuring out what characters to allow is much trickier than preventing injection attacks every other place (SQLString function, stored procs, prepared statements), so it’s probably more harmful to your product than beneficial. For example, a naive coder writing a regex to only accept numbers might write /[0-9]+/, only to realize he forgot about negative numbers  and decimals (now he has [.0-9-]). That works fine until somebody tries to enter scientific notation, and now he has [.eE0-9+-]. And even then it would fail for somebody in Europe who uses commas for decimal places. I believe the correct regex is [.,eE0-9+-], and even then I could be forgetting about some valid possibilities I hadn’t thought of.

    Cross-site scripting (XSS) attacks are much harder to prevent, though. HTML-encoding every literal string is easy, but user input that can include HTML is much more of a problem. These really do require a very strict HTML white-list for tags and attributes. Otherwise you will have to write an HTML parser that keeps up with all the latest browser features to know what’s code and what isn’t. Besides, you probably want to be careful about what tags you allow anyway (you likely don’t want <meta> or </body> tags, and unclosed tables can screw up your layout).

  16. Jonas says:

    is no one else bothered by the fact that using stored procedures for simple queries is blaringly bad design? You move business logic (what do I want to know) into the storage layer of your application where it doesn’t belong and is hard to manage and maintain.

    Use prepared statements, preferably with named parameters! And avoid the problem of having to change a query that is used multiple times in the application by moving it into a constant.

    and @Gabe: I know you only talk about SQL Server, but your supposed fix breaks on MySQL if you input " blah’ ", because MySQL allows different escaping characters. Rule-of-thumb: only vendor-supplied character escape functions are safe (and even those have bugs sometimes, prepared statements are a much better solution).

  17. Well, OK, I’m a dinosaur.

    I’m not proud of this, and I’m not going to boast.  The only reason that the server-side systems I write (or more accurately co-write) are not vulnerable to SQL injection attacks, or indeed any other sort of insertion attack, is that they tend to have very constrained interfaces; basically yacc’n’lex parsing.

    Now, as Hunter S Thompson once said, "I don’t recommend drugs and violence for everybody; they just happen to work for me…"

    I loathe stored procedures, and if you happen to use them across an unprotected interface that’s open to dynamic SQL, then you are basically a moron and deserve to be kicked in the kidneys.  Like many other database features, they have their place, but they are wildly overused.

    Now, let’s attack this problem from the other end of the telescope.  Sure, you can layer one, two, three or more defences against SQL injection attacks and the like, but these things tend to be automated nowadays.  There are a million idiots out there who will patch together vulnerable sites — and the percentage vulnerability is quite frankly terrifying.

    You could take the Darwinian approach and claim that these sites deserve to be attacked, and perhaps they do.  I don’t suppose it does Joe Public many favours to be exposed to identity fraud and the like, and it probably does none of us a favour in the long run, but what they heck? These people deserve it.  Their sites should be hunted down and destroyed like the rats’ nests they are.

    On the other hand we could take a more aggressive approach to fraud (which is basically what an SQL injection attack is).  Why not welcome it? Why not make it easy for an attacker to feed destructive guff into a bare SQL statement?

    Think about it.

    It’s easier to parse an attack like this (hell, a simple regular expression could do it — even PHP manages this) and throw an alert at the proper authority than it is to build in n-million layers of security into what is, fundamentally, an insecure and ill-thought-out mass-distribution network.  Yup, that’s WWW for you.

    Seriously.  Has anyone considered this sort of approach?

    — Pete

  18. Shaun Kruger says:

    One of the first things I do is find a function that will escape my untrusted input strings.

    Basicly I look for backslash, quote, and double quote characters.

    For my PostgreSQL database projects I always throw these functions into an appropriate class (usually class DBEscape).

    public static string Escape(string str)

    {

    return Escape(str,"\"’");

    }

    public static string Escape(string str,string chrs)

    {

    string ret = "";

    int i;

    int o;

    int oct = chrs.Length;

    int ct = str.Length;

    bool found;

    for(i=0;i<ct;i++)

    {

    found = false;

    for(o=0;o<oct;o++)

    {

    if(str[i] == chrs[o])

    {

    ret += "\"+str[i].ToString();

    found = true;

    }

    }

    if(found == false)

    ret += str[i].ToString();

    }

    return ret;

    }

    For my lisp projects I do this which even has support for adding the encasing single quotes, but not for passing a list of characters to escape.

    (defun db-escape (str &optional addquote)

     (format nil (if addquote

    "’~a’" "~a")

     (format nil "~{~a~}"

     (loop for c across str collect

    (case c

     (#’ "”")

     (#\ "\")

     (t c))))))

    If it’s an input string escape it.  If it’s an input number (in string form) make sure it passes a Convert.Int32().

    I got tired of errors from users wanting to type names to things into the database like "Bart’s call list".  It wasn’t until I wrote my Escape() function that these became a thing of the past.

    I quickly learned that if you escape and validate types you will throw less errors and you’ll go a long way to avoiding SQL injection attacks.  This is a habit that will do me good the rest of my days.

  19. Matthew says:

    >> Just so long as all user inputs (even dates and numbers) are passed through this function, no injection attack is possible with SQL Server. Why doesn’t everybody do this?

    Because most people are smart enough to use parameterised queries, which avoid the problem altogether.

  20. Chad Okere says:

    Stored procedures suck. They make version control a bitch and a half. Used <b>prepared</b> statements. Defined in your code, so they’ll be version controlled, but not mailable. Also you can’t use dynamic SQL with them.