Dapper SqlInjection

[AlzaMartinHoly]

Hi, I read (here: SonarCloud Community: Major updates on security rules for Python, C# and Java) that SonarCloud should support C # code analysis against SqlInjection even using Dapper. I have the following code:

    public async Task<UserDto[]> FindAll(string param)
    {
        SqlCommand command;
        string sensitiveQuery = string.Format("INSERT INTO Users (name) VALUES (\"{0}\")", param);
        command = new SqlCommand(sensitiveQuery); // Correctly detected

        using (var uow = ConnectionProvider.Create())
        {
            var entities = await uow.QueryAsync<UserDto>(sensitiveQuery); // Not detected
            return entities.ToArray();
        }
    }

Sonar correctly identifies line 5 as dangerous, but does not mark line 9 as a potential risk.

Can you please advise me how to arrange the designation of line 9 as a potential risk?

Tested on https://sonarcloud.io/ and on SonarLint VS add-on (version 4.25.0.20544)

[Pierre-Loup_Tristant]

Hi @AlzaMartinHoly and welcome to the community forum.

Is it a public project you analyzed on SonarCloud? If yes, could you point me to this project?

If this is not a private project, could you please provide a reproducer that I could easily build? It would be easier, for example, if you provided the import and and the implementation of UserDto.

Thank you.

[AlzaMartinHoly]

Hi Pierre, thanks for your reply. I’m trying to create the smallest functional solution, but I’m currently in a situation where SonarLint does not consider any SQL Injection variant to be dangerous. I think I have something wrong with Visual Studio. But I am sending the functional solution here:

using Dapper;
using System;
using System.Data.SqlClient;
using System.Threading.Tasks;

namespace SonarTest
{
    public class Program
    {
        static void Main(string[] args)
        {    
            var res = FindAll("Martin Holy").Result;
        }

        public static async Task<UserDto> FindAll(string param)
        {
            string sensitiveQuery = "SELECT U.User_ID, U.FullName FROM Users AS U WHERE U.FullName IN ('" + param + "')";
            var command = new SqlCommand(sensitiveQuery);
            using var sqlConnection = new SqlConnection("Server=localhost;Database=myDataBase;Trusted_Connection=True;");
            command.Connection = sqlConnection;
            await sqlConnection.OpenAsync();
            var commandResult = command.BeginExecuteReader();
            using (SqlDataReader reader = command.EndExecuteReader(commandResult))
            {
                reader.Read();
                var commandUser = new UserDto() { User_ID = reader.GetInt32(0), FullName = reader.GetString(1) };
            }

            var dapperUser = await sqlConnection.QueryFirstOrDefaultAsync<UserDto>(sensitiveQuery);
            return dapperUser;
        }

        public class UserDto
        {
            public int User_ID { get; set; }
            public string FullName { get; set; }
        }
    }
}

[Pierre-Loup_Tristant]

Hi @AlzaMartinHoly,

Correct me if I m wrong but, in the first code sample you provided, this is the vulnerability rule that raised an issue:

Rules explorer

This rule is not enabled in SonarLint (see details here) so that’s probably why you see nothing in Visual Studio.

Thank you for your code sample I will have a look and then come back to you.

Best.

[Pierre-Loup_Tristant]

I had a look at your code sample and noticed this:

@AlzaMartinHoly

public class Program

Your program should inherit from Controller from Microsoft.AspNetCore.Mvc otherwise we will not consider param as a user controller value.

After changing this you should see 2 SQL injection detected in your source code.

[AlzaMartinHoly]

Thank you very much for your investigation. I appreciate that very much.

You were correct that I would love to see RSPEC-3649.

I understand. I did not expect the analyzer to be dependent on the source from which the value is taken. Our situation is that we use GraphQL for communication, not REST. GraphQL classes are registered in the Startup class of the project. Class do not inherits from any other class. For example, the code I sent above can be called like this:

namespace SonarTest
{
    public class GraphqlQuery 
    {
        public async Task<UserDto> GetUser([Inject] Query query, string input)
            => await query.FindAll(input);
    }
}

Do you have please solution for this situation too?

[Pierre-Loup_Tristant]

Sorry we don’t support yet user input coming from GraphQL query.

In the previous example I was advising you to add Controller because we support this API and we know that public methods from this receive user inputs as parameters.

It would be useful if you could point me to the GraphQL library you use on your project.
Also, based on you previous example, how do you define that GraphqlQuery class is a GraphQL entry point?

Thank you.

[AlzaMartinHoly]

I understand. GraphQL is not so popular as REST.

We’re using https://github.com/graphql-dotnet/graphql-dotnet library.
Unfortunatelly I cannot share our concrete implementation (we have all this functionality packed in our shared libraries in big and complex system) but the main point is:

using GraphQL.Conventions;

public class Startup
{
    public void ConfigureServices(IServiceCollection services)
    {
        var engine = GraphQLEngine.New();
        var schema = new SchemaDefinition<GraphqlQuery>();
        engine = engine.BuildSchema(schema.GetType());
        services.AddSingleton(engine);
    }
}

Thank you.