SQL Injection issue with the query in C#

Question

In some C# code, table names passed in via an enum are being inserted into SQL queries using string.Format like so:

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 run the Veracode tool it shows this query has possibility of SQL injection when executing.

command.ExecuteNonQuery();

I want to avoid the possibility of SQL injection with the above code. I tried adding a tag (@tablename), but that did not work.

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

How do I avoid this?

@DragandDrop why you suggesting duplicate about parameters when you probably know that table names can't be parametrized? — Alexei Levenkov
– Alexei Levenkov, Commented Oct 18, 2017 at 6:23
You should always use parameterized query, search on parameterized query for more detail — Jameel Hussain
– Jameel Hussain, Commented Oct 18, 2017 at 6:24
Possible duplicate of SqlParameter does not allows Table name - other options without sql injection attack? — tia
– tia, Commented Oct 18, 2017 at 6:27
@jimmi94 showing example would be much more useful than suggesting something that can't be done... stackoverflow.com/questions/3128582/… is probably the best you can do for table names if you must have them inserted... — Alexei Levenkov
– Alexei Levenkov, Commented Oct 18, 2017 at 6:27
@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
– Drag and Drop, Commented Oct 18, 2017 at 6:34

Amit Kumar Singh · Accepted Answer · 2017-10-18 06:23:48Z

0

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.

answered Oct 18, 2017 at 6:23

Amit Kumar Singh

4,4952 gold badges11 silver badges24 bronze badges

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

Comments

Zohar Peled · Accepted Answer · 2017-10-18 06:23:58Z

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.

Collectives™ on Stack Overflow

SQL Injection issue with the query in C#

2 Answers 2

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related