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?

Yes it is very easy to do SQL Injection on your query above. 1 answer. Parameters parameters parameters. [Parameters](http://stackoverflow.com/questions/7505808/why-do-we-always-prefer-using-parameters-in-sql-statements) & this goes side by side this days [Using Add instead of AddWithValue](http://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/) — P. Pat, Mar 24 '17 at 07:39
Why using Odbc insted of Sql .Net driver (e.g. SqlConnection)? Use parametrized queries as @P.Pat and Damien_The_Unbeliever mention. Why don't you use some ORM? — Klaudiusz bryjamus, Mar 24 '17 at 07:45
But, how i use parameters inside "DataBaseConnection Class" ? i used this class get and set data all-over my program,through this class parse more than 150 different quary to the data base?how am i customize parameters according to that different quary? any one can help? @P.Pat — Gayan Chinthaka Dharmarathna, Apr 11 '17 at 03:06
@GayanChinthakaDharmarathna What I can suggest is that you learn stored procedures and see if you can implement it in your current DBMS. It also prevents sql injection. — P. Pat, Apr 11 '17 at 08:14

score 9 · Answer 1 · answered Mar 24 '17 at 07:40

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)

Wernfried Domscheit · Accepted Answer · 2017-04-25T08:24:38.750

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<>

But, how i use parameters inside "DataBaseConnection Class" ? i used this class get and set data all-over my program,through this class parse more than 150 different quary to the data base?how am i customize parameters according to that different quary? — Gayan Chinthaka Dharmarathna, Apr 11 '17 at 03:09
You can prepare an array (or `List(T)`, etc.) of Parameters outside and pass this array as an argument to DataBaseConnection constructor. Then allocate parameters in a loop. — Wernfried Domscheit, Apr 11 '17 at 07:15

score 1 · Answer 3 · answered Mar 24 '17 at 09:08

Damien_The_Unbeliever's answer is mostly good, but I would like to improve it/ version it.

You can also change the SetData and GetData methods and add them an array of parameters (although I do share his/her thoughts in getting rid of the class, you could make it abstract to make more specifica DAL classes).

The only requirement to avoid SQL Injection is using parameters, either using stored procedures (impossible due to question requirements) or queries written in your code.

There is a slight chance of getting into SQL Injection even if using parameters, if the SQL executed (in coded query or in the stored procedure) does use sp_execute_sql or similar. In case of using sp_execute_sql in your query, make sure to avoid writting it with user provided info. You can set parameters to the sp_execute_sql function as a second optional parameter.

bbsimonbb · Answer 4 · 2017-03-27T09:58:56.003

This is a recurring (for the last 20 years at least) question, but I earnestly believe I have a new answer... use QueryFirst. You get the advantages of stored procedures, but your SQL lives in .sql files in your application, versioned with your app. You create parameters just by referencing them in your SQL. All the parameter handling code is generated for you. You (and your team) have to use parameters because there's no other way. The possibility of doing something unsafe is removed. And there are tons of other advantages: you edit your sql in a real environment, with syntax validation and intellisense. Your queries are continually integration tested against your db, and their wrapper code regenerated. All errors are trapped as you type, or when you save a .sql, or, last resort, when you build. In theory, there are no runtime errors from data access.

Usually rigour comes at the cost of simplicity/ease-of-dev. This approach is much more rigorous and much easier to use than the traditional sql-in-string-literals approach. Coming soon, language and platform portability. Drag and drop a sql query from C# project on windows into a Node express app on linux or mac, rebuild, and you get a typescript wrapper instead of a C# one. That should get folk's attention.

disclaimer: I wrote QueryFirst. Download here. Little blog here.

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

4 Answers4