Search code examples
javasql-serverpostgresqlsql-injection

java CallableStatement vs SELECT regards to SQL injection


I am hoping to understand CallableStatement a little better. So the case is I have a stored functions in my postgre database.

Calling these functions from java where the parameters are user input (HTTP POST parameters) I have to worry about SQL injections.

So doing this in java:

String statementSQL = "SELECT my_function(" + toPostgresString(person.getEmail()) + "," + toPostgresString(person.getPhone()) + "," + toPostgresString(person.getDealType())+ ");"; 

where

private static String toPostgresString(String value) {
    if (value == null || value.equals("")) {
        return POSTGRES.NULL;
    } else {
        return "'" + value + "'";
    }
}

The latter method makes sure that all params are treated as strings, but I read that this solutions is still dangerous when params are malicious.

So I tried using CallableStatements as they are said to be safer.

CallableStatement statement = connection.prepareCall(" { call my_function( ?, ?, ? ) } ");

statement.setString(1, person.email());
statement.setString(2, person.phone());
statement.setString(3, person.getDealType());
statement.execute();
statement.close();

Question: In both cases SELECT and CallableStatement the params are sent to database function and treated there as datatypes defined by the function itself. CallableStatement only fails when datatypes don't match (in the declared function and the statement.set().

How is CallableStatement actually safer vs SELECT when in both cases the params are treated as string (but between '').

Thanks.


Solution

  • Both PreparedStatement and CallableStatement offer the setXXX methods to set parameters. They don't actually sanitize the parameters, they just use a binary type for them on the wire, so the "in both cases the params are treated as string" part is wrong.

    PreparedStatement also offers some minor performance advantages since the driver will actually create a server side prepared statement after a query has been called enough times.

    You should never use a normal Statement unless absolutely necessary, and always use the setXXX methods since that's what they're for.