CWE 89: SQL Injection

Flaw

CWE 89: SQL Injection flaws occur when you create a SQL statement by building a string that includes untrusted data, such as input from a web form, cookie, or URL query-string.

For example:

public class BankAccountController : Controller 
{
    ...
    public ActionResult GetAccountBalance(string accountID)
    {
        string accountBalanceQuery = $"SELECT accountNumber, balance FROM accounts WHERE accountID = {accountID}";
        try
        {
            using (SqlCommand command = new SqlCommand(accountBalanceQuery, _connection)
            {
                using (SqlDataReader reader = command.ExecuteReader()
                {
                    while (reader.Read()
                    {
                        Data.Add(new AccountBalance { AccountNumber = reader.GetString(0), Balance = reader.GetDecimal(1) });
                    }
                }
            }
        }
        catch (SqlException ex)
        {
            ...
        }
        return View(Data);
    }
    ...
}

Under normal operation, the user with ID 984 might be logged in, and go to the URL https://bankingwebsite/BankAccount/GetAccountBalance?accountID=984

This means that accountBalanceQuery would then be:

SELECT accountNumber, balance FROM accounts WHERE accountID = 984

This information is passed to the database, the accounts and balances for user 984 are returned, and rows are added to the page to display.

The attacker could change the parameter accountID in the URL to:

https://bankingwebsite/BankAccount/GetAccountBalance?accountID=984%20OR%201%3D1

Which would cause the accountID parameter to be interpreted as:

984 OR 1=1

And this results in accountBalanceQuery being:

SELECT accountNumber, balance FROM accounts WHERE accountID = 984 OR 1=1

When this query is passed to the database, it returns all the account numbers and balances it has stored, and rows are added to the page to display. The attacker now knows every user’s account numbers and balances.

Fix

You can fix this flaw by using the SqlCommand to create a Parameterized Query as below:

public class BankAccountController : Controller 
{
    ...
    public ActionResult GetAccountBalance(string accountID)
    {
        const string accountBalanceQuery = $"SELECT accountNumber, balance FROM accounts WHERE accountID = @accountID";
        try
        {
            using (SqlCommand command = new SqlCommand(accountBalanceQuery, _connection))
            {
                command.Parameters.Add(new SqlParameter("@accountID", accountID));
                using (SqlDataReader reader = command.ExecuteReader()
                {
                    while (reader.Read())
                    {
                        Data.Add(new AccountBalance { AccountNumber = reader.GetString(0), Balance = reader.GetDecimal(1) });
                    }
                }
            }
        }
        catch (SqlException ex)
        {
            ...
        }
        return View(Data);
    }
    ...
}

...
     public ActionResult GetAccountBalance(string accountID)
     {
-        string accountBalanceQuery = $"SELECT accountNumber, balance FROM accounts WHERE accountID = {accountID}";
+        const string accountBalanceQuery = $"SELECT accountNumber, balance FROM accounts WHERE accountID = @accountID";
         try
         {
             using (SqlCommand command = new SqlCommand(accountBalanceQuery, _connection))
             {
+                command.Parameters.Add(new SqlParameter("@accountID", accountID));
                 using (SqlDataReader reader = command.ExecuteReader()
                 {
                     while (reader.Read())

Note that the query string has become a constant text value and contains a placeholder for the user-supplied data, in this example it is the @accountID. The user-supplied data is added to the Parameters collection of the SqlCommand object. In this way, the flaw is fully remediated and not vulnerable to SQL injection any longer.

References

CWE ↪ OWASP ↪ WASC ↪