‘Solving’ SQL injection in Java


So during the summer I worked on a large enterprisey Java program. (Singleton pattern ahoy!)

One of the annoying things (besides massive code duplication) was it used database queries that naively appended user input (particularly search queries) onto selects.

And from my web background, I knew that SQL injection makes wiping the table trivial. Or even dropping the database.

So I wanted to convert as much stuff to a prepared statement as possible.

I started off with this:

Statement stmt = conn.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY);
ResultSet rs = stmt.executeQuery("select * from view where date = ' "+ inputBox.toString() + " ' ");
if (rs.first()) {

Trial and error get me this:

PreparedStatement test = conn.prepareStatement("select * from viewCalendar where date = ?");
test.setDate(1, new java.sql.Date(inputBox.toString())); 
ResultSet rs = test.executeQuery(); if (rs.next()) {

The hardest part was replacing the functionality of rs.first() – the rest of the code requires a ResultSet that’s started at the first row, but the ResultSet returned by the prepared statement wasn’t. But the Java API docs had the solution – next() “moves the cursor forward one row from its current position. A ResultSet cursor is initially positioned before the first row; the first call to the method next makes the first row the current row.”

Doing this also makes the code cleaner – instead of having

if (rs.first()){
  do {
    //bunch of stuff
  } while (rs.next());
}

The equivalent becomes

while (rs.next()){
    // Bunch of stuff
}

which I consider a whole lot cleaner & easier to read.

So I got my fix – my naive selects were now using preparedStatements (also, possibly JDBC/MSSQL execution path caching?), hence the solved bit in the title.

However, searches are still an issue – due to the way the multiple criteria used resulted in a variable number of parameters, and PreparedStatement not supporting variable numbers of parameters, I didn’t see an alternative to assembling a string, even if I modified the logic to insert nulls – because that would just cause the database to return no rows, because the columns don’t have NULL in the rows that I would want.

Hence the quotes around solved. 🙁

===

And a bonus: When assembling a variable parameter where string, don’t use

if (where.isEmpty()) where = string;
else where += " or " + string;

For one or two parameters it’s ok. But for 20+ parameters, you’re going to have a stupid amount of if/else blocks. I used an ArrayList (variable length!) of type string, and then had a method called buildWhere(ArrayList<String>) that builds a string parameter by parameter with ORs in the appropriate places.

,

  1. No comments yet.
(will not be published)