Question

I have written a CLR stored procedure in C# like this

[Microsoft.SqlServer.Server.SqlProcedure]
public static void IsUserNameExists(string strUserName, out SqlBoolean returnValue)
{      
    using (SqlConnection connection = new SqlConnection("context connection=true"))
    {
        connection.Open();
        SqlCommand command = new SqlCommand("Select count(UserName) from [User] where UserName='" + strUserName + "'", connection);

        int nHowMany = int.Parse(command.ExecuteScalar().ToString());

        if (nHowMany > 0)
            returnValue = true;
        else
            returnValue = false;
    }
}

Is it vulnerable to SQL injection? I am using SqlParameter. Any best practises?

Was it helpful?

Solution 3

CLR Stored procedure doesn’t prevent this by default. You need to do this yourself since CLR doesn’t do this automatically (I guess this was the actual questions you wanted to know)

Just update your code like this and you should be all good.

 [Microsoft.SqlServer.Server.SqlProcedure]
    public static void IsUserNameExists(string strUserName, out SqlBoolean returnValue)
    {
        using (SqlConnection connection = new SqlConnection("context connection=true"))
        {
            connection.Open();
            SqlCommand command = new SqlCommand("Select count(UserName) from [User] where UserName=@UserName", connection);
            command.Parameters.Add(new SqlParameter("@UserName", strUserName));

            int nHowMany = int.Parse(command.ExecuteScalar().ToString());

            if (nHowMany > 0)
                returnValue = true;
            else
                returnValue = false;
        }
    }

OTHER TIPS

The only correct way to prevent sql injection should be using parameterized queries. What you are doing is not safe, since you are concatenating strings.

Look into this here for reference How do parameterized queries help against SQL injection?

For clearification, why your code is vulnerable:
In terms of SQLParameter even something like '); DROP TABLE YourTable;-- will be a valid input (since it is a string). This will then be used by you to create the inner query and there's your SQL-Injection.

Is it vulnerable to SQL injection?

It is:

SomeType.IsUserNameExists("'; insert into [User](UserName) values ('Malefactor_Username'); select '1", out returnValue);

Any best practises?

Always use parametrized queries.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top