Press "Enter" to skip to content

Tag: security

Common C# Vulnerabilities – SQL Injection

If you include user-entered values as parameters in your SQL queries, you have a security problem if you use string concatenation, like this:

string query1 = "SELECT count(*) FROM [User] WHERE [Account] = '" + account + "' AND [Password] = '" + password + "'";

This problem also exists if you use String.Format.

string query2 = String.Format("SELECT count(*) FROM [User] WHERE [Account] = '{0}' AND [Password] = '{1}'", account, password);

Code like this will let hackers run any SQL they want. Which means they can login to your website without knowing the password, or, possibly, even destroy the data in your database.

XKCD example of a SQL injection problem
XKCD example of a SQL injection problem

 

The Problem

Someone familiar with SQL can enter values into your application that will cause your SQL statement to work the way they want – which may not be the way you planned.

You might use the querystrings above to check if a user entered a valid account name and password. If the user entered valid values for an existing account, the query would return “1”. If the values didn’t match a row in the User table, it would return “0”.

For this example, we’ll have one record in the User table. The Account value is “JoeUser”, and the Password value is “Password123”.

Screenshot of the User table, with one row

And here is some insecure code someone might use to check if a user’s login information is valid:

public bool LoginIsValid_INSECURE_EXAMPLE(string account, string password)
{
    using(SqlConnection connection = newSqlConnection(CONNECTION_STRING))
    {
        using(SqlCommand command = connection.CreateCommand())
        {
            command.CommandType = CommandType.Text;
            command.CommandText = "SELECT COUNT(*) FROM [User] WHERE [Account] = '" + account + "' AND [Password] = '" + password + "'";
            connection.Open();
            int count = (int)command.ExecuteScalar();
            if(count > 0)
            {
                return true;
            }
        }
    }
    return false;
}

Expected results with valid password

If the user entered an account of “JoeUser” and a Password of “Password123”, the querystring created would be this:

SELECT COUNT(*) FROM [User] WHERE [Account] = ‘JoeUser’ AND [Password] = ‘Password123’

When you run that query, the result is “1”.

Expected results with bad password

If someone entered “JoeUser” for the account and “BadPassword” for the password, the querystring would be:

SELECT COUNT(*) FROM [User] WHERE [Account] = ‘JoeUser’ AND [Password] = ‘BadPassword’

And the results from running the query would be “0”.

Unexpected results with SQL injection

However, if someone wants to hack your application, they may enter an account value of “JoeUser” and a password value of “‘ OR 1 = 1 –“. Notice that the first character for the password is a single quote.

This would create a querystring of:

SELECT COUNT(*) FROM [User] WHERE [Account] = ‘JoeUser’ AND [Password] = ” OR 1 = 1 –‘

And the result from this query is “1”, even though the password was not correct.

Starting the password value with a single quote closes the opening single quote that normally wraps around the password value.

Then, the user “injected” their own SQL condition “OR 1 = 1”, which will always be true. The two hyphens at the end tell the database server to ignore anything after them, which would be the original closing single quote.

If you wrote the rest of the code to treat “0” as a bad account/password combination, and anything greater than 0 as a valid combination, then you have a security hole in your application.

The Solution

Fortunately, there is a simple solution.

Instead of using string concatenation to add user-entered values into your querystring, use SqlParameter objects.

Here’s how the method would look using SqlParameter objects to prevent SQL injection attacks.

public bool LoginIsValid_SECURE_EXAMPLE(string account, string password)
{
    using(SqlConnection connection = newSqlConnection(CONNECTION_STRING))
    {
        using(SqlCommand command = connection.CreateCommand())
        {
            command.CommandType = CommandType.Text;
            command.CommandText = "SELECT COUNT(*) FROM [User] WHERE [Account] = @Account AND [Password] = @Password";
            command.Parameters.Add(newSqlParameter("@Account", account));
            command.Parameters.Add(newSqlParameter("@Password", password));
            connection.Open();
            int count = (int)command.ExecuteScalar();
            if(count == 1)
            {
                return true;
            }
        }
    }
    return false;
}

Changes from the insecure method

When setting the CommandText string of the SqlCommand object, use “@” and a parameter name as placeholders for where to place the user-entered values.

Next, we add SqlParameter values to the SqlCommand object’s Parameters list. Each SqlParameter value contains the parameter name and the user-entered value to put into the query.

When the query runs, the user-entered values are automatically used in the querystring, but in a safe way, that prevents SQL injection attacks.

Additional improvement to the login checking code

You may have noticed that I also changed the check of the count variable to only return true if the method returns exactly “1”, and not any number greater than 0.

Something you might want to add is a check to see if the “count” variable was larger than 1. If it was, have your program write a message to its log, with the account and password values used. Assuming the application and database are set up so each user has a unique account name; this would let you know there is a problem – either with your code, the data in the database, or with someone trying to use a SQL injection attack on your site.

Summary

Never trust user-entered data.

Besides users who mistype values, or format their input in a different way (for example, MM/DD/YYYY, when you expect DD/MM/YYYY), there are people who will try common techniques to hack into your site.

By always using SqlParameter objects to add user-entered values into your queries, instead of string concatenation, you protect yourself against these SQL injection attacks.

2 Comments