Avoiding "Security - Prepared Statement Created from Inconstant String" FindBugs Warning

Question

I am working on a project that has a piece of code, as shown below:

String sql = "SELECT MAX(" + columnName + ") FROM " + tableName; PreparedStatement ps = connection.prepareStatement(sql); 

Is there any way to change this code so that FindBugs stop giving me "Security - prepared statement is generated from an inconsistent string warning?

Suppose this code is safe against SQL INJECTION, because I can control the possible values ​​for "tableName" and "columnName" elsewhere in the code (they do not come directly from user input).

Answer 1

Do not concatenate sql string by + . you can use

 String sql = String.format("SELECT MAX(%s) FROM %s ", columnName, tableName); 

This is slower than string concatenation, so you should initialize this static , then this is not a problem.

I think using StringBuilder also eliminate this warning.

Another way to avoid this warning is to add @SuppressWarnings("SQL_PREPARED_STATEMENT_GENERATED_FROM_NONCONSTANT_STRING") above this line (or method / or class).

You can also use the Filter File to determine the rules to exclude.

Answer 2

 private static final String SQL = "SELECT MAX(?) FROM ?"; PreparedStatement ps = connection.prepareStatement(sql); ps.preparedStatement.setInt(1,columnName); ps.preparedStatement.setString(2,tableName); 

if you use a prepared statement, then the parameter must be the final string, and the parameters must be added later using the setInt, setString methods.

this will resolve the findbug warning.

Answer 3

Try using the following ...

 private static final String SQL = "SELECT MAX(%s) FROM %s"; 

And then using String.format () call when you use it ...

 PreparedStatement ps = connection.prepareStatement(String.format(sql,columnName,tableName)); 

If this does not solve the problem, you can always ignore this check; disable it in the FindBugs configuration.

If this does not work (or it is not an option), some IDEs (for example, IntelliJ) will also allow you to warn warnings with specially formatted comments or annotations.

Answer 4

Neither String.format nor StringBuilder (or StringBuffer) helped me.

The solution was isolated by "prepareStatement":

 private PreparedStatement prepareStatement(Connection conn, String sql) throws SQLException { return conn.prepareStatement(sql); } 

Answer 5

You can use concatenation to create your string. This does not raise a security warning. And preferable for clarity, when it comes to long SQL statements that would be better written, breaking into many lines

Using variables to build a string triggers a security warning.

This will result in a warning:

 String columnName = getName(); String tableName = getTableName(); final String sql = "SELECT MAX(" + columnName + ") FROM " + tableName; PreparedStatement ps = connection.prepareStatement(sql); 

This will not work:

 String columnName = getName(); String tableName = getTableName(); final String sql = "SELECT MAX(" + "?" + ")" + "FROM " + "?"; PreparedStatement ps = connection.prepareStatement(sql); ps.setString(1, columnName); ps.setString(2, tableName); 

This does not work, because prepared statements only allow you to bind parameters to the "values" bits of the SQL statement.

This is a solution that works:

 private static final boolean USE_TEST_TABLE = true; private static final boolean USE_RESTRICTED_COL = true; private static final String TEST_TABLE = "CLIENT_TEST"; private static final String PROD_TABLE = "CLIENT"; private static final String RESTRICTED_COL ="AGE_COLLATED"; private static final String UNRESTRICTED_COL ="AGE"; .................... final String sql = "SELECT MAX(" + ( USE_RESTRICTED_COL ? RESTRICTED_COL : UNRESTRICTED_COL ) + ")" + "FROM " + ( USE_TEST_TABLE ? TEST_TABLE : PROD_TABLE ); PreparedStatement ps = connectComun.prepareStatement(sql); 

But it only works if you need to choose between two tables whose names are known at compile time. You can use compound ternary operators for more than two cases, but then it becomes unreadable.

The first case may be a security issue if getName () or getTableName () gets the name from untrusted sources.

It is possible to build a secure SQL statement using variables if these variables have been previously validated. This is your case, but FindBugs cannot figure it out. Findbugs cannot know which sources trust or not.

But if you must use the column or table name from the user or untrusted input, then there is no way around it. You must confirm yourself with this line and ignore the Findbugs warning by any of the methods suggested in the other answers.

Conclusion There is no ideal solution for the general case of this question.

Answer 6

If you are convinced that this is not possible with SQL injection, use the SuppressFBWarnings annotation method:

 @edu.umd.cs.findbugs.annotations.SuppressFBWarnings("SQL_PREPARED_STATEMENT_GENERATED_FROM_NONCONSTANT_STRING")