C# and MySQL .NET Connector - Any way of preventing SQL Injection attacks in a generic class?

Question

My idea is to create some generic classes for Insert/Update/Select via a C# (3.5) Winforms app talking with a MySQL database via MySQL .NET Connector 6.2.2.

For example:

public void Insert(string strSQL)
{
   if (this.OpenConnection() == true)
   {
       MySqlCommand cmd = new MySqlCommand(strSQL, connection);
       cmd.ExecuteNonQuery();
       this.CloseConnection();
   }
}

Then from anywhere in the program I can run a query with/without user input by just passing a SQL query string.

Reading around on SO is starting to give me the indication that this may lead to SQL injection attacks (for any user-input values). Is there anyway of scrubbing the inputted strSQL or do I need to go and create individual parameterized queries in every method that needs to do a database function?

UPDATE1:

My Final solution looks something like this:

public void Insert(string strSQL,string[,] parameterValue)
{
   if (this.OpenConnection() == true)
   {
       MySqlCommand cmd = new MySqlCommand(strSQL, connection);

       for(int i =0;i< (parameterValue.Length / 2);i++)
       {                        
       cmd.Parameters.AddWithValue(parameterValue[i,0],parameterValue[i,1]);          
       }

       cmd.ExecuteNonQuery();
       this.CloseConnection();
   }}

This screams horrible idea. You should use an ORM or write stored procs. — Chris Marisic, May 05 '10 at 18:29

score 12 · Answer 1 · answered May 05 '10 at 18:24

12

Parametrization is very easy to do. Much easier than scrubbing SQL queries, and less messy or error prone than manual escaping.

Slightly edited copy/paste from this tutorial page because I'm feeling lazy:

// User input here
Console.WriteLine("Enter a continent e.g. 'North America', 'Europe': ");
string userInput = Console.ReadLine();

string sql = "SELECT Name, HeadOfState FROM Country WHERE Continent=@Continent";
MySqlCommand cmd = new MySqlCommand(sql, conn);
cmd.Parameters.AddWithValue("@Continent", userInput);

using (MySqlDataReader dr = cmd.ExecuteReader())
{
    // etc.
}

That wasn't so hard, was it? :)

answered May 05 '10 at 18:24

Thorarin

47,289
11
75
111

Thanks @Thorarin - I went with @Justin Niessner's answer due its generic nature (I don't want connection/MySQL code in every method) but your answer definitely helps with the syntax. – John M May 06 '10 at 15:57
@John M: You're welcome to write wrapper methods of course. My goal was to show the bare metal of it. – Thorarin May 06 '10 at 16:55

score 11 · Accepted Answer · answered May 05 '10 at 18:20

You should definitely be using parameterized queries to keep yourself safe.

You don't have to hand create parameterized queries each time though. You could modify the generic method you provided to accept a collection of MySqlParameters:

public void Insert(string strSQL, List<MySqlParameter> params)
{
    if(this.OpenConnection() == true)
    {
        MySqlCommand cmd = new MySqlCommand(strSQL, connection)
        foreach(MySqlParameter param in params)
            cmd.Parameters.Add(param);

        cmd.ExecuteNonQuery();
        this.CloseConnection();
    }
}

I should also mention that you should be VERY careful about cleaning up your connections after you're finished using them (typically handled in a using block, but I don't see that level of detail in your code example).

Thanks Justin - I was starting to think of a way to hybridize a generic class with parameterized queries but you beat me to it. Can you clarify what you mean by a using block? — John M, May 06 '10 at 15:55

score 1 · Answer 3 · answered May 05 '10 at 18:16

1

It's impossible to detect SQL injection after the fact (in other words, once you've constructed a dynamic query string, it's impossible to differentiate what the "real" SQL is versus any injected SQL).

If your intent is to allow users to execute arbitrary SQL, then it would seem like you wouldn't be too worried about SQL injection (since that is the aim of SQL injection).

answered May 05 '10 at 18:16

Adam Robinson

182,639
35
285
343

Thanks @Adam - I hadn't thought about that way - I suppose there may be some methods of using Regex, etc on the queries but each approach would probably have a vulnerability. – John M May 06 '10 at 15:59
2

@John: There's no generic way, RegEx or no. Whatever method you use is going to have to know *something* about how the query/statement *should* look, so there's no way to do it in a context-neutral way. – Adam Robinson May 06 '10 at 16:29

score 1 · Answer 4 · answered May 05 '10 at 18:17

I would expect that it would be pretty hard to scrub raw text that will be used for SQL. If at all possible I would try to use parameterized operations.

One exception would be if you didn't expose the function publicly, and you never passed in a string that was constructed from raw user input.

score 1 · Answer 5 · answered May 05 '10 at 18:17

1

if you use MySqlParameter and do not generate plain string queries you are safe.

answered May 05 '10 at 18:17

Andrey

59,039
12
119
163

score 1 · Answer 6 · answered May 05 '10 at 18:17

1

You can't really do this - you'd need to write a SQL parser which to say the least is non-trivial and error prone.

Bite the bullet and parametrize your queries.

answered May 05 '10 at 18:17

Joe

122,218
32
205
338

score 1 · Answer 7 · answered May 05 '10 at 18:18

1

I would suggest utilizing IDataParameter objects to parameterize your queries.

answered May 05 '10 at 18:18

programatique

850
1
7
13

score 1 · Answer 8 · answered May 05 '10 at 18:25

1

YES you need to create parameterized queries, anything else is going to introduce a risk of SQL injection

answered May 05 '10 at 18:25

Mitchel Sellers

62,228
14
110
173

I wouldn't go that far. There are airtight ways of escaping user input. Unfortunately programmers are only human, and might forget to escape something... – Thorarin May 05 '10 at 18:28
The question was to process/scrub AFTER it has been combined with SQL....at that point is is almost impossible to do – Mitchel Sellers May 05 '10 at 19:14

C# and MySQL .NET Connector - Any way of preventing SQL Injection attacks in a generic class?

8 Answers8

Linked