Are ResultSet update{ColumnType} methods vulnerable to SQL injection?

Question

A security scan made by AppScan source flags that the input has to be validated (Validation.Required) on the line uprs.updateString in the code below:

    PreparedStatement statement = 
        conn.prepareStatement (query, ResultSet.TYPE_SCROLL_SENSITIVE, ResultSet.CONCUR_UPDATABLE);
    ...
    ResultSet uprs = statement.executeQuery ();
    ...
    // Update DB ColumnA with input coming from client
    uprs.updateString ('ColumnA', unvalidatedUserInput);
    ...
    // Updates the underlying database
    uprs.updateRow();

I assume that the intention behind this is to avoid SQL injection attacks, but I'm not sure whether that is possible in that scenario.

Questions: Are SQL Injection attacks possible through these JDBC methods? How does JDBC implements this under the scenes? Would this be another false positive reported by AppScan?

Answer 1

I'm not sure about bluemix-app-scan, but I'm providing my explanation here. (This is my assumption based on the below tests and code pasted)

I ran a test code to check this (in H2 DB)
value of testName String : (select 'sqlInjection' from dual)

1. Using createStatement (Not-Safe):

   String query = "update TEST_TABLE set TEST_CHAR = " + testName + " where ID = 1";
   Statement statement = connection.createStatement();
   statement.executeUpdate(query);
   

   Output: TEST_CHAR in DB was sqlInjection.

2. Using ResultSet of createStatement (Safe in H2 DB):

   String query = "select * from TEST_TABLE where ID = 1";
   Statement statement = connection.createStatement(ResultSet.TYPE_SCROLL_SENSITIVE, ResultSet.CONCUR_UPDATABLE);
   ResultSet executeQuery = statement.executeQuery(query);
   executeQuery.next();
   executeQuery.updateString("TEST_CHAR", testName);
   executeQuery.updateRow();
   

   Output: Surprisingly TEST_CHAR in DB was (select 'sqlInjection' from dual).

3. Using PreparedStatement (Safe):

   String query = "update TEST_TABLE set TEST_CHAR = ? where ID = 1";
   PreparedStatement statement = connection.prepareStatement(query);
   statement.setString(1, testName);
   statement.executeUpdate();
   

   Output: Expected - TEST_CHAR in DB was (select 'sqlInjection' from dual).

4. Using ResultSet of prepareStatement (Safe in H2 DB):

   String query = "select * from TEST_TABLE where ID = 1";
   PreparedStatement statement = connection.prepareStatement(query, ResultSet.TYPE_SCROLL_SENSITIVE, ResultSet.CONCUR_UPDATABLE);
   ResultSet uprs = statement.executeQuery();
   uprs.next();
   uprs.updateString("TEST_CHAR", testName);
   uprs.updateRow();
   

   Output: Expected - TEST_CHAR in DB was (select 'sqlInjection' from dual).

Back to Questions:

Are SQL Injection attacks possible through these JDBC methods?

Maybe. It depends on the database driver that you're using.
How? : The reason SQL Injection failed in my result set update was because H2 database internally uses PreparedStatement to update the row when ResultSet.updateRow() is invoked.

public void updateRow(Value[] current, Value[] updateRow) throws SQLException {
    StatementBuilder buff = new StatementBuilder("UPDATE ");
    ...
    buff.append(" SET ");
    appendColumnList(buff, true);
    ...
    appendKeyCondition(buff);
    PreparedStatement prep = conn.prepareStatement(buff.toString());
    ...
    for (int i = 0; i < columnCount; i++) {
       ...
        v.set(prep, j++);
    }
    setKey(prep, j, current);
    int count = prep.executeUpdate();
    ...
}

I'm not sure if all DB drivers in java implemented updateRow() method using preparedStatement or not. However it's clear that this is left to the driver and if bluemix is suggesting you to add a validation here, I suggest you follow that :)

How does JDBC implements this under the scenes?

Well, as shown above this is driver specific. However there is a good explanation on how PreparedStatement handles it over here.

Would this be another false positive reported by AppScan?

I don't think this is false positive (but in cases like H2 DB it is) but you'll never know if all database drivers implemented this securely.

Edit -
Even PostgreSQL and MySQL use PreparedStatement to handle this.

public synchronized void updateRow() throws SQLException
{
    ...
    updateStatement = ((java.sql.Connection) connection).prepareStatement(updateSQL.toString());
    ...  
    updateStatement.executeUpdate();
    ...
}