SQL Injection: Use Parameters

The biggest cause of SQL injection is developers not using parameterised statements or stored procedures (which are usually parameterised). For example in c# code it is possible to call SQL commands using the SqlCommand object like below:

string selectString = @”select MyColumn from Mytable where MyOtherColumn = ” + value;
SqlCommand cmd = new SqlCommand();
cmd.CommandText = selectString;
cmd.Connection = conn;
cmd.ExecuteReader();

In reality the code should look more below with using statements.

string selectString = @”select MyColumn from Mytable where MyOtherColumn = ” + value;
using (var command = new SqlCommand(selectString, conn)) {

conn.Open();
using (SqlDataReader rdr = cmd.ExecuteReader()) {

while (rdr.Read())
{
///do something
}

}
conn.Close();

}

The examples above is prone to injection. If value is supplied as “1 union select MyColumn1 from DifferentTable” you may find other data such as your usernames or similar displayed in a list on the screen. The code above when appends value to the selectString and the final string is

select MyColumn from Mytable where MyOtherColumn = 1 union select MyColumn1 from DifferentTable

There are approaches around blacklisting words, escaping quotes & so on to constrain the input and prevent the query being built up. These approaches have generally proven ineffective as someone works out a way to by pass the controls or even in some cases to pervert the check itself to actually introduce a problem. It is generally agreed trying to blacklist rarely works very well. A better approach is to whitelist, however that may not be practical if your input allows larger amounts of free text.

Instead you should be using parameterised statements or stored procedures. First lets look at stored procedures which (in my opinion) is the best approach, others of course will disagree.

What you should ideally be seeing in the code is a call to a stored procedure like below. The use of the parameters escapes the parameter values and therefore prevents injection attacks. In this case @MyOtherColumn is passed as a parameter.

SqlCommand cmd = new SqlCommand(“MyStorePro”, conn);
cmd.CommandType = CommandType.StoredProcedure;
cmd.Parameters.Add(new SqlParameter(“@MyOtherColumn”, value));
using (SqlDataReader rdr = cmd.ExecuteReader()) {

while (rdr.Read())

{
///do something
}

}

The stored procedure it would call is shown below

CREATE PROCEDURE [dbo].[MyStorePro]
(
 @MyOtherColumn int
)
AS
BEGIN
 SET NOCOUNT ON
 DECLARE @Err int

 select MyColumn from Mytable where MyOtherColumn = @MyOtherColumn

 SET @Err = @@Error
 RETURN @Err
END

First of all the parameter is passed as an int preventing a string such as “1 union select MyColumn1 from DifferentTable” being passed at all.

If the parameter and MyOtherColumn was a varchar(200) instead and the string “null union select MyColumn1 from DifferentTable” was passed. The final command operated by sql would look like

select MyColumn from Mytable where MyOtherColumn = ‘null union select MyColumn1 from DifferentTable’ <- Note the single quote

The quotes mean MyOtherColumn must equal  ‘null union select MyColumn1 from DifferentTable’ this will return zero rows.

Another approach, which in my opinion is slightly less safe (as a developer is more likely to make a mistake) is a parameterised command. Note instead of appending the text we use a parameter @MyValue and then pass the parameter in. This again will wrap the input in quotes and prevent the union

string selectString = @”select MyColumn from Mytable where MyOtherColumn = @MyValue”;
using (SqlCommand cmd = new SqlCommand(selectString, conn))
{

     SqlParameter param = new SqlParameter();
    param.ParameterName = “@MyValue”;
    param.Value = inputValue;

    cmd.Parameters.Add(param);

    using (reader = cmd.ExecuteReader()){
      while(reader.Read())
      {
      ///do something
      }
    }
}

 

For future development it is probably best to use stored procedures. Note the use of a stored procedure will normally improve query plan reuse and it is usually easier to tune a stored procedure than sql in code.

Obviously some developers prefer the SQL statements in code. The important point is consistency. Agree a common way to do it and then make sure everyone understands the correct\safe way to implement the approach.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s