c# - Parametrized Query

Question

I am working on a .net website which uses a DB2 database which uses Insert/Update and Select Queries. I researched about SQL Injection and I believe I've parametrized my query to avoid SQL Injection. Could you check if I've done it correctly and is there a better way or more sufficient way of doing it?

strInsert = "INSERT INTO DATABASE.Table(NUMBER,SIGNATURE,MESSAGE,CDATE,CTIME) VALUES (?,?,?,?,?)";

DB2Command cmdInsertQuery = new DB2Command(strInsert, db2Connection1);

cmdInsertQuery.Parameters.Add("NUMBER", i);
cmdInsertQuery.Parameters.Add("SIGNATURE", strSignature.Trim());
cmdInsertQuery.Parameters.Add("MESSAGE", strMessage.Trim());
cmdInsertQuery.Parameters.Add("CDATE", DateTime.Now.ToShortDateString());
cmdInsertQuery.Parameters.Add("CTIME", DateTime.Now.ToShortTimeString());
cmdInsertQuery.ExecuteNonQuery();

The query inserts the data correctly and works fine.

Looks good to me. Glad to see you're cognizant of SQL injection. SO is full of questions where people are obviously not, and it makes me cringe every time. I often wonder how much of the important software I use every day -- e.g. online baking software, etc. -- hasn't dealt with the SQL injection risks properly. — rory.ap, Apr 12 '16 at 11:52
Thanks. Is using ?,?,? fine or is using values(@NUMBER etc) better/more secure? Ive seen some people using @ and only seen a few using?,?,? etc — user6097989, Apr 12 '16 at 11:59
The cdate and ctime part looks suspect. You are passing strings for your parameters. If those fields have string datatypes, the code is ok but your database design is not. — Dan Bracuk, Apr 12 '16 at 12:45
The database has the fields as Characters and stores it fine for my needs. Thanks :) — user6097989, Apr 12 '16 at 13:07
@user6097989 - No, you have a hidden issue that could bite you at any time. What do `CDATE` and `CTIME` (or their combination) represent? If it's a log of the creation time (as it appears to be), you should be using a single `TIMESTAMP` field on the database side. You should also be storing any absolute times in UTC - DB2 (and C# for that matter) has poor support for messing with timezones. Also, `ToShortDateString` is **culture dependent**, which means you could get multiple date formats, including ambiguous ones, in that column - you absolutely don't want that. — Clockwork-Muse, Apr 12 '16 at 14:10

score 0 · Answer 1 · answered Apr 12 '16 at 12:01

0

Yes, you have done it correctly. Good to know you are aware of SQL injection problems and trying ways to eradicate it.

answered Apr 12 '16 at 12:01

Kunal

33
1
2
6

score 0 · Answer 2 · answered Apr 12 '16 at 12:29

0

add is deprecated use addwithvalue, it still almost does the same thing as add but my visual studio always cries about it

Example

string insertStatement =
           "Insert Login VALUES(@username,@password,@publicKey,@privateKey,@salt)";
            SqlCommand insertCommand = new SqlCommand(insertStatement, connection);
            insertCommand.Parameters.AddWithValue("@username", username);

answered Apr 12 '16 at 12:29

Francesco Ventura

607
8
19

`Add` is safer than `AddWithValue`. See: https://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/ – Adam Calvet Bohl Aug 01 '18 at 04:55

score 0 · Answer 3 · answered Apr 12 '16 at 15:10

0

and is there a better way or more sufficient way of doing it?

one of options is to use https://github.com/StackExchange/dapper-dot-net

answered Apr 12 '16 at 15:10

Ali

1,982
1
15
16

c# - Parametrized Query

3 Answers3