Findbugs not finding potential SQL injection vulnerability

Question

I just installed the FindBugs plugin for Eclipse, with the hope that it will help me find SQL injection vulnerabilities in my code. However, it doesn't seem to be finding anything, even when I deliberately put some in.

In the following examples, assume staticFinalBaseQuery is declared as follows:

public static final String staticFinalBaseQuery = "SELECT foo FROM table where id = '";

and assume userInputfilterString is an argument to the method wrapping the example snippets. It comes direct from user input, and is not sanitized.

For example, the following snippet will not trigger a warning:

String query = staticFinalBaseQuery + userInputfilterString;
pstmt = dbConnection.prepareStatement(query);

Where staticFinalBaseQuery is a static final string, and userInputfilterString is a string direct from user input, available only at runtime, not scrubbed at all. Clearly, this is a vulnerability.

I expect the "A prepared statement is generated from a nonconstant String" warning to be triggered.

The following snippet also does not cause a warning (not surprising, since the compiled forms of these are probably identical):

pstmt = dbConnection.prepareStatement(staticFinalBaseQuery + userInputfilterString);

However, this will cause a warning:

pstmt = dbConnection.prepareStatement(staticFinalBaseQuery + userInputfilterString + "'");

If I append an empty string, or a space, no warning is triggered.

So, my question is, how can I get FindBugs to trigger on my first example? I am also curious why the first doesn't cause a warning, but the last does?

Thanks in advance!

EDIT: I submitted a bug to FindBugs's bug tracking system, as it seems this might be a bug. However, if anyone has any tips, I'd love to hear them.

Answer 1

It is hard to distinguish between safe code and unsafe code here. Sure, userInputfilterString may be unsafe, but it is impossible to determine this at compile time. However, the single-quote character in a string concatenation is a tell-tale sign of using inject-able code. That's why FindBugs is triggering on the line containing this character, but not on the line with mere string concatenation.

Basically, this isn't a bug, but a limitation of how much can be done by software to check for SQL injection. Since the string may contain anything (i.e. it could have the vulnerable concatenation in another function) it is impossible to have the tool determine with any certainty that a problem exists.

Answer 2

I don't think PMD or Checkstyle will catch it either, but you might give them a try (I use all 3 on a regular basis, good tools to use).

EDIT: PMD was the correct link, but I called it findbugs... findbugs on the brain I guess...

Answer 3

Consider upgrading to commercial software such as http://www.ouncelabs.com/ which will serve your purpose much better...