SQL Injection issue with the query in C#

Question

I have some SQL Queries written in my C# code.Table names are passing to the constructor using an enum.Then it assign to a global variable and append to the string,

const string ADD_SQL = "INSERT INTO {0} (ColumnOne) VALUES (@valueOne)";
const string CLEAR_SQL = "DELETE FROM {0}";

var commandText = string.Format(ADD_SQL , _tableName);

But when I am running Veracode tool it shows this query has possibility of SQL Injection when going to execute.

command.ExecuteNonQuery();

Any possible solution to avoid this SQL Injection scenario from the code.Need a recfatoring to the above const.I tried with adding a tag (@tablename) and tried.But it is not succeeded.

const string ADD_SQL = "INSERT INTO @tablename (Data) VALUES (@valueOne)";
var commandText = ADD_MESSAGE_SQL.Replace("@tablename", _tableName);

Any other possible solution to avoid this?

@DragandDrop why you suggesting duplicate about parameters when you probably know that table names can't be parametrized? — Alexei Levenkov, Oct 18 '17 at 06:23
You should always use parameterized query, search on parameterized query for more detail — Jameel Hussain, Oct 18 '17 at 06:24
Possible duplicate of [SqlParameter does not allows Table name - other options without sql injection attack?](https://stackoverflow.com/questions/17947736/sqlparameter-does-not-allows-table-name-other-options-without-sql-injection-at) — tia, Oct 18 '17 at 06:27
@jimmi94 showing example would be much more useful than suggesting something that can't be done... https://stackoverflow.com/questions/3128582/table-name-and-table-field-on-sqlparameter-c is probably the best you can do for table names if you must have them inserted... — Alexei Levenkov, Oct 18 '17 at 06:27
@AlexeiLevenkov, you are right. Flag Retracted as it's not a dupe. I read to fast, But as there is no proof of parameter beeing used, I will add the link as related. Related: [Executing query with parameters](https://stackoverflow.com/questions/11905185). — Drag and Drop, Oct 18 '17 at 06:28
@DragandDrop Tables names are not known.They are randomly changing according to the several scenarios. — LahiruD, Oct 18 '17 at 06:31
@LahiruD, `SELECT table_name FROM INFORMATION_SCHEMA.TABLES WHERE table_name=@Param` now you are pretty sure there is no sql injection. it's from a comment on @Tia dupe target. — Drag and Drop, Oct 18 '17 at 06:34

score 0 · Answer 1 · answered Oct 18 '17 at 06:23

There is a good chance that Veracode does not like you putting SQL queries like your current statements, and instead wants to use it's prescribed way of writing this code. And as visible in the documentation in the Repair section, it wants you to use prepared statement to create a parameterized query.

It's upto your choice now. My take on this would be that stored procedures will be better, but if you have to keep query in C#, just don't try to make one query too generic for all scenarios and tables.

score 0 · Answer 2 · answered Oct 18 '17 at 06:23

Concatenating strings into SQL statements is risky if your strings comes from user input.
While that is not the case you described, I'm guessing the Veracode tool doesn't know where the strings come from, it just see string concatenation and issue a warning.

A better approach would be to write complete SQL statements for each table (Usually I prefer using stored procedures, but that's another topic [you can search for stored procedures vs inline SQL]) and use parameters for values (Identifiers can't be parameterized in SQL, as you already found out).

So instead of

const string ADD_SQL = "INSERT INTO {0} (ColumnOne) VALUES (@valueOne)";
const string CLEAR_SQL = "DELETE FROM {0}";

and adding the table names at run time, here is a better solution:

const string ADD_tableName = "INSERT INTO TableName (ColumnOne) VALUES(@ValueOne)"
const string CLEAR_tableName = "DELETE FROM TableName";

There are even better solutions out there, but this is the easiest fix for the code you presented.

SQL Injection issue with the query in C#

2 Answers2