Can I still use parameters for sql command?

Question

Im trying to make a generic code for building Insert/Update query. So far i have only created the Update query, but i'mhaving doubts considering SQL injection.

My primary target is trying to create code to decrease the time of retyping the same code over and over.

public SqlConnection SqlConn;
private SqlCommand SqlComm;

public void UpdateRow(string TableName, string UpdateCondition, List<KeyValuePair<string, string>> FieldAndValueList)
{
  SqlOpen();
  try
  {
    string UpdateString = $"UPDATE {TableName} ";
    int counter = 0;
    foreach (KeyValuePair<string, string> FieldAndValue in FieldAndValueList)
    {
      if (counter > 0) { UpdateString += "," };
      UpdateString += $"SET {FieldAndValue.Key} = {FieldAndValue.Value} ";
      Counter += 1;
    }
    if (UpdateCondition.Trim() != "") { UpdateString += $"WHERE {UpdateCondition};"; }
    SqlComm = SqlConn.CreateCommand();
    SqlComm.CommandText = UpdateString;
    SqlComm.ExecuteNonQuery;
  }
  catch { ShowError(); }
  finally { SqlClose(); } 
}

It would then be executed like so:

List<KeyValuePair<string, string>> UpdateValues = new List<KeyValuePair<string, string>>;
UpdateValues.Add(new KeyValuePair<string, string>("age", txtAge.text)); 
UpdateRow("user", "user_id = X", UpdateValues);

Im trying to create it so SQL injection is not possible.

Your code is not safe. Use Parameters, see stackoverflow.com/questions/4892166/… — Ian Mercer
– Ian Mercer, Commented Jun 7, 2019 at 8:23
List<KeyValuePair<string, string>> == {"myField", "123; delete from table myTable; --"} The text created will be update SomeTable set myField = 123; delete from table myTable; -- UpdateCondition, you'll update SomeTable and clear MyTable — Dmitrii Bychenko
– Dmitrii Bychenko, Commented Jun 7, 2019 at 8:26
@Ian, I already had my doubt about it not being safe. But i cant find out how to actually make it safe. This method makes the size of the update statement variable Im looking for a way to handle parameters while keeping the variable size of the update statement. — Roberto
– Roberto, Commented Jun 7, 2019 at 8:27
@Roberto: where is the user input (i.e. arbitrary, potentially malicious strings) in the routine? Is it List<KeyValuePair<string, string>> only? — Dmitrii Bychenko
– Dmitrii Bychenko, Commented Jun 7, 2019 at 8:29
additional note: FieldAndValueList as a <string,string> is also probably a very bad idea, unless all the values are strings. That isn't the way to do data, basically. If the value is a float, send it as a float - otherwise, you can get into a lot of problems with conversions, including things like date formats, and different (international) representations of numbers (commas vs periods, etc) — Marc Gravell
– Marc Gravell, Commented Jun 7, 2019 at 8:33

Marc Gravell · Accepted Answer · 2019-06-07 08:28:44Z

3

I'm trying to create it so SQL injection is not possible.

Then you must use parameters; for the individual fields, something like the following would work:

if (counter > 0) { sb.Append(","); }
var pName = "@p" + counter.ToString();
sb.Append("[").Append(FieldAndValue.Key).Append("]=").Append(pName);
cmd.Parameters.AddWithValue(pName, ((object)FieldAndValue.Value) ?? DBNull.Value);
Counter += 1;

where sb is a StringBuilder (to avoid too many string concatenations)

However! Your planned string UpdateCondition is inherently not solvable. That cannot be made injection-safe. You need to think of a better way of doing that.

Or, better: use any off-the-shelf ORM.

answered Jun 7, 2019 at 8:28

Marc Gravell

1.1m273 gold badges2.6k silver badges3k bronze badges

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

3 Comments

Roberto Over a year ago

Hallo Marc, Ill try this. As for the Condition. Its hardcoded from the form that actually calls the function. So no user input.

Marc Gravell Over a year ago

@Roberto just to emphasize: the last line in the answer is my strong recommendation; IMO you're going to get yourself into a lot of nasty holes trying to re-invent this stuff

Roberto Over a year ago

Guess i was looking from the wrong angle as i couldnt find that. But it's more that im trying to get back into C# (havent done anything for 2y as i have been doing Delphi). But i'll have a look into it.

Collectives™ on Stack Overflow

Can I still use parameters for sql command?

1 Answer 1

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related