ADO.NET - Convert EXEC Statements To Avoid SQL Injection Attacks

By Robbe D. Morris

Printer Friendly Version

Robbe Morris
Robbe & Melisa Morris
For whatever reason, you've been calling your SQL Server stored procedures using the following syntax in ADO.NET:  exec CustOrderHist 'ALFKI' or exec CustOrderHist @CustomerID='ALFKI' and you suddenly realize that what you thought was code not vulnerable to SQL Injection attacks is, in fact, vulnerable.  My guess is that you have been running these queries through a generic method in your database access layer class that returns either a DataTable or a DataSet.
If so, then the following little code snippet can help you adjust your code very quickly to use the SqlHelper.cs of the Data Access Application Block for .NET v2 from Microsoft.  If you aren't familiar with the Data Access Application Block for .NET, it provides a less troublesome way to create a parameterized SqlCommand object for use in retrieving DataSets (you can modify the block to only return DataTables if you want).


What I've done here is give you a string parser that pulls the stored procedure name out of the string as well as turn the comma delimited list of stored procedure parameter values into an object[] of parameters suitable for use with SqlHelper.cs.  This enables you to modify your generic database access method to become compliant without going through every page, class, or assembly changing code and introducing the potential of new bugs.  Keep in mind, my little string parser does not handle batch processing (ex. exec CustOrderHist 'ALFKI';GetSomeData 12 ).  It would turn the second part of the batch into a parameter for the first one and most likely throw an error when the first statement is processed.
Clearly not rocket science but it is a good way to remove the immediate threat of SQL Injection attacks in your .NET code when calling stored procedures in this manner.  I've also included some of the source code down below from the .NET Framework itself to explain in more detail why you should use parameterized SqlCommand objects for performance reasons.
 
Procedure Parse
 
 private void ProcedureParse(string ProcedureQuery,ref string ProcedureName,ref object[] ProcedureParameters)
 {

   try
   {

     ProcedureQuery = ProcedureQuery.Trim();
                
     if (ProcedureQuery.Length < 1) { return; }
                
     ProcedureName = ProcedureQuery;
                
     int ProcNameEnd = ProcedureName.IndexOf(" ");
                
     if (ProcNameEnd < 0) { return; }
                
     string Prm = ProcedureName.Substring(ProcNameEnd,ProcedureName.Length - ProcNameEnd).Trim(); 
               
     ProcedureName = ProcedureName.Substring(0,ProcNameEnd).Trim();
                
     ProcedureParameters = Prm.Replace("'","").Split(",".ToCharArray());   

   }
   catch (Exception e)  { throw e;}
   return;
 }
 
  public DataSet GetDataSet(string Query,string ConnectionString)
  {
 
    DataSet oDataSet = new DataSet();
    string ProcedureName="";
    object[] ProcedureParameters = null;

    // sample of old EXEC query:   query="exec CustOrderHist 'ALFKI'  ";
		 
    try
    {   
      ProcedureParse(Query,ref ProcedureName,ref ProcedureParameters);
      oDataSet = SqlHelper.ExecuteDataset(ConnectionString,ProcedureName,ProcedureParameters); 
    }
    catch (Exception e) { throw e; }
    return oDataSet;
 }
 
I'm a somewhat stubborn developer when it comes to all these best practicies claims.  There is so much misinformation out there as well as information that is out of date with the current release of the .NET Framework.  For me, I need to see it to believe it.  Rather than just spout gospel from MSDN or from the SQL Server help files, I'll show you the source code of the .NET Framework to explain what it took to convince me that it is better from a performance perspective to use parameterized SqlCommand objects instead of passing strings either in the form of SQL statements or EXEC stored procedures.  The following .NET Framework source code was extracted from the assemblies using Lutz Roeder's Reflector .NET.
You will find that no matter whether you use a SqlDataAdapter or SqlCommand object, it all eventually goes through the System.Data.SqlClient.SqlCommand.ExecuteReader() method.  Strings sent through the SqlDataAdapter get converted to SqlCommand objects with their CommandType set to .Text and sent through the .ExecuteReader() method anyway.
Notice in the .ExecuteReader method below what happens if the CommandType = CommandType.Text.  Rather than process the command through the preferred Remote Procedure Call (RPC) to SQL Server, the command is processed through the SqlClient.TdsParser.TdsExecuteSQLBatch method after the string is converted to a byte array in .NET.  However, .NET processes a parameterized SqlCommand object through the SqlClient.TdsParser.TdsExecuteRPC method instead.
The performance impact occurs largely on the database server and not so much on the client (webserver/pc).  Thus, if you've tried running simple performance tests on your development PC, you probably won't see much difference.  In fact, you could make an arguement that ADO.NET has to do alot more to process a parameterized SqlCommand object versus just passing the string and the .NET source code below reflects that - to some extent.
The biggest impact occurs on the database server in parsing the text of an insert, update, or complex sql statement.  The process of parsing this statement: exec MyProcName @MyVariable='blah' isn't very resource intensive but does force SQL Server to take extra steps in processing the request that it wouldn't have to take if the process went through RPC.
Will this make a huge difference in the performance of your application?  For small to medium usage applications, it won't make much of a noticable difference.  But, keep in mind as to how many applications will be processing data against your SQL Server database server.  Over time, the negative impact will grow.  So, to squeeze out that extra bit of performance and protect yourself from SQL Injection attacks, you'll probably want to adjust your code to use parameterized SqlCommand objects.
One last thing that I'd like you to take away from this little exercise.  You should get into the habit of using Lutz's Reflector tool to determine what your code is "really" doing under the hood before making a decision as to whether your current coding practices are good, bad, or whether they are bad enough to warrant rewriting some of your code.  Just because a book on .NET says something is possible or even a good idea doesn't make it so.  Read the .NET source code yourself before making that decision.
 
P.S.
You may also run across some information stating that SQL Server recompiles the stored procedure every time if it is called through the TdsExecuteSQLBatch methodology instead of using the cached copy.  I studied this pretty thoroughly across several databases and corresponding stored procedures and rarely found instances where SQL Server recompiled the procedure unnecessarily.
 
System.Data.SqlClient.SqlDataAdapter

 public SqlDataAdapter(string selectCommandText, SqlConnection selectConnection)
 {
      SqlCommand command1;
      GC.SuppressFinalize(this);
      this.SelectCommand = command1 = new SqlCommand(selectCommandText, selectConnection);
      this.internalCmdSelect = command1;
 }
 
System.Data.SqlClient.SqlCommand.ExecuteReader

 internal SqlDataReader ExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, bool returnStream)
 {
      SqlDataReader reader1 = null;
      this._rowsAffected = -1;
      bool flag1 = CommandBehavior.Default != (cmdBehavior & CommandBehavior.SchemaOnly);
      if ((CommandBehavior.SingleRow & cmdBehavior) != CommandBehavior.Default)
      {
            cmdBehavior |= CommandBehavior.SingleResult;
      }
      this.ValidateCommand("ExecuteReader", true);
      string text1 = null;
      try
      {
            if ((CommandType.Text == this.CommandType) && (this.GetParameterCount() == 0))
            {
                  string text2 = this.GetCommandText(cmdBehavior) + this.GetSetOptionsOFF(cmdBehavior);
                  this._activeConnection.Parser.TdsExecuteSQLBatch(text2, this.CommandTimeout);
            }
            else
            {
                  _SqlRPC lrpc1;
                  if (CommandType.Text == this.CommandType)
                  {
                        if (this.IsDirty)
                        {
                              this.Unprepare();
                              this.IsDirty = false;
                        }
                        if (this._execType == 1)
                        {
                              lrpc1 = this.BuildExecute();
                        }
                        else if (this._execType == 2)
                        {
                              lrpc1 = this.BuildPrepExec(cmdBehavior);
                              this._execType = 1;
                              this._inPrepare = true;
                        }
                        else
                        {
                              lrpc1 = this.BuildExecuteSql(cmdBehavior);
                        }
                        if (this._activeConnection.IsShiloh)
                        {
                              lrpc1.options = 2;
                        }
                        this._activeConnection.Parser.TdsExecuteRPC(lrpc1, this.CommandTimeout, flag1);
                  }
                  else
                  {
                        lrpc1 = this.BuildRPC();
                        text1 = this.GetSetOptionsON(cmdBehavior);
                        if (text1 != null)
                        {
                              this._activeConnection.Parser.TdsExecuteSQLBatch(text1, this.CommandTimeout);
                              this._activeConnection.Parser.Run(RunBehavior.UntilDone, this, null);
                              text1 = this.GetSetOptionsOFF(cmdBehavior);
                        }
                        this._activeConnection.CheckSQLDebug();
                        this._activeConnection.Parser.TdsExecuteRPC(lrpc1, this.CommandTimeout, flag1);
                  }
            }
            if (returnStream)
            {
                  reader1 = new SqlDataReader(this);
            }
            if (runBehavior == RunBehavior.UntilDone)
            {
                  this._activeConnection.Parser.Run(RunBehavior.UntilDone, this, reader1);
            }
            if (reader1 == null)
            {
                  return reader1;
            }
            reader1.Bind(this._activeConnection.Parser);
            reader1.Behavior = cmdBehavior;
            reader1.SetOptionsOFF = text1;
            this._activeConnection.Reader = reader1;
            try
            {
                  this._cachedMetaData = reader1.MetaData;
                  return reader1;
            }
            catch (Exception exception1)
            {
                  reader1.Close();
                  throw exception1;
            }
      }
      catch (Exception exception2)
      {
            SQL.IncrementFailedCommandCount();
            throw exception2;
      }
      return reader1;
}

System.Data.SqlClient.TdsParser.TdsExecuteRPC

internal void TdsExecuteRPC(_SqlRPC rec, int timeout, bool inSchema)
{
      this.timeRemaining = new TimeSpan(0, 0, timeout);
      this.timeout = timeout;
      this.msgType = 3;
      this.WriteShort(rec.rpcName.Length);
      this.WriteString(rec.rpcName);
      this.WriteShort((short) rec.options);
      SqlParameter[] parameterArray1 = rec.parameters;
      try
      {
            for (int num1 = 0; num1 < parameterArray1.Length; num1++)
            {
                  SqlParameter parameter1 = parameterArray1[num1];
                  parameter1.Validate();
                  SqlDbType type1 = parameter1.SqlDbType;
                  if (parameter1.Direction == ParameterDirection.Output)
                  {
                        parameter1.Value = null;
                  }
                  bool flag1 = false;
                  bool flag2 = false;
                  if ((parameter1.Value == null) || Convert.IsDBNull(parameter1.Value))
                  {
                        flag1 = true;
                  }
                  if (parameter1.Value is INullable)
                  {
                        flag2 = true;
                        if (((INullable) parameter1.Value).IsNull)
                        {
                              flag1 = true;
                        }
                  }
                  if ((parameter1.ParameterName != null) && (parameter1.ParameterName.Length > 0))
                  {
                        this.WriteByte((byte) (parameter1.ParameterName.Length & 0xff));
                        this.WriteString(parameter1.ParameterName);
                  }
                  else
                  {
                        this.WriteByte(0);
                  }
                  byte num2 = 0;
                  object obj1 = parameter1.Value;
                  if ((parameter1.Direction == ParameterDirection.InputOutput) || (parameter1.Direction == ParameterDirection.Output))
                  {
                        num2 = 1;
                  }
                  if (((parameter1.Direction != ParameterDirection.Output) && (obj1 == null)) && !inSchema)
                  {
                        num2 = (byte) (num2 | 2);
                  }
                  this.WriteByte(num2);
                  MetaType type2 = parameter1.GetMetaType();
                  this.WriteByte(type2.NullableType);
                  if (type2.TDSType == 0x62)
                  {
                        this.WriteSqlVariantValue(obj1, parameter1.ActualSize, parameter1.Offset);
                  }
                  else
                  {
                        if (!flag1)
                        {
                              obj1 = parameter1.CoercedValue;
                        }
                        int num3 = MetaType.IsSizeInCharacters(parameter1.SqlDbType) ? (parameter1.Size * 2) : parameter1.Size;
                        int num4 = parameter1.ActualSize;
                        int num5 = 0;
                        if (MetaType.IsAnsiType(parameter1.SqlDbType))
                        {
                              if (!flag1)
                              {
                                    string text1 = flag2 ? ((SqlString) obj1).Value : ((string) obj1);
                                    num5 = this.GetEncodingCharLength(text1, num4, parameter1.Offset, this.defaultEncoding);
                              }
                              this.WriteParameterVarLen(type2, (num3 > num5) ? num3 : num5, false);
                        }
                        else if (parameter1.SqlDbType == SqlDbType.Timestamp)
                        {
                              this.WriteParameterVarLen(type2, 8, false);
                        }
                        else
                        {
                              this.WriteParameterVarLen(type2, (num3 > num4) ? num3 : num4, false);
                        }
                        if (type1 == SqlDbType.Decimal)
                        {
                              if (!flag1)
                              {
                                    if (flag2)
                                    {
                                          obj1 = this.AdjustSqlDecimalScale((SqlDecimal) obj1, parameter1.Scale);
                                          if (parameter1.Precision != 0)
                                          {
                                                SqlDecimal num8 = (SqlDecimal) obj1;
                                                if (parameter1.Precision < num8.Precision)
                                                {
                                                      throw SQL.ParameterValueOutOfRange(obj1.ToString());
                                                }
                                          }
                                    }
                                    else
                                    {
                                          SqlDecimal num6;
                                          obj1 = this.AdjustDecimalScale((decimal) obj1, parameter1.Scale);
                                          num6 = new SqlDecimal((decimal) obj1);
                                          if ((parameter1.Precision != 0) && (parameter1.Precision < num6.Precision))
                                          {
                                                throw SQL.ParameterValueOutOfRange(obj1.ToString());
                                          }
                                    }
                              }
                              if (parameter1.Precision == 0)
                              {
                                    this.WriteByte(0x1c);
                              }
                              else
                              {
                                    this.WriteByte(parameter1.Precision);
                              }
                              this.WriteByte(parameter1.Scale);
                        }
                        if (this.isShiloh && MetaType.IsCharType(type1))
                        {
                              SqlCollation collation1 = (parameter1.Collation != null) ? parameter1.Collation : this.defaultCollation;
                              this.WriteUnsignedInt(collation1.info);
                              this.WriteByte(collation1.sortId);
                        }
                        if (num5 == 0)
                        {
                              this.WriteParameterVarLen(type2, num4, flag1);
                        }
                        else
                        {
                              this.WriteParameterVarLen(type2, num5, flag1);
                        }
                        if (!flag1)
                        {
                              if (flag2)
                              {
                                    this.WriteSqlValue(obj1, type2, num4, parameter1.Offset);
                              }
                              else
                              {
                                    this.WriteValue(obj1, type2, num4, parameter1.Offset);
                              }
                        }
                  }
            }
      }
      catch
      {
            int num7 = this.packetNumber;
            this.ResetBuffer();
            this.packetNumber = 1;
            if ((num7 != 1) && (this.state == TdsParserState.OpenLoggedIn))
            {
                  this.SendAttention();
                  this.ProcessAttention();
            }
            throw;
      }
      this.FlushBuffer(1);
      this.pendingData = true;
}
 
System.Data.SqlClient.TdsParser.TdsExecuteSQLBatch

 internal void TdsExecuteSQLBatch(string text, int timeout)
{
      this.timeRemaining = new TimeSpan(0, 0, timeout);
      this.msgType = 1;
      this.timeout = timeout;
      try
      {
            this.WriteString(text);
      }
      catch
      {
            this.ResetBuffer();
            this.packetNumber = 1;
            throw;
      }
      this.FlushBuffer(1);
      this.pendingData = true;
}


Robbe has been a Microsoft MVP in C# since 2004.  He is also the co-founder of NullSkull.com which provides .NET articles, book reviews, software reviews, and software download and purchase advice.