# Friday, 13 April 2012

SQL Injection  is one of the most frequently-exploited vulnerabilities in the software world. It refers to user-entered data making its way into commands sent to back-end systems. It is common because so many developers are unaware of the risk and how to mitigate it.

Most of the applications I work with read from and write to a relational database, such as Microsoft SQL Server.  I frequently run across ADO.NET code like the following:

string lastName = "'Adams'";
string sql = "Select * from dbo.Customer where LastName = '" + lastName + "'";
string connString = ConfigurationManager.ConnectionStrings["LocalConn"].ConnectionString;
using (var conn = new SqlConnection(connString))
{
    conn.Open();
    var cmd = conn.CreateCommand();
    cmd.CommandText = sql;
    SqlDataReader reader = cmd.ExecuteReader();
    while (reader.Read())
    {
        Console.WriteLine("Bad Name: {0} {1}", reader["FirstName"], reader["LastName"]);
    }
}

This code is designed to call a stored procedure like the following:

CREATE PROCEDURE [dbo].[GetCustomersByFirstName]
    @FirstName NVARCHAR(50)
AS
BEGIN
    -- SET NOCOUNT ON added to prevent extra result sets from
    SET NOCOUNT ON;

    SELECT 
            Id, 
            FirstName, 
            LastName
        FROM dbo.Customer
        WHERE FirstName = @FirstName
        ORDER BY Id
END

GO

This method of code has several disadvantages

  1. This code is not optimal because SQL Server does not have a chance to reuse a cached query plan unless the user happens to send the exact same text into SQL Server.
  2. The string concatenation opens the system to SQL Injection attacks.

A SQL Injection Attack is an attempt by an unscrupulous user to pass malicious commands to a database. In the above example, imagine that the variable x was provided by a user inputting text into a text box on a web age. An evil user might type something like

"Smith';DROP TABLE Customer;//"

If that code runs with sufficient permissions, it would wreak havoc on your database. The following query would be passed to SQL Server.
Select * from dbo.Customer where LastName = 'Smith';DROP Table Customer;//'

Clearly, dropping the customer table is not what your code is intended to do.

Many of you will read the above example and decide that you are safe because

  1. Your web code runs under a context with insufficient privileges to drop a table; and
  2. You are validating all user inputs to ensure a user cannot enter anything bad.

There are problems with this reasoning.

  1. A clever hacker can sometimes trick a user into running code under elevated privileges. Often there are multiple steps to an attack.
  2. Even if you have caught every possible injection possibility in your user interface, you cannot guarantee that every call to this API will be made only from your UI for all eternity. You may open up the API to the public or you may subcontract writing a mobile application that calls this API or you may hire a new programmer who doesn't know better.

The point is that you need to check security at every level of your application. And part of checking security is to not trust your inputs.

A far better approach than concatenating strings to form a SQL statement is to create parameter instances; set the value of each parameter; and add these parameters to a Parameters collection.

The code below shows how to do this.

string lastName = "Adams";
string sql = "Select * from dbo.Customer where LastName = @LastName";
string connString = ConfigurationManager.ConnectionStrings["LocalConn"].ConnectionString;
using (var conn = new SqlConnection(connString))
{
    conn.Open();
    var cmd = conn.CreateCommand();
    cmd.CommandText = sql;
    SqlParameter lnParam = cmd.CreateParameter();
    lnParam.ParameterName = "@LastName";
    lnParam.Value = lastName;
    cmd.Parameters.Add(lnParam);
    SqlDataReader reader = cmd.ExecuteReader();
    while (reader.Read())
    {
        Console.WriteLine("Good Name: {0} {1}", reader["FirstName"], reader["LastName"]);
    }
    Console.WriteLine();

Pass an unexpected parameter here and it will no t be executed on the end of the query because SQL Server is expecting a parameter for a specific use.

The same pattern works if I want to pass in a dynamic string of SQL. Passing Parameter instances is more secure than concatenating SQL and passing that string to SQL Server.

Below is a console application that uses the vulnerable string concatenation method to call SQL Server via ADO.NET

using System;
using System.Configuration;
using System.Data.SqlClient;

namespace PassingSql_WrongWay
{
    class Program
    {
        static void Main(string[] args)
        {
            CallSqlQuery();
            CallStoredProc();
            Console.ReadLine();
        }

        private static void CallSqlQuery()
        {
            string lastName = "'Adams'";
            //string lastName = "Adams';DROP TABLE dbo.ExtraTable;--";
            string sql = "Select * from dbo.Customer where LastName = '" + lastName + "'";
            string connString = ConfigurationManager.ConnectionStrings["LocalConn"].ConnectionString;
            using (var conn = new SqlConnection(connString))
            {
                conn.Open();
                var cmd = conn.CreateCommand();
                cmd.CommandText = sql;
                SqlDataReader reader = cmd.ExecuteReader();
                while (reader.Read())
                {
                    Console.WriteLine("Bad Name: {0} {1}", reader["FirstName"], reader["LastName"]);
                }
            }
            Console.WriteLine();
        }

        private static void CallStoredProc()
        {
            string firstName = "James";
            string sql = "EXEC GetCustomersByFirstName '" + firstName + "'";
            string connString = ConfigurationManager.ConnectionStrings["LocalConn"].ConnectionString;
            using (var conn = new SqlConnection(connString))
            {
                conn.Open();
                var cmd = conn.CreateCommand();
                cmd.CommandText = sql;
                SqlDataReader reader = cmd.ExecuteReader();
                while (reader.Read())
                {
                    Console.WriteLine("Bad Name: {0} {1}", reader["FirstName"], reader["LastName"]);
                }
                Console.WriteLine();
            }
        }
    }
}

Below is a similar console app, using the more secure parameters pattern

using System;
using System.Configuration;
using System.Data.SqlClient;

namespace PassingSql_RightWay
{
    class Program
    {
        static void Main(string[] args)
        {
            CallSqlQuery();
            CallStoredProc();
            Console.ReadLine();
        }

        private static void CallSqlQuery()
        {
            string lastName = "Adams";
            //string lastName = "Adams;DROP TABLE dbo.ExtraTable;--";
            string sql = "Select * from dbo.Customer where LastName = @LastName";
            string connString = ConfigurationManager.ConnectionStrings["LocalConn"].ConnectionString;
            using (var conn = new SqlConnection(connString))
            {
                conn.Open();
                var cmd = conn.CreateCommand();
                cmd.CommandText = sql;
                SqlParameter lnParam = cmd.CreateParameter();
                lnParam.ParameterName = "@LastName";
                lnParam.Value = lastName;
                cmd.Parameters.Add(lnParam);
                SqlDataReader reader = cmd.ExecuteReader();
                while (reader.Read())
                {
                    Console.WriteLine("Good Name: {0} {1}", reader["FirstName"], reader["LastName"]);
                }
                Console.WriteLine();
            }
        }

        private static void CallStoredProc()
        {
            string firstName = "James";
            string storedProcName = "GetCustomersByFirstName";
            string connString = ConfigurationManager.ConnectionStrings["LocalConn"].ConnectionString;
            using (var conn = new SqlConnection(connString))
            {
                conn.Open();
                var cmd = conn.CreateCommand();
                cmd.CommandText = storedProcName;
                cmd.CommandType = System.Data.CommandType.StoredProcedure;
                SqlParameter lnParam = cmd.CreateParameter();
                lnParam.ParameterName = "@FirstName";
                lnParam.Value = firstName;
                cmd.Parameters.Add(lnParam);
                SqlDataReader reader = cmd.ExecuteReader();
                while (reader.Read())
                {
                    Console.WriteLine("Good Name: {0} {1}", reader["FirstName"], reader["LastName"]);
                }
                Console.WriteLine();
            }
        }
    }
}

If you wish to use the above code, create a new database named TestData and run the following SQL DDL to create the database objects.

USE [TestData]
GO

/****** Object:  Table [dbo].[ExtraTable]    
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[ExtraTable](
    [foo] [nchar](10) NULL,
    [bar] [nchar](10) NULL
) ON [PRIMARY]
GO

/****** Object:  Table [dbo].[Customer]    
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[Customer](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [FirstName] [nvarchar](50) NULL,
    [LastName] [nvarchar](50) NOT NULL
) ON [PRIMARY]
GO

INSERT INTO dbo.Customer (FirstName, LastName) VALUES ('George', 'Washington') 
GO 
INSERT INTO dbo.Customer (FirstName, LastName) VALUES ('John', 'Adams') 
GO 
INSERT INTO dbo.Customer (FirstName, LastName) VALUES ('Thomas', 'Jefferson') 
GO 
INSERT INTO dbo.Customer (FirstName, LastName) VALUES ('James', 'Madison') 
GO 
INSERT INTO dbo.Customer (FirstName, LastName) VALUES ('James', 'Monroe') 
GO 
INSERT INTO dbo.Customer (FirstName, LastName) VALUES ('John Quincy', 'Adams') 
GO 

/****** Object:  StoredProcedure [dbo].[GetCustomersByFirstName]   
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE PROCEDURE [dbo].[GetCustomersByFirstName]
    @FirstName NVARCHAR(50)
AS
BEGIN
    SET NOCOUNT ON;

    SELECT 
            Id, 
            FirstName, 
            LastName
        FROM dbo.Customer
        WHERE FirstName = @FirstName
        ORDER BY Id
END
GO

With a little bit of thought and a few lines of code, you can significantly reduce the risk of SQL injection in your ADO.NET code.

.Net | C# | SQL Server