This use of PreparedStatement; can be vulnerable to SQL injection (with JDBC)

Question

My sql query looks like this:

String sqlAlloc = " select %1$s from %2$s "
  + "where plot_fk in (%3$s) and plot_fk between ? and ? "
  + "and f001=? "
  + "and repdate=TO_CHAR(TO_DATE(?,'YYYYMMDD'), 'DD-Mon-YY') "
  + "and reportname = ? and change_status in (0,2,6,8,9)";
if (!cond.isEmpty()) {
                sqlAlloc += " and C007=?";
            }
if (tableName.getKey().equals(ALLOC_PENSIONFUNDS)) {
  sqlAlloc += " group by REPDATE, F001, C007, REPORTNAME, COLNAME, ROWNAME";
            }
List<String> values = Arrays.asList(tableName.getValue().split(","));
String sqlAllocFormatted = String.format(sqlAlloc,
  values.stream().collect(Collectors.joining(",")),
  jdbcUsernameMaster + "." + key,
  plotFkMasterPublicList.stream()
    .collect(Collectors.joining(",")));
try (final Connection conn = ds.getConnection();
     final PreparedStatement stmtAlloc =
       conn.prepareStatement(sqlAllocFormatted);) {
  ...

When I scan my code with sonarqube I get the following msg: This use of PreparedStatement; can be vulnerable to SQL injection (with JDBC)

I don't really understand what is wrong with the sql query and how to fix this?

Answer 1

The vulnerability lies in the fact that you are using String.format to inject things into your query string. If the values of values/tableName and jdbcUsernameMaster, key and plotFkMasterPublicList come from an untrusted source, then this could be a potential source of SQL injection.

To fix this, you either need to not use String.format, but static query strings, or you need to ensure that your values are not from an untrusted source (e.g. user input, external services, etc), and then consciously suppress the warning as a false-positive.