Fortify flagging query as sqlInjection when passing in parameters to a method

Question

We have a method in our database layer which looks like this:

public List<String> getNamesFromId(List<Long> idsList){
   StringBuilder query = new StringBuilder();
   query.append("Select first_name from person where id in (");

    for (int pos = 0; pos < idsList.size(); pos++) {
        query.append("?");
        query.append(",");
    }
    query.deleteCharAt(query.length() - 1).append(")");

    try {
        conn = establishConnection();           
        pstmt = conn.prepareStatement(query.toString());
        for (int i = 0; i < selections.size(); i++) {
            pstmt.setLong(i + 1, idsList.get(i));
        }
        rs = pstmt.executeQuery();
    } catch (SQLException e) {
        //
    }

    try {
       List<String> namesList = new ArrayList<String>();

        while (rs.next()) {


                namesList.add(rs.getString("FIRST_NAME"));


        }
    } catch (SQLException e) {
        //
    }
    // close the Connection object
    try {
        rs.close();
        pstmt.close();
        conn.close();
    } catch (SQLException e) {
        //
    }

During our fortify scan it is flagging this as SQL Injection saying "invokes a SQL query built using input potentially coming from an untrusted source. This call could allow an attacker to modify the statement's meaning or to execute arbitrary SQL commands."

Is this because this is a public facing method and we are passing in the parameters for the IN part of the prepared statement? If so how can we do it better? Or is it a false alarm from fortify?

Answer 1

This is a false alarm, you're doing it the right way.

There are frameworks that can help you with this (e.g. Spring's NamedParameterJdbcTemplate, but they basically do the same thing under the hood.

The static analyzer is probably catching the fact you're building your query by concatenating strings, or that the size of input is somehow involved, and flagging it as a danger (only guessing here).

---

On a side note, a potential problem that however has nothing to do with SQL injection is that you can only use certain number (DB-dependent) of those parameters - AFAIK the limit's 1000 in Oracle, about 2000 in Teradata, not sure about others. If you need to put many values in that IN clause, you'll need to use a different approach like using a temporary table or performing the query in smaller batches and merging the results in Java.