[FIXED] SQL injection vulnerability with order by clause in Java

Issue

Veracode report is showing a SQL injection flaw for the below query. There are few parameters which I need to get the property file and then need to inject into my SQL, i.e. schema name, sorting order, etc.

I tried to use the %s with String.format, but veraCode is still showing it as a flaw. For the parameter it is fine, since I used map, but for schema and sorting order, it's still showing a flaw.

Any approach to solve this vulnerability?

phoneLogsQuery = "(select * from %s.SHORETEL_EVENTS_CALL_LOGS where CONVERT( date, CallDateTime,112 ) >  CONVERT( date, GETDATE()-%s,112) "
                    + " and (CALLER_CONTACT_ID in (:contactId) or CALLED_CONTACT_ID in (:contactId)) and EXTERNAL_CALL = 1 "
                    + "UNION "
                    + "select * from %s.SHORETEL_EVENTS_CALL_LOGS where CONVERT( date, CallDateTime,112 ) >  CONVERT( date, GETDATE()-%s,112) "
                    + " and (CALLER_CONTACT_ID in (:contactId) or CALLED_CONTACT_ID in (:contactId))"
                    + " and GUILOGIN_NAME = :guiloginName and EXTERNAL_CALL = 0)"
                    + " order by CallDateTime %s %s ";
        
phoneLogsQuery = String.format(phoneLogsQuery, schemaname, phoneLogAllData, schemaname, phoneLogAllData, sortDir, offsetQuery);
shoretelPhoneLogRow = jdbcTemplate.query(phoneLogsQuery,params,
        new ShoretelPhoneLogMapper());

Solution

For column values you should use prepared statement. It makes injection impossible.Example :

jdbcTemplate.query("Select * from user where id=?"), new PreparedStatementSetter() {
            public void setValues(PreparedStatement preparedStatement) throws SQLException
            {
                preparedStatement.setLong(1, id);
            }
        }, new ResultSetExtractor<User>() {
...                
}});

Unfortunately, for column names, schema names, etc., you can use only concatenation or replace method:

"SELECT quantity from $tableName where event_id=?".replace("$tableName", "myTable")

The main thing that you should understand is that injection comes from end users and if those schema name, table name, etc., are inner info enduser can't change it. So you shouldn't be afraid of SQL injection.

Answered By - Ivan Lymar
Answer Checked By - Mary Flores (JavaFixing Volunteer)