Parameterized Queries Against SQL Injections

Question

I am new to Asp.net and I am just starting to work with classes. I recently created a class that will handle most of my SQL queries for me, so I don’t need to repeatedly create new connections across all my files.

One of the methods I created takes an SQL query as a parameter and returns the result. I know that I should use parameterized queries to avoid SQL injections. My question is: how can I do this when I pass the request as a string parameter?

For example, here is the method that I will call:

public static DataTable SqlDataTable(string sql) { using (SqlConnection conn = new SqlConnection(DatabaseConnectionString)) { SqlCommand cmd = new SqlCommand(sql, conn); cmd.Connection.Open(); DataTable TempTable = new DataTable(); TempTable.Load(cmd.ExecuteReader()); return TempTable; } } 

So, from another file, I would like to use this method as follows:

 DataTable dt = new DataTable(); dt = SqlComm.SqlDataTable("SELECT * FROM Users WHERE UserName='" + login.Text + "' and Password='" + password.Text + "'"); if (dt.Rows.Count > 0) { // do something if the query returns rows } 

It works, but will still be vulnerable to injection? Is there a way to pass variables into a string as parameters? I know that I can do this if I create a new SQLCommand object for the query and use Parameters.AddWithValue, but I wanted all my SQL commands to be in a separate class.

Answer 1

It works, but will still be vulnerable to injection?

Yes, your code is terribly vulnerable to SQL injection.

I know that I should use parameterized queries to avoid SQL injections.

Oh of course yes.

My question is: how to do this when I pass the request as a string parameter?

You simply should not pass the request as a string parameter. Instead, you should pass the request as a string parameter containing placeholders and values ​​for these placeholders:

 public static DataTable SqlDataTable(string sql, IDictionary<string, object> values) { using (SqlConnection conn = new SqlConnection(DatabaseConnectionString)) using (SqlCommand cmd = conn.CreateCommand()) { conn.Open(); cmd.CommandText = sql; foreach (KeyValuePair<string, object> item in values) { cmd.Parameters.AddWithValue("@" + item.Key, item.Value); } DataTable table = new DataTable(); using (var reader = cmd.ExecuteReader()) { table.Load(reader); return table; } } } 

and then use your function as follows:

 DataTable dt = SqlComm.SqlDataTable( "SELECT * FROM Users WHERE UserName = @UserName AND Password = @Password", new Dictionary<string, object> { { "UserName", login.Text }, { "Password", password.Text }, } ); if (dt.Rows.Count > 0) { // do something if the query returns rows } 

Answer 2

What you are trying to do makes perfect logical sense, and I can understand why you came to this implementation. However, what you are trying to do is very dangerous and, as a beginner in ASP.NET, you may not know that there are many other options available to you that simplify and greatly simplify the management of your data.

@iamkrillin hinted at one of these technologies - Object Relational Mapping (ORM). The .NET framework actually has first-class support for ORM, called the Entity Framework . I believe that the reason he suggested that you look into ORM is because your design is actually very similar to how ORM works. They are abstract classes that represent tables in your database that you can easily query with LINQ. LINQ queries are automatically parameterized and relieve you of the stress in managing the security of your queries. They generate SQL on the fly (just like you pass strings to your data access class) and are much more flexible in how they can return data (arrays, lists, you name it).

However, one of the drawbacks of ORM is that they have pretty steep learning curves. A simpler option (albeit a bit older than EF) is to use Typed Datasets. Typed datasets are much easier to create than creating ORMs and, as a rule, much easier to implement. Although they are not as flexible as ORMs, they do exactly what you are trying to do in a simple, safe and already resolved manner. Fortunately, when ASP.NET first released instructional videos that focused on typed datasets, and there are so many high-quality, freely available videos / tutorials for you to work quickly and quickly.

Answer 3

You are on the right track, and I really did what you are looking for too. However, instead of just passing the string to your function, I pass the SQL Command object ... So you can correctly build all your commands and parameters, and then say ... here, run this, it is ready to go. Sort of

 public static DataTable SqlDataTable(SqlCommand cmd) { using (SqlConnection conn = new SqlConnection(DatabaseConnectionString)) { cmd.Connection = conn; // store your connection to the command object.. cmd.Connection.Open(); DataTable TempTable = new DataTable(); TempTable.Load(cmd.ExecuteReader()); return TempTable; } } public DataTable GetMyCustomers(string likeName) { SqlCommand cmd = new SqlCommand(); cmd.CommandText = "select * from SomeTable where LastName like "@someParm%"; cmd.Parameters.Add( "whateverParm", likeName ); // don't have SQL with me now, guessing syntax // so now your SQL Command is all built with parameters and ready to go. return SqlDataTable( cmd ); } 

Answer 4

My suggestion: use orm. There are many options now days