1
con1.Open();
cmd = new SqlCommand("create table [dbo.][" + textBox2.Text + "](sno int primary key identity(1,1),[Week] [int]  not null ,Date nvarchar(30) not null,Time nvarchar(30) not null,Monday_1st_half nvarchar(20),Monday_2nd_half nvarchar(20),Tuesday_1st_half nvarchar(20),Tuesday_2nd_half nvarchar(20),Wednesday_1st_half nvarchar(20),Wednesday_2nd_half nvarchar(20),Thursday_1st_half nvarchar(20),Thursday_2nd_half nvarchar(20),Friday_1st_half nvarchar(20),Friday_2nd_half nvarchar(20),Saturday_1st_half nvarchar(20),Saturday_2nd_half nvarchar(20),Sunday_1st_half nvarchar(20),Sunday_2nd_half nvarchar(20))", con1);
cmd.ExecuteNonQuery();
con1.Close();

This query works if i give the table a hard coded name instead of the textbox.text value, a table is created. But when i use this code it creates a table with the name 'dbo.'. If i delete the "[dbo.]" from the query, it gives an error saying 'object is missing'. can someone help me out?.

James
  • 23
  • 1
  • 7
  • 3
    fyi this is asking for sql injection. – Daniel A. White Nov 28 '16 at 17:31
  • 2
    Aside from the horrible SQL injection vulnerability, the square brackets should be around the `dbo` not including the period e.g. `create table [dbo].[table] ...` – DavidG Nov 28 '16 at 17:34
  • A stored procedure would be much better to do this, pass the table name as a parameter. and of course don't use this command text, as its very easy for SQL injection – Yaman Nov 28 '16 at 17:38
  • @Yaman I don't think you can pass table names as a parameter. – LarsTech Nov 28 '16 at 17:41
  • @LarsTech something like this http://stackoverflow.com/questions/1246760/how-should-i-pass-a-table-name-into-a-stored-proc – Yaman Nov 28 '16 at 17:45
  • Guys... Thank you so much for your replies.. I know its a bad practice to code like this..its actually my very first project.. i am a beginner in college and its a test..i only have to get this code it work ...ill eventually get used to a safer method of coding like you guys mentioned. anyways ill try using a stored procedure like you mentioned @Yaman. Thank you DavidG, corrected that...still no effect.. – James Nov 28 '16 at 17:52
  • You can do this (more) safely with a stored procedure and dynamic sql. I have posted an example. – RBarryYoung Nov 28 '16 at 17:54
  • @Yaman I do like the answer at that link ... ;-) – RBarryYoung Nov 28 '16 at 18:00

1 Answers1

0

Here's a better way to do this. First create the following stored procedure:

CREATE PROC dbo.MakeUserTable(@Tablename as SYSNAME) As

    DECLARE @CleanName As SYSNAME;
    SET @CleanName = QUOTENAME(@Tablename, '[');

    DECLARE @sql As NVARCHAR(MAX);
    SELECT @sql = 'create table [dbo].' + @CleanName + '(sno int primary key identity(1,1),[Week] [int]  not null ,Date nvarchar(30) not null,Time nvarchar(30) not null,Monday_1st_half nvarchar(20),Monday_2nd_half nvarchar(20),Tuesday_1st_half nvarchar(20),Tuesday_2nd_half nvarchar(20),Wednesday_1st_half nvarchar(20),Wednesday_2nd_half nvarchar(20),Thursday_1st_half nvarchar(20),Thursday_2nd_half nvarchar(20),Friday_1st_half nvarchar(20),Friday_2nd_half nvarchar(20),Saturday_1st_half nvarchar(20),Saturday_2nd_half nvarchar(20),Sunday_1st_half nvarchar(20),Sunday_2nd_half nvarchar(20));'

    PRINT 'Executing "'+@sql+'"';
    EXEC(@sql);

Now change your client code to execute this instead, passing in the tablename as a parameter.

I cannot 100% guarantee that this is immune to SQL Injection attacks, but if you have to accept a tablename from a user, this is about as safe as you can get.

RBarryYoung
  • 55,398
  • 14
  • 96
  • 137
  • Oops, found a bug. Corrected now. – RBarryYoung Nov 28 '16 at 17:52
  • Thank you so much @RBarry Young..I'll try this and get back to you :) – James Nov 28 '16 at 17:58
  • 1
    While using `QUOTENAME` fixes the SQL injection problem, it doesn't fix what is a very bad design. Much better to use a single table, perhaps with an extra column to identify which user those rows belong to. – DavidG Nov 28 '16 at 18:00
  • @DavidG It depends on why and what is really going on here, but yeah, probably. – RBarryYoung Nov 28 '16 at 18:01
  • Also @RBarryYoung why not close as a dupe as this answer is effectively the same as your other one linked above? (and that answer is much more comprehensive!) – DavidG Nov 28 '16 at 18:01
  • @DavidG It's not actually the same. That question was asking about passing an *existing* tablename, for which there is a much better solution. (and probably a much better justification) – RBarryYoung Nov 28 '16 at 18:03
  • Conceptually it's the same though, "passing a table name into a query". – DavidG Nov 28 '16 at 18:04
  • @DavidG but again, the answer I posted there cannot be used here. The use of `QUOTENAME` in that answer is peripheral, but in this answer it is essential. – RBarryYoung Nov 28 '16 at 18:05
  • My rule of thumb is that is a destination answer could be made into a great canonical, then it's a valid dupe target. That answer could be that with a little work. – DavidG Nov 28 '16 at 18:07
  • While I immodestly agree that that is a great answer (one of my best ever), IMHO what makes it great is the same thing that makes it inapplicable to this question: the use of the existing table names. – RBarryYoung Nov 28 '16 at 18:13