FromRawSql detection in EF Core?

  • SonarQube 9.0.1.46107
  • I have SQ deployed and running for months. I was trying to show how SQ prevents a SQL Injection vulnerability by detecting bad code in a PR. SQ let me PR a ‘BobbyTables’ 101 exploit

Example - from a .NET 3.1 EF Core project -

public void ExecuteSQL(string sqlText)
{
    var query = _dbContext.Users.FromSqlRaw(sqlText);

Does SQ not support EF Core?

Hi,

Do you have a commercial edition of SonarQube?

 
Ann

Yes, Developer edition

1 Like

Additionally, using the Solar Lint extension, the examples are not being detected - Only warning is that the API is obsolete in .NET

Hi Bart,

In general these functions seem to be classified as sinks for SQL injections in SonarQube. It will only raise an issue though if it also detects user input (like a GET parameter) that ends up in these functions.
Is it possible for you to share your test project, so that I can have a look? Thanks!

Btw. SonarLint itself can not detect this taint-style vulnerabilities. Though, if SonarLint is connected to SonarQube and SonarQube finds them, you can also display them in SonarLint.

A DLL, a public method the above contents of the method. How is that NOT an injection???

If you are saying that it’s ok to have an injection in a public method as long as the analysis does not note that the public method is not in a rest call directly… I have some serious argument with that thinking.

I am not saying that it is okay to have an injection vulnerability in a public method, but unless an attacker controls part of the data in the sink there is strictly speaking no injection vulnerability. If we would simply say every parameter of a public function can be tainted and should raise an issue if it ends up in a sensitive sink the number of issues that are found would be too much to handle, and the number of false-positives would be very high. Take the ExecuteQuery function from your screenshot: the sqlText can be already escaped/sanitized/validated before it is passed to the function.

I agree that in the case of SQL it is never a good idea to concatenate SQL queries manually and even though it might be done in a secure manner, it is a bad idea nonetheless. We have a separate rule for this, S2077. This rule is detected (internally) by a different analyzer though, so the methods where it raises an issue are not exactly the same as for the taint-analysis.

For now, our taint-analysis to detect SQL injections and similar vulnerabilities is pretty much focused on web applications (not just rest calls though). This is something that will be extended in the future, for example we plan to better cover injection vulnerabilities in mobile applications soon. Other platforms will follow. Having a DLL/library mode that considers every parameter of public functions as tainted was also already discussed in that regard, though no decisions were made yet.