Security - SQL Injection squid:S2077 shortage

[gilbert...@googlemail.com]

Using Sonarqube 5.4

Squid:S2077 "Values to SQL commands should be sanitized" is activated,
but shows a strange behaviour, f.e.

Connection con = DriverManager.getConnection("");
Statement stmt = con.createStatement();
ResultSet rs = stmt.executeQuery(s);

doesn't raise an issue, whereas the following does raise an issue squid:S2077

Connection con = DriverManager.getConnection("");
Statement stmt = con.createStatement();
ResultSet rs = stmt.executeQuery(s + "");

Regards,
Gilbert

[Michael Gumowski]

Hello Gilbert,

Thank you for your feedback. Variables, when provided directly to the query, are not considered by the rule as we can't say something about it (maybe it has already been sanitized prior to the method call).

As soon as a string concatenation is done, however (your 2nd case), the string is modified, so we raise an issue. As you can notice, our implementation is pretty naive for the moment, as we don't read values from constants and literals. So we could think about adding the exception for this one but that seems a bit complex to handle with the approach we chose.

We plan to rework the rule when we will be able to efficiently track changes (requires symbolic execution and cross-procedural analysis). Once done, this cases should be correctly handled.

Anyway, I created the following JIRA ticket to keep the issue in mind: https://jira.sonarsource.com/browse/SONARJAVA-1710

Cheers,

Michael GUMOWSKI | SonarSource

Software Developer @ Language Team
http://sonarsource.com

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/cc2f391c-eeb8-4b05-80dd-3d41b55fb242%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[gilbert...@googlemail.com]

Hi Michael,

thanks for your honest response.
You're right, dealing with possible SQL injections is not that easy.
We've evaluated other tools, like f.e. Checkmarx CxSAST.
They are raising issues for every stmt.executeQuery(s) and also for other methods with a string parameter.
Finally you have hundreds of issues : "the method takes a string that might not be
sanitized" .. etc. that need to be checked manually, which is a PITA
Otherwise raising an issue only after string concatenation might be fuzzy.

//Gilbert