Questions about SQL injection and Parameterised Queries

Question

I am working on a form based visual C# project in visual studio. It takes input into a C# form, the text boxes update our mysql server, and when the form is loaded the existing values in the data base are loaded from the DB.

I am a student at CSUS and the project is a part of my introduction to software engineering course. My team and I have taught ourselves C# and SQL in order to do the project, so pretty much every thing we do is either not exactly correct or really bad. I have recently discovered SQL injection and am trying to write my application to protect against it. I have read that I need to use paramertised inputs, but I am struggling with the syntax a bit. Most examples I find seem to assume I know more about C# or SQL then I do, and they show the correct syntax without many examples of how to convert it from the bad kind.

The code below is a simplified example of the kind of queries we use often in our program. PDFExporter is a library I created, and it has a globalvariable class that contains some string constants, one of them being our sql server connection string. Something that would help me a lot is an example of a similar SQL query written with parameters, and any tips or comments would be much appreciated.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
using System.Data.SqlClient;
using MySql.Data.MySqlClient;
//.
//.
//.

string column = "FR18_ClientID, ClientDSMIV, ClientPCounselor, ClientMethPayInt;
        string values = "'" + client + "', + '" + DSMIV.Text + "', + '" + PCounselor.Text + "', + '" + PaymentMethod.Text + "'";
        string onupdate = "FR18_ClientID = VALUES(FR18_ClientID) , ClientDSMIV = VALUES(ClientDSMIV), ClientPCounselor = VALUES(ClientPCounselor), ClientMethPayInt = VALUES(ClientMethPayInt);
        MySqlConnection con = null;
        con = new MySqlConnection(PDFExporter.GlobalVar.SERVER);
        con.Open();

            MySqlCommand command = con.CreateCommand();
 command.CommandText = "INSERT INTO FR18_Bookkeeping (" + column + ")" + " VALUES(" + values + ")ON DUPLICATE KEY UPDATE " + onupdate;   
        command.ExecuteNonQuery();
        con.Close();

Our program also has a lot of select statements, however the value we use to query the data base with is an internal value that is not set or altered by user input. A simple example might be

string autofill = " SELECT * FROM FR18_Bookkeeping WHERE FR18_ClientID = " + client;

Client is never set by the user, and its the foriegn key for pretty much all our tables. Is it necessary to paramertise this query?

sql injection is not the only reason to use query parameters. In fact, I don't even think it's the most important reason. My personal favourite is escaping apostrophes. — Dan Bracuk
– Dan Bracuk, Commented Apr 24, 2014 at 16:17

Marc Gravell · Accepted Answer · 2014-04-24 16:17:12Z

4

Parameters are not just about sql injection, although that is certainly a very important usage (and frankly, I'd still worry about sql injection even for internal data), but other advantages of parameters:

query plan cache / re-use
data-type correctness (avoids issues with formats, i18n, l10n, etc - especially when string dates and numbers)
data content correctness - i.e. when you need to store the client name O'Brien

I strongly recommend you use parameters. If you use tools like "dapper", this does not need to be complicated. For example:

int client = ...
var rows = conn.Query<Bookkeeping>(
    "SELECT * FROM FR18_Bookkeeping WHERE FR18_ClientID = @client",
    new { client }).ToList();

which is fully parameterized, without any pain.

answered Apr 24, 2014 at 16:17

Marc Gravell

1.1m273 gold badges2.6k silver badges3k bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Steve Over a year ago

I would also add, as an additional benefit, the clarity of the command text. No confusion caused by the concatenation operator and the string quoting. Particularly evident if the command is somewhat more complex. It is also easier to use the string literal character @ .

Collectives™ on Stack Overflow

Questions about SQL injection and Parameterised Queries

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related