0

I am trying to write a very simply query using the PreparedStatement class. I read here:Fail to convert to internal representation JDBC that you cannot parameterize column names, only values. Since my query is very simple, the only 'value' I can parameterize is count (*).

This is the query: SELECT COUNT (*) FROM EZ_DAY

If I try to parameterize it like this: SELECT ? FROM EZ_DAY

I get an error: Fail to convert to internal representation when using the method getInt() on the ResultSet.

How can I use PreparedStatement and parameterize something in this query to prevent SQL injection? Also I know you can't parameterize column names, does that include table names? For example, can I do something like:

SELECT COUNT (*) FROM ? ?

Community
  • 1
  • 1
DanGordon
  • 671
  • 3
  • 8
  • 26
  • It's not clear what you're trying to parameterize in the first place. Which part of your query can vary? Is the set of "known good values" fixed? – Jon Skeet Jun 17 '14 at 15:00
  • 2
    You don't have to prevent SQL injection on this query, as there is nothing that could be injected from outside. You don't have any parameters passed into the query, so there is no need to parametrize anything. – MicSim Jun 17 '14 at 15:01
  • Yes that seems to be the problem. I got an automatic warning detection from a service that told me this is susceptible to sql injection - my error was assuming the automation was correct in its evaluation. If there is no possibility for sql injection, then I don't need to use `PreparedStatement`, correct? EDIT: meaning, I can just use `Statement` instead? – DanGordon Jun 17 '14 at 15:05
  • @Dan, the statement `SELECT COUNT (*) FROM EZ_DAY` is not prone to injection. But if you make it `"SELECT " + whatToSelect + " FROM " + whatTableToSelectFrom` it *is* prone. Is that what you wanted to achieve? – chiccodoro Jun 17 '14 at 15:14
  • Nope, keeping it hard-coded as `SELECT COUNT (*) FROM EZ_DAY` is the query I need. I don't need to be changing any parameters. Only needed to if it would help prevent sql injection. – DanGordon Jun 17 '14 at 15:54
  • This question appears to be off-topic because it is based on a false premise. (It is trying to "fix" a non-existent problem.) – Gord Thompson Jun 19 '14 at 21:09

2 Answers2

4

That query cannot fall into SQL injection. The queries that fall in this category are those queries that you build by plain String concatenation. For example:

String query = "SELECT COUNT(*) FROM EZ_DAY WHERE colX = " + stringParameter;
Statement stmt = con.createStatement(query);
ResultSet rs = stmt.executeQuery();

In your case, there's no parameter to inject, so there's no way to have a SQL injection attack for your specific case.

If you need to prevent from SQL injection attacks, use PreparedStatement and do not concatenate the query. Instead, pass the parameters through the interface, which will escape any invalid character for you:

String query = "SELECT COUNT(*) FROM EZ_DAY WHERE colX = ?";
PreparedStatement pstmt = con.prepareStatement(query);
pstmt.setString(1, stringParameter);
ResultSet rs = pstmt.executeQuery();

In case you need to build a dynamic query, then you may fall back into concatenating strings, regardless if you use plain String concatenation or a StringBuilder:

//Common solution, still suffers from SQL injection
String query = "SELECT COUNT(*) FROM EZ_DAY WHERE 1 = 1 ";
if (stringParameter != null) {
    query = query + = "AND colX = " + stringParameter;
}

Instead, it is better to use a COALESCE or IFNULL function to the parameter to avoid such situations:

//Better solution
String query = "SELECT COUNT(*) FROM EZ_DAY WHERE colx = COALESCE(?, colx)";

In the case above:

  • If the parameter has a different value than null, the query would be like this:

    String query = "SELECT COUNT(*) FROM EZ_DAY WHERE colx = ?";
    
  • If the parameter has null value, then the query would be like this:

    String query = "SELECT COUNT(*) FROM EZ_DAY WHERE colx = colx";
    

In the last example, you're still able to use PreparedStatement and avoid SQL injection attacks.

Related:

Community
  • 1
  • 1
Luiggi Mendoza
  • 85,076
  • 16
  • 154
  • 332
  • I am afraid you took the OP wrong. The OP wants to parametrize "COUNT(*)" so something different can be selected, and also parametrize which table to select from, if I am not completely mistaken. – chiccodoro Jun 17 '14 at 15:13
  • @chiccodoro there's no difference using `COUNT(1)` against `COUNT(colXxx)`, the result of `COUNT` is based on the `WHERE`. – Luiggi Mendoza Jun 17 '14 at 15:14
  • if he wants to leave it open whether "COUNT(*)" should be selected or maybe "col1, col2", then it is a difference. – chiccodoro Jun 17 '14 at 15:15
  • @chiccodoro that would need grouping, but it's about how you create your query. I'm not trying to give an Introduction to SQL class here, so I'm assuming OP and readers understand how to create queries. – Luiggi Mendoza Jun 17 '14 at 15:18
  • My only concern here is SQL injection, if I didn't make that clear. I only chose `COUNT (*)` because it was the only parameter (everything else is a table name or SQL keyword). This solution proposed by @LuiggiMendoza is likely what I will implement, as it still lets me use PreparedStatement. Although if you guys are telling me that the simple `select count (*)` query is cannot be subject to SQL injection, then it seems unnecessary. – DanGordon Jun 17 '14 at 15:20
0

As explained in this post, SQL injection can lead to very serious issues like:

  • call a sleep function so that all your database connections will be busy, therefore making your application unavailable
  • extracting sensitive data from the DB
  • bypassing the user authentication

And it's not just SQL that can be affected. Even JPQL can be compromised if you are not using bind parameters.

Bottom line, you should never use string concatenation when building SQL statements. Use a dedicated API for that purpose:

Vlad Mihalcea
  • 142,745
  • 71
  • 566
  • 911