SQL Injection not detected when query is a result of joining function

[voitech (Wojciech)]

Hello, I am considerating using SonarQube in my company and would like to ensure something about false negatives.

I am using:
SonarQube CE server 8.8.0.42792
Gradle plugin id “org.sonarqube” version “3.0”

while scanning code to verify against SQL Injection, I noticed that scanner missed manually confirmed vulnerability. After closer look and few tests of it I noticed, that SonarQube doesn’t detect sql injection when query is a result of eg. joining function.

This will be properly flagged as security hotspot:

public void testSqlInjection_A(String input) {
    try (Connection conn = dataSource.getConnection()) {
	
        String query = "select * from table where id = ? OR x = '" + input + "'";
			
        PreparedStatement preparedStatement = conn.prepareStatement(query);
        preparedStatement.setString(1, input);
        ResultSet resultSet = preparedStatement.executeQuery();
    } catch (Exception ignored) {
    }
}

While this function will not be flagged:

public void testSqlInjection_B(String input) {
    try (Connection conn = dataSource.getConnection()) {
		
        List<String> queryParts = new ArrayList<>();
        queryParts.add("select * from table where id = ? OR x = '" + input + "'");
        String query = com.google.common.base.Joiner.on("").skipNulls().join(queryParts);
			
        PreparedStatement preparedStatement = conn.prepareStatement(query);
        preparedStatement.setString(1, input);
        ResultSet resultSet = preparedStatement.executeQuery();
    } catch (Exception ignored) {
    }
}

Is it possible at all for sonar to detect such vulnerabilities or the ammount of possibilities is just too great? I could imagine somebody using some EscapeSQL class instead of Joiner and that being flagged as false positive, but isn’t false positive better than false negative?

testSqlInjection_A was flagged by RSPEC-2077, but shouldn’t it be also detected by RSPEC-3649 ?

[eric.therond]

Hello @voitech
Welcome to the community

Thank you for this nice feedback

On my case:

• testSqlInjection_A is detected by both s2077 and s3649
• testSqlInjection_B is not detected at all

I created a first ticket to improve s3649 to raise on testSqlInjection_B.

The purpose of s2077 is a little bit different, as explained in the rule description, it doesn’t really search for SQL injections but just the most obvious complex queries (basic formatting and concatenation).

One of the reason why testSqlInjection_A and testSqlInjection_B are not detected by s3649 in your case is that the rule is likely not able to mark the input variable as untrusted, we can fix that but in your code I can’t guess where the input came from (url parameter? other?).

Eric

[eric.therond]

@voitech I didn’t look at your environment:

s3649 is an injection rule available from SonarQube Developer Edition
So it’s normal you don’t have any issues with SonarQube CE and s3649

Eric

[voitech (Wojciech)]

Oh, I missed that s3649 is available from Dev edition.

in your code I can’t guess where the input came from (url parameter? other?)

Here I made simple poc. The real one is quite entangled, but it’s a user input which was put into custom java object and then used in query construction. Query is constructed by multiple methods appending fragments one by one. I think that case is too complex to discuss it here.