1

I've got the following

SqlCommand cmd = getSQLCommand();
using (cmd.Connection)
using (cmd)
{
    try
    {
        string dbName = txt_DatabaseName.Text;
        var createDatabaseQuery = "exec ('CREATE DATABASE ' + @databaseName)";
        var sqlCommand = new SqlCommand(createDatabaseQuery, cmd.Connection);
        sqlCommand.Parameters.Add("@databaseName", SqlDbType.Text);
        sqlCommand.Parameters["@databaseName"].Value = dbName.ToString();

        cmd.Connection.Open();

        sqlCommand.ExecuteNonQuery();
    }
    catch (SqlException ex)
    {
        Console.WriteLine(ex.ToString());
        ScriptManager.RegisterClientScriptBlock(this, this.GetType(), "alertMessage", "alert('SQL Error. Record not added.')", true);
    }
    finally
    {
        cmd.Connection.Close();
    }
}

I'm fully aware that params are not supported in DDL operations, so I've got this really cool thread that I've been using to help me write the contents within the "try". How to use SqlCommand to CREATE DATABASE with parameterized db name?

That said, I'm still getting an exception error for incorrect syntax near 'Database'. This might be a user error but I've been stuck around this for an hour or so now.

Any thoughts/ improvements?

p.s. All I'm trying to do is to create a database programmatically by using a dynamic value of whatever happens to be in txt_DatabaseName.Text (and yes I will try to error handle this in case there's white spaces entered or any chars that are not supported in SQL.

p.p.s Any articles that I can have a look at against sql injection attacks or any suggestions around constructing the method I have to prevent it? This is a simple exercise that I'm doing on my local machine, not public facing but still would like to get ahead of the game if possible.

Massimiliano Kraus
  • 3,638
  • 5
  • 27
  • 47
MrDedupe
  • 23
  • 4
  • 4
    Why you use `exec` and not simply `"CREATE DATABASE " + dbName.ToString()`? – Tim Schmelter Jul 18 '17 at 12:09
  • I still get Message = "Incorrect syntax near 'DATABASE'." – MrDedupe Jul 18 '17 at 12:13
  • but if I do something simple "CREATE DATABASE TEST" it works just fine.. it's the syntax of the "CREATE DATABASE " + dbName.ToString(); that is doesn't like – MrDedupe Jul 18 '17 at 12:13
  • @TimSchmelter let me introduce you to SQL Injection. (Albeit as it is being substituted into a string parametrisation will not help here: the value needs to be carefully checked, however given the number of concatenations seen around here any – however futile – attempt at parametrisation should be seen positively) – Richard Jul 18 '17 at 12:14
  • @Richard you can assume that Tim Schmelter already knows about SQL Injection. The question is why the OP tried such a convoluted query - EXEC and parentheses and concatenation. And a `.ToString()` call on something that can only be a string – Panagiotis Kanavos Jul 18 '17 at 12:20
  • To be clear - is it failing no matter what value is given for the parameter, or have you only tried with one test value? If the latter, what test value are you providing? – Damien_The_Unbeliever Jul 18 '17 at 12:29
  • @MrDedupe what is the *value* of the parameter? I just verified that `declare @databaseName nvarchar(20) ='foo'; exec ('drop DATABASE ' + @databaseName)` works. Your statement *still* uses concatenation, so it's *still* vulnerable to concatenation issues. A space or other unexpected character would cause this to fail. In the end, you aren't doing anything better than concatenating `CREATE DATABASE ` with another string – Panagiotis Kanavos Jul 18 '17 at 12:30
  • I can reproduce the error if `@databaseName` is an empty string - working out why its empty is outside of the scope of the code you've shown and should be a matter of debugging your code. – Damien_The_Unbeliever Jul 18 '17 at 12:34
  • This code will run if you enter `foo; SELECT 1;` as the database name. It will create the database and return 1. This is *still* vulnerable to injection – Panagiotis Kanavos Jul 18 '17 at 12:35
  • Possible duplicate of [SQL Server: use parameter in CREATE DATABASE](https://stackoverflow.com/questions/5626850/sql-server-use-parameter-in-create-database) – mjwills Jul 18 '17 at 12:58
  • @Panagiotis any suggestions on how to not make it vulnerable to injections? any relevant threads that can be correlated with this? – MrDedupe Jul 18 '17 at 13:01
  • 1
    Use [SMO](https://learn.microsoft.com/en-us/sql/relational-databases/server-management-objects-smo/tasks/creating-altering-and-removing-databases) to create database objects etc, not raw ADO.NET – Panagiotis Kanavos Jul 18 '17 at 13:06

2 Answers2

0

Take a look with the sql profiler to see what is being fired against the database. If it is not working try to execute the query in Management studio to see it that is working. It's probably some kind of special character that is not allowed.

Jordy van Eijk
  • 2,718
  • 2
  • 19
  • 37
  • No need for the profiler, If i enter something simple like "CREATE DATABASE TEST" it works, it just doesn't understand the syntax if we do it with a variable rather than a hardcoded text. The catch method gives us the following error during debug ex = {"Incorrect syntax near 'DATABASE'."} – MrDedupe Jul 18 '17 at 12:16
  • @MrDedupe who says it works? If you take a look look at the profiler you can see what your code is generating and firing against the database. Maybe it is doing something else that you didn't expect. its clearly not `CREATE DATABASE TEST` – Jordy van Eijk Jul 18 '17 at 12:19
  • it works because it creates the database called TEST under the instance of my SQL that the cmd had all the connection strings established. – MrDedupe Jul 18 '17 at 12:22
  • So you are saying you execute your C# code it creates a database but it returns `incorrect syntax near database`, am i understanding that? – Jordy van Eijk Jul 18 '17 at 12:30
0

You don't need the exec part at all. Again you are getting error after removing exec cause you are wrapping your query in single quote 'CREATE DATABASE ' which is getting considered as string literal. It should just be

var createDatabaseQuery = string.Format("CREATE DATABASE {0}",DBnamevariable);
var sqlCommand = new SqlCommand(createDatabaseQuery, cmd.Connection);
Rahul
  • 76,197
  • 13
  • 71
  • 125
  • I'm pretty certain that you can't use a parameter with `CREATE DATABASE`. Tha's DDL, not DML – Panagiotis Kanavos Jul 18 '17 at 12:21
  • @PanagiotisKanavos, real embarrassing ... thanks for pointing that. how I missed that !!! – Rahul Jul 18 '17 at 12:23
  • 1
    Unless the OP performs some rigorous validation of `DBnamevariable` this will expose the server to SQL injection attacks – Panagiotis Kanavos Jul 18 '17 at 12:23
  • hmm very interesting! we have a different error var createDatabaseQuery = "CREATE DATABASE @databaseName)"; so it actually recognizes the Create Database command – MrDedupe Jul 18 '17 at 12:24
  • @MrDedupe what is the value of the parameter? You are still concatenating strings. Any weird values, spaces, dots, etc will result in an invalid statement. Add a `;` and you will execute *two* statements – Panagiotis Kanavos Jul 18 '17 at 12:37