Part 2 of A Handful of Vulnerabilities

Category: Input Validation

Short Description: A user is able to craft some input to the program that modifies an SQL query into doing something that it shouldn't.

Alarm-bell Code: sqlStatement.execute("UPDATE users SET firstName = '" + userInputFirstname + "' where userid = " + userId);

Discussion

In the above alarm-bell code, what happens if the user supplies a name as follows is left as an exercise for the reader:

This is a really, really common problem in web applications. Parameter quoting is often forgotten, or hacked on the side of the SQL library, and the more complicated the application, the more likely something vital is missed. Just the week before writing this article, two new SQL injection vulnerabilities were announced for Bugzilla (one, two), a Perl application that has had its source open to public scrutiny for over five years.

The good news is that in Java, 95% of this problem is fixed by using PreparedStatement instead of Statement. Most JDBC drivers will prepare a statement with the server, and then supply the parameters separately.

The bad news is that none of this behaviour is mandated by the JDBC specification (as some primitive SQL servers or drivers don't support preparing statements this way). If the driver doesn't prepare statements properly, you have to trust that it knows how to quote parameters. While this is still more likely to be reliable than a home-cooked quoting solution, it means that as part of your system testing, you should test for the possibility of SQL injection against whichever driver you've chosen.

Try entering data with combinations of single quotes, double quotes, the same escaped by various levels of backslashes, and even NUL (\0) characters. If you get an SQLException, you have a problem.

SQL injection is not a particularly big problem in Java because PreparedStatement is mostly safe, and most of us know by now to use PreparedStatement. We are taught to do this for reasons of efficiency, but security is just as important a reason.

Because we're only taught to use PreparedStatement for efficiency, when you're building up a long dynamic statement it's sometimes tempting to hand-hack it together with string concatenation. After all, you're less likely to get the benefits of statement caching if you're building the query dynamically, and dealing with parameter numbering issues can be a pain. If you're querying on user-supplied data, resist that temptation.

Languages that don't have prepared statements should come with standard functions for quoting SQL parameters. Use them. However, you should mistrust any function that assumes generality across multiple databases: it's likely to have missed something. Use the one that was written specifically for the database driver you are accessing.

Related:

Java programs rarely exec() other apps, because it goes against that whole cross-platform thing. It is, however, a common way to get things done in a lot of other languages. You should therefore keep in mind that it is possible to manipulate quotes to supply bogus parameters to an application in Runtime.exec() in the same way that it is possible to inject code into an SQL query.

Your first line of defence is using the form exec() that takes an array of arguments instead of a single string. This should prevent you from having to worry about people escaping from quotes within a single argument, but when testing this theory on JDK1.3 on my Debian box, I was able to craft a command line using triple-backslashed double-quotes that crashed the JVM. So be very, very careful when it comes to exec().