How do I mitigate a SQL injection vuln?

How do I mitigate a SQL injection vuln?

  • Comments 22

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!

  • Indeed!  See

    http://blogs.msdn.com/ericlippert/archive/2003/11/01/53329.aspx

    and

    http://blogs.msdn.com/ericlippert/archive/2003/11/04/53335.aspx

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

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

  • 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

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

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

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

Page 2 of 2 (22 items) 12