C# rule S3649 missing after upgrade to 7.2?

SonarQube
,
1

Hi,

After upgrading to 7.2 (from 7.1) I ran a recent build to see how the rules had changed. To my surprise several vulnerabilities had dissapeared. Looking into it it seems like some SQL Injection related issues was removed.

I looked at next.sonarqube.com to see if it was there. And it was, but under a different name:
“csharpsquid:S3649 - SQL queries should not be vulnerable to injection attacks”. But that rule doesn’t exist in my sonarqube server, even though you provided the latest C# plugin with the installation.

I see in the announcement and see “Security Analysis” and under that “SQL query injection”. And this seems to be a licensed feature.

So my question is, have you removed features from the community version of SonarQube and put the same into the paid version? And if so, why isn’t this mentioned in the upgrade notes? “Feature X is now only in the paid version”.

We can still use the product just fine, was just suprised that something changed that I hadn’t read about beforehand.

2

Hello Emil,

You are right: Prior to SonarQube 7.2, an implementation of rule S3649 to detect SQL Injections was provided freely and open-source through the Sonar C# plugin. That implementation was quite naive, as it was essentially forbidding any kind of string concatenation to construct the query. This lead to many false-positive complains from our users, such as in this StackOverflow question. In the Java plugin, we had 3 other similar “user-injection” rules, that we eventually had to disable by default in the “Sonar way” quality profile because of too many unhappy users. In C#, the SQL Injection rule was the only “user-injection” rule delivered, and it still was enabled by default in “Sonar way”.

At the same time, we wanted to deliver the improved version of the rule: The one that correctly tracks user-provided data (e.g. from web frameworks) all the way to the SQL query. That, as you can imagine, takes quite more work compared to simply detecting and forbidding any kind of string concatenation.

Then, we did not want to deliver two versions of the same rule at the same time: This will only confuse our users, and anyway we no longer believed in the value generated by the trivial implementation ourselves.

So, it would not be correct to simply say that we moved a free feature into a paid one. Behind the scene, there is a completely different implementation, and comparing the two would be like trying to compare oranges and bananas. If you need more convincing :slight_smile: on this, I invite you to play with the new rule and evaluate it on SonarCloud.io, where it is available for free for open-source projects.

Finally, answering your question about the upgrade notes: We only removed 1 rule out of 300+ in the Sonar C# plugin. We believe that the impact for most users is minimal, which is why we did not mention this point in the upgrade notes, where it would be impractical to list all minor changes. However, this is not a change we wanted to secretly hide. In fact, on the Sonar C# plugin, there is an explicit GitHub issue that was closed in version 7.2 to remove the rule.

I hope that this helps to give you a bit more context about what and why this rule S3649 changed. Please do let me know if you have any further question.

5 Likes