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.
