How avoid SQL Injection without stored procedure In C# Desktop Application

Question

I am developing C# desktop appllication using MS SQL server database. I Keep different class as follow connect to database.

 using System.Data.Odbc;

class DataBaseConnection
    {
        private OdbcConnection conn1 = new OdbcConnection(@"FILEDSN=C:/OTPub/Ot.dsn;" + "Uid=sa;" + "Pwd=otdata@123;"); //"DSN=Ot_DataODBC;" + "Uid=sa;" +  "Pwd=otdata@123;"

        //insert,update,delete
        public int SetData(string query)
        {
            try
            {
                conn1.Open();
                OdbcCommand command = new OdbcCommand(query, conn1);
                int rs = command.ExecuteNonQuery();
                conn1.Close();
                return rs;
            }
            catch (Exception ex)
            {
                conn1.Close();
                throw ex;
            }
        }
        //select
        public System.Data.DataTable GetData(string sql)
        {
            try
            {
                conn1.Open();
                OdbcDataAdapter adpt = new OdbcDataAdapter(sql, conn1);
                DataTable dt = new DataTable();
                adpt.Fill(dt);
                conn1.Close();
                return dt;
            }
            catch (Exception ex)
            {
                conn1.Close();
                throw ex;
            }
        }

   }

in my reqierd place i make object to that DatabaseConnection class and call to get and set method as requirment.

as an example ----

 DataBaseConnection db = new DataBaseConnection();

 string SaveNewEmp = "INSERT INTO Employee  (Service_ID, Title, Name, Initials, ) VALUES ('" + servicenumber + "','" + title + "','" + fullname + "','" + initials + "')";
                    int returns = db.SetData(SaveNewEmp);

• am i allow to SQl injection from this method?
• how avoid sql injection without using stored procedure?

Answer 1

You avoid SQL Injection the same way as you would anywhere else - by keeping SQL code separate from data. You can't do that if you insist on having the interface be based on just passing in a string.

I'd get rid of your wrapper class (it's just obscuring things) and make use of Parameters to pass the data alongside your query.

---

(I'd also recommend that you just use using statements around the various database objects rather than your current manual efforts to ensure Close is called which is also slightly breaking good error handling by re-throwing exceptions)

(Also, I'd recommend using new OdbcConnection objects wherever you need them rather than trying to share a single one - you'll be thankful you've done this as soon as any notion of multi-threading enters your codebase, which is practically inevitable these days)

Answer 2

Most important technique is to used bind variables like this:

 string SaveNewEmp = 
   "INSERT INTO Employee  (Service_ID, Title, Name, Initials) VALUES (?, ?, ?, ?)";

command.Parameters.Add("@servicenumber", OdbcType.Int).Value = ...;
command.Parameters.Add("@title", OdbcType.VarChar).Value = ...;
command.Parameters.Add("@fullname ", OdbcType.VarChar).Value = ...;
command.Parameters.Add("@initials ", OdbcType.VarChar).Value = ...;

Usually this lead also into a performance gain and you don't have to take care about quoting, imagine the title would be It's your day - this would fail with your approach.

Update

Using a list of parameters is straight forward:

public int SetData(string query, OdbcParameterCollection parList)
{
   ...  
   OdbcCommand command = new OdbcCommand(query, conn1);
   OdbcCommand.Parameters.Add(parList);
}


var parList = new OdbcParameterCollection();
parList.Add("@servicenumber", OdbcType.Int);
parList.Add("@title", OdbcType.VarChar);
...
int ret = SetData(query, parList);

However, I did not test it perhaps you have to run

foreach ( OdbcParameter aPar in parList ) { 
    OdbcCommand.Parameters.Add(aPar);
}

Using List<>