Building a dynamic query in C# (SQL Injection Attack)

Question

I have the following code

public DataSet GetProject(string projectID)
{
   DataSet dataTable = new DataSet(); 
   DataAccess dataAccess = new DataAccess();
   OracleCommand commandOb = new OracleCommand();
   strQuery.Append("select projectName, managerName");
   strQuery.Append("from project ");
   strQuery.Append("where projectID = '" + projectID + "'");

   cmd.CommandText = strQuery.ToString();
   dataTable = dataAccess.ExecuteDataAdapter(commandOb);

   return dataTable;
}

Is this an okay way to build a query and execute it? Would this be vulunerable to SQL injection attacks?

Is there a recommended approach when it comes to building queries dynamically. Any help would be appreciated.

`Stay away from Dynamic querys` try using Parameterized Query's instead and if you are not familiar with how to do that.. then use `string.Format(Select projectName,managerName from project where ProjectID={0}", projectID);` — MethodMan, Jul 24 '13 at 02:35
Use parameterized queries or better yet, call a stored procedure... — sgeddes, Jul 24 '13 at 02:38
It's not read what I said.. I said in case he doesn't know how. I am more than aware on how to write a parameterized query.. I am not going to do the OP's work for him — MethodMan, Jul 24 '13 at 02:39
@DJKRAZE -- fair enough, but I wouldn't recommend it - Cheers! — sgeddes, Jul 24 '13 at 02:40

score 3 · Answer 1 · answered Jul 24 '13 at 03:04

Building a query this way does make it vulnerable to SQL injection attacks unless you have manually escaped your input (I.E. Made it impossible for the value of 'projectID' to change the structure of the query by using database engine specific escape sequences). However, the recommended way to do this is using parameterized queries (sometimes called "Prepared Statements"). With parameterized queries, you simply define the structure of the query and then provide the input values separately as parameters, preventing the structure of your query from ever being changed via SQL injection.

Here is your example, changed to use parameterization:

public DataSet GetProject(string projectID)
{
   DataSet dataTable = new DataSet(); 
   DataAccess dataAccess = new DataAccess();
   OracleCommand commandOb = new OracleCommand();
   strQuery = @"select projectName, managerName
                  from project
                  where projectID = :ProjectID"

   cmd.CommandText = strQuery;
   cmd.Parameters.AddWithValue("ProjectID", projectID);
   dataTable = dataAccess.ExecuteDataAdapter(commandOb);

   return dataTable;
}

The parameter ':ProjectID' in the query will be replaced with the value given in the 'AddWithValue' method. No matter what value is in the 'projectID' variable, it will always be evaluated as part of the WHERE clause. Whereas, before, a value similar to ['; DELETE FROM project;--] could have undesirable affects by changing your query to read as follows:

select projectName, managerName
  from project
  where projectID = ''; DELETE FROM project;--'

Luke Hutton · Answer 2 · 2013-07-24T03:21:54.580

Yes, the query is vulnerable to sql injection. If projectID is internally retrieved in your application via some other query, it is less vulnerable since there is no direct user input.

However, if it was some user input that was: "=1' OR 'a'=a'", then all projects would be retrieved allowing access to data without proper authorization. Even worse, executing unintended commands can be made, such as deleting all records.

Nonetheless, it's best practice to use Parameterized Queries or bind parameters for better performance and security against injection. Parameterized Queries escape reserved characters but you still have to hand write out all the queries. Alternatives include using an ORM, among others.

See https://www.owasp.org/ for more info and a useful .net specific blog series on sql injection, and more: http://www.troyhunt.com/2010/05/owasp-top-10-for-net-developers-part-1.html

using (var cn = new OracleConnection(connString))
{
  var sql = "select projectName, managerName from project where projectID = :p1";
  using (var cmd = new OracleCommand(sql, cn))
  {
    cmd.BindByName = true; 
    cmd.Parameters.Add(new OracleParameter(":p1", OracleDbType.Varchar2, projectID,
                                            ParameterDirection.Input));
    using (var adapter = new OracleDataAdapter(cmd))
    {
      cn.Open();
      var dataSet = new DataSet();
      adapter.Fill(dataSet);
      return dataSet;
    }
  }
}

Note: this is with odp.net

Building a dynamic query in C# (SQL Injection Attack)

2 Answers2

Linked