Enterprise edition not picking up sql injection

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?

Edit

It looks like this is the rule for sql injections: https://rules.sonarsource.com/csharp/type/Security%20Hotspot/RSPEC-2077

and it looks like it only applies to these methods:

  • System.Data.SqlClient.SqlCommand.SqlCommand(string, ...)
  • System.Data.SqlClient.SqlDataAdapter.SqlDataAdapter(string, ...)
  • System.Data.Odbc.OdbcCommand.OdbcCommand(string, ...)
  • System.Data.Odbc.OdbcDataAdapter.OdbcDataAdapter(string, ...)
  • System.Data.SqlServerCe.SqlCeCommand.SqlCeCommand(string, ...)
  • System.Data.SqlServerCe.SqlCeDataAdapter.SqlCeDataAdapter(string, ...)
  • System.Data.OracleClient.OracleCommand.OracleCommand(string, ...)
  • System.Data.OracleClient.OracleDataAdapter.OracleDataAdapter(string, ...)
  • Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions.ExecuteSqlCommand(...)
  • Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions.ExecuteSqlCommandAsync(...)
  • Microsoft.EntityFrameworkCore.RelationalQueryableExtensions.FromSql<TEntity>(System.Linq.IQueryable<TEntity>, System.FormattableString)
  • System.Data.SqlClient.SqlCommand.CommandText.set
  • System.Data.Odbc.OdbcCommand.CommandText.set
  • System.Data.SqlServerCe.SqlCeCommand.CommandText.set
  • System.Data.OracleClient.OracleCommand.CommandText.set

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?

Hello @blake,
Can you please make sure that the security hotspots rules are active in your quality profile:


Alex.

Yes, I can see that. It even says it was used in the last 2 minutes after I run a scan.

Hello @blake

The only ExecuteReader method we support comes from Dapper ORM with this definition:

Dapper.SqlMapper.ExecuteReader(System.Data.IDbConnection, string, object, System.Data.IDbTransaction, int?, System.Data.CommandType?)

In your code, what is the class of db object?

Eric

Hello @blake,

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.

Regards
Alex

@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:

public void Bar( string name)
{
    SqlCommand command;
    string sensitiveQuery = string.Format("INSERT INTO Users (name) VALUES ('{0}')", name);
    command = new SqlCommand(sensitiveQuery); // Sensitive

    command.CommandText = sensitiveQuery; // Sensitive
    db.Open();
    command.Connection = db;
    command.ExecuteNonQuery();
}

Is this expected behavior?