6

I'm trying to set up so that the table name is passed to the command text as a parameter, but I'm not getting it to work. I've looked around a bit, and found questions like this: Parameterized Query for MySQL with C#, but I've not had any luck.

This is the relevant code (connection == the MySqlConnection containing the connection string):

public static DataSet getData(string table)
{
    DataSet returnValue = new DataSet();
    try
    {
        MySqlCommand cmd = connection.CreateCommand();
        cmd.Parameters.AddWithValue("@param1", table);
        cmd.CommandText = "SELECT * FROM @param1";

        connection.Open();

        MySqlDataAdapter adap = new MySqlDataAdapter(cmd);
        adap.Fill(returnValue);
    }
    catch (Exception)
    {   
    }
    finally
    {
        if (connection.State == ConnectionState.Open)
            connection.Close();
    }
    return returnValue;
}

If I change:

cmd.CommandText = "SELECT * FROM @param1";

to:

cmd.CommandText = "SELECT * FROM " + table;

As a way of testing, and that works (I'm writing the xml from the dataset to console to check). So I'm pretty sure the problem is just using the parameter functionality in the wrong way. Any pointers?

Also, correct me if I'm mistaken, but using the Parameter functionality should give complete protection against SQL injection, right?

2 Answers 2

9

You can not parameterize your table names, column names or any other databse objects. You can only parameterize your values.

You need to pass it as a string concatenation on your sql query but before you do that, I suggest use strong validation or white list (only fixed set of possible correct values).

Also, correct me if I'm mistaken, but using the Parameter functionality should give complete protection against SQL injection, right?

If you mean parameterized statements with "parameter functionality", yes, that's correct.

By the way, be aware, there is a concept called dynamic SQL supports SELECT * FROM @tablename but it is not recommended.

As we have seen, we can make this procedure work with help of dynamic SQL, but it should also be clear that we gain none of the advantages with generating that dynamic SQL in a stored procedure. You could just as well send the dynamic SQL from the client. So, OK: 1) if the SQL statement is very complex, you save some network traffic and you do encapsulation. 2) As we have seen, starting with SQL 2005 there are methods to deal with permissions. Nevertheless, this is a bad idea.

There seems to be several reasons why people want to parameterise the table name. One camp appears to be people who are new to SQL programming, but have experience from other languages such as C++, VB etc where parameterisation is a good thing. Parameterising the table name to achieve generic code and to increase maintainability seems like good programmer virtue.

But it is just that when it comes to database objects, the old truth does not hold. In a proper database design, each table is unique, as it describes a unique entity. (Or at least it should!) Of course, it is not uncommon to end up with a dozen or more look-up tables that all have an id, a name column and some auditing columns. But they do describe different entities, and their semblance should be regarded as mere chance, and future requirements may make the tables more dissimilar.

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

3 Comments

Ah, I see. Thank you for the suggestion. I'll accept it when the timer is up. And yeah I meant MySqlCommand.Patameters functionality (hence the capitalization of Parameters, sorry if it wasn't clear enough)
@user2875994 Glad to help. Added a little bit more information.
Useful information! I think I might use the sqlcommandbuilder mentioned below, because the table variable will never be set by user input (it's used in a private method in a static class. I know it says public right now but that's just messing around). This class simply needs to fetch a lot of tables, so it'd be nice to not have to update a whitelist every time a new table is added. And as I mentioned it's not set by user input, so security isn't really a concern in that regard (but it's still nice to block injects because who knows, right?).
5

Using table's name as parameter is incorrect. Parameters in SQL just works for values not identifiers of columns or tables.

One option can be using SqlCommandBuilder Class, This will escape your table name and not vulnerable to SQL Injection:

SqlCommandBuilder cmdBuilder = new SqlCommandBuilder();
string tbName = cmdBuilder.QuoteIdentifier(tableName);

You can use the tbName in your statement because it's not vulnerable to SQL Injection now.

5 Comments

Thanks for the reply!
@user2875994 ... You're Welcome. Check my updated answer.
That's actually really useful! Thanks!
Addendum after playing around with it: When you run that sqlcommandbuilder code you pasted there, the "tbname" string is wrapped in ''s inside the string. So in your above example if tableName is generictable1, then tbName is 'generictable1'. However, it still finds the table, but it makes it a bit weird to test against a whitelist containing the allowed tables as well unless you either add the ''s to the whitelist, or remove them from the tbName string before you test against the whitelist, or you test the tableName variable against the whitelist and then use tbName.
I know that answer has been around for a while, but it references sqlcommandbuilder class which is for ms sql server, while, the question is about mysql. So, the answer should reference mysqlcommandbuilder class, see dev.mysql.com/doc/dev/connector-net/8.0/html/…

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.