0

I'm trying to create a re-usable MySQL database class in c# but I'm concerned about keeping it secure (Possible business use). I've done a lot of reading and people recommend using parameters to add data into the table and fetch it etc. My problem is keeping the class re-usable through a number of different tables My Insert Method Currently (Inside a database class):

public void Insert(string incomingQuery) //Insert Statement
{
    //Assign the incomingQuery for use.
    string query = incomingQuery;

    //Open Database Connection
     if(this.OpenConnection() == true)
     {
         //Create command and assign the query and connection from the constructor
         MySqlCommand command = new MySqlCommand(query, connection);

        //Execute the command
        command.ExecuteNonQuery();

        //Close the connection
        this.CloseConnection();
    }

}

And my Create method passing the SQL Query in from another class (Users):

public void Create()
{
    //Add a new user to the database
    string sqlQuery = "INSERT INTO Users(first_name, last_name, pc_username) VALUES('" + firstName + "','" + lastName + "','" + userName + "');";
    database.Insert(sqlQuery);
    Login();
}

How can I make this more secure?

mdml
  • 22,442
  • 8
  • 58
  • 66

3 Answers3

2

How can I make this more secure?

Use parameters. Your current query is susceptible to SQL Injection.

Also it seems that you have an open connection in your method, its better to open the database connection as late as possible and then close it as early as possible. Use using statement with your connection and open it in your method, since Connection and Command implements IDisposable interface, it will ensure its disposal (closing connection).

Habib
  • 219,104
  • 29
  • 407
  • 436
  • Hi, Habib. Can you provide a code example, I understand that I need to use Parameters but all my attempts at doing so have cost me the re-usable requirement of the class because of the need to hard code the exact fields into it. – Mark Tallentire Nov 01 '13 at 13:24
  • @MarkTallentire, you can still use the class that you have, you should modify it for opening connection in the method, See: http://dev.mysql.com/doc/refman/5.0/es/connector-net-examples-mysqlcommand.html#connector-net-examples-mysqlcommand-parameters for parameters – Habib Nov 01 '13 at 13:25
1

Use stored procedures in your MySQL database, then call these procedures with their required parameters. Write your utility methods so that they take a Dictionary of Key-Value pairs, and parse those into the Parameters property of the OdbcCommand

Call a stored procedure with parameter in c#

foreach(KeyValuePar kvp in dictionary)
{
    command.Parameters.Add(kvp.Key).Value = kvp.Value;
}

I forget the precise code...

Community
  • 1
  • 1
Colin Steel
  • 1,045
  • 1
  • 11
  • 24
0

Pass all the parameters (firstName, lastName etc) as a string[] (or better, a Dictionary) to a method resembling the following:

AddParamsToCommand(SqlCommand cmd, Dictionary<string,string> paramList)
{
    foreach (string s in paramList.Keys)
    {
        sqlParameter p = cmd.CreateParameter();
        p.Name = s;
        p.Value = paramList[s];
        cmd.Parameters.Add(p);
    }
}

Just as an example.

Edited since I've been using OleDB and forgot about putting names on params in for SqlCommands.

TJennings
  • 432
  • 3
  • 14
  • Wait... Doesnt yours name a parameter the same as its value, you need both, separately – Colin Steel Nov 01 '13 at 13:28
  • Yes. You /can/ do it this way, if you're passing the value through a lot. I've been using OleDB a lot lately, where params can't have names. – TJennings Nov 01 '13 at 13:30