2

I have been trying to add a column programmatically in ASP.NET to modify the tables in SQL Server.

Please see the following code:

 string suppliernotxt = supplieridlist[1].ToString();
 //SqlCommand cmd2 = new SqlCommand("ALTER TABLE [ProductNormalDB] ADD suppliernotxt nvarchar(20) NULL", con);
 SqlCommand cmd2 = new SqlCommand("ALTER TABLE ProductNormalDB ADD @supplierlist nvarchar(20) NULL", con);
 cmd2.Parameters.AddWithValue("@supplierlist", suppliernotxt);
 //cmd2.Parameters.AddWithValue("@supplierlist", suppliernotxt.ToString());
 //cmd2.Parameters["@supplierlist"].Value = supplieridlist[x];
 cmd2.ExecuteNonQuery();

supplieridlist is an array that acquires all the column names to add into the SQL Server database. For some reason the parametrized method is not working and shows the following error:

Incorrect syntax near '@supplierlist'.

The basic idea is to have a user select from a check box the name of the suppliers, based on the selected number of suppliers the array will create the supplier names for ex. if we selected 3 suppliers, the array will save "Supplier1", "Supplier2", "Supplier3" and then the SqlCommand is supposed to alter the table and add the new columns.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Soulblade
  • 71
  • 1
  • 2
  • 14
  • 2
    You should normalize your DB. For example, you could create a many-to-many table `AsocProductSupplier` with following columns: `ProductID` and `SupplierID`. – Bogdan Sahlean Dec 21 '13 at 14:04

1 Answers1

9

You cannot use parameters to express the name of columns.
Parameters could only be used to express values for WHERE clause or for INSERT or UPDATE statements.

You could use string concatenation for your query text, passing the string value to a stored procedure or use some form of dynamic sql.

Please be very carefull with these kind of approaches because if you don't keep absolute control on the values passed to your code you will be exposed to Sql Injection.

Adding as an example of Dynamic SQL execution, but still vulnerable to SQL Injection

string suppliernotxt = supplieridlist[1].ToString();
string execSQL = "DECLARE @sup nvarchar(15); " + 
                 "SET @sup = '" + suppliernotxt + "'; " +
                 "EXEC ('ALTER TABLE ProductNormalDB  ADD ' + @sup + ' nvarchar(20) NULL')"
SqlCommand cmd2 = new SqlCommand(execSQL, con);
cmd2.ExecuteNonQuery();

As you can see, even with Dynamic SQL there is nothing that prevent an SQL Injection attack passing via the suppliernotxt variable

EDIT As explained in the comments below from @RBarryYoung, a good improvement on the SQL Injection problem for this case of dynamic sql could be the usage of the QUOTENAME function to obtain an Unicode string with the required delimiters around the input string 

string execSQL = "DECLARE @sup nvarchar(15); " + 
                 "SET @sup = QUOTENAME('" + suppliernotxt + "'); " +
                 "EXEC ('ALTER TABLE ProductNormalDB  ADD ' + @sup + ' nvarchar(20) NULL')"
Steve
  • 213,761
  • 22
  • 232
  • 286
  • I would suggest the use of the T-SQL `QUOTENAME(..)` function on the `@sup` variable to [address the Injection threat](http://msdn.microsoft.com/en-us/magazine/cc163523.aspx). It's not guaranteed (the way parameterization is, where it applies), but it's the next best thing and this is MS recommends for this situation. – RBarryYoung Dec 21 '13 at 17:08
  • @RBarryYoung thank you for your observation. I was not aware of this workaround. So do you suggest: `"SET @sup = QUOTENAME('" + suppliernotxt + "'); "` or directly in the ALTER TABLE statement? – Steve Dec 21 '13 at 17:15
  • The first one, in the `SET` statement. – RBarryYoung Dec 21 '13 at 17:17
  • IMHO, the order of precedence for SQL Injection fixes, 1) parametization, always, 2) [Whitelisting](http://stackoverflow.com/a/1246848/109122) on parameters, when you have to use Dynamic SQL, and 3) `QUOTENAME` when whitelisting cannot be used on the parameters (as in this example). – RBarryYoung Dec 21 '13 at 17:24