I’m evaluating Sonarqube enterprise edition 8.1.0.31237, using dotnet-sonarscanner 4.8.0.
I’m scanning a .net core project on my local machine to which I’ve added a sql injection vulnerability. This is the code I’ve added:
[Route("api/GetNode")]
[HttpGet]
public string GetWithSqlInjection(string nodeId) {
using(var reader = db.ExecuteReader("select Label, Id from apphierarchy where id = '" + nodeId + "'")) {
if(reader.Read())
return reader.GetString(0);
}
return "Not Found";
}
The scan completes normally, but it’s not reporting any sql injection vulnerability. Is this normal? It seems like it should pick this up. This is the most basic sql injection I could do in .net.
I just set this server up, is there something I need to do to enable the security vulnerability scanning?
So I guess ExecuteReader is not considered by SonarQube to be a security risk. Is that right? Is there any way to extend the rule to make it apply to more methods?
The rule S2077 is a Security Hotspot. Its goal is to identify when a SQL query is built through String concatenations. The rule won’t detect in itself SQL Injection vulnerabilities but just the bad practice to do String concetenations.
If you want to detect SQL Injection vulnerabilities, you need to look at the rule S3649: Database queries should not be vulnerable to injection attacks. This one is based on taint analysis and track the data flow from user inputs to sinks where SQL queries are executed. S3649 supports Dapper ORM and will be improved in a near future thanks to MMF-1885.
@eric.therond For the code I posted, the type of db is SqlConnection.
@Alexandre_Gigleux, I looked at rule S3649 and I’ve copied the example noncompliant code into my project verbatim. Making sure I have a UsersContext to test with. Still no Security Hotspot is identified. I tried a couple of different ways of writing an injection, and the only one that was identified was this:
public void Foo(DbContext context, string query, string param)
{
string sensitiveQuery = string.Concat(query, param);
context.Database.ExecuteSqlCommand(sensitiveQuery); // Sensitive
context.Query<User>().FromSql(sensitiveQuery); // Sensitive
var value = 1;
context.Database.ExecuteSqlCommand($"SELECT * FROM mytable WHERE mycol={value}", param); // Sensitive, the FormattableString is evaluated and converted to RawSqlString
string query2 = $"SELECT * FROM mytable WHERE mycol={param}";
context.Database.ExecuteSqlCommand(query2); // Sensitive, the FormattableString has already been evaluated, it won't be converted to a parametrized query.
}
But this code was not identified as Security Hotspot:
public class SqlInjection : Controller
{
private readonly UsersContext _context;
public SqlInjection()
{
}
public SqlInjection(UsersContext context)
{
_context = context;
}
// GET /SqlInjection/Authenticate
public IActionResult Authenticate(string user)
{
var query = "SELECT * FROM Users WHERE Username = '" + user + "'"; // Unsafe
var userExists = _context.Users.FromSql(query).Any(); // Noncompliant
// An attacker can bypass authentication by setting user to this special value
user = "' or 1=1 or ''='";
return Content(userExists ? "success" : "fail");
}
}
which is taken directly from the example for S3649.
Also this code was not identified as a Security Hotspot either:
Is it possible for you to run your scan using debug mode (sonar.verbose=true) and provide the full output of the log (zip and attach). This will help me see all of your settings to help further.
Thank you for the additional information. I don’t see anything in your configuration that would exclude this.
Can you zip and post the full solution of the UserContext and SqlInjection you are using when you expect the issue to be detected? I am trying to recreate your issue but I fear that me filling in the missing code will lead to different results.
For the rule S3649, we discovered that the “Noncompliant Code Example” we are providing is NOT covered by the implementation of S3649. We cover 283 sinks for the SQL Injection rule but not this one
Microsoft.EntityFrameworkCore.RelationalQueryableExtensions.FromSql<TEntity>(System.Linq.IQueryable<TEntity>, FormattableString) is not considered as a sink, hence why there is no Vulnerability raised even when you copy/paste the Noncompliant Code.
