0

I am working with C#. I need to write a select inline query.

The table name should be taken from config. I cannot write a stored procedure.

SqlCommand myCommand= new SqlCommand();
myCommand.CommandText = "Select  * from " + tableName;
myCommand.CommandType = CommandType.Text;
myCommand.Connection = connString;

How to avoid sql injection ?

Yahia
  • 69,653
  • 9
  • 115
  • 144
user386258
  • 1,933
  • 8
  • 22
  • 28
  • 2
    If the name is to be taken from a configuration file, then make sure that you lock down the configuration file. If only a specific user can edit the configuration file, then the only chance for injection is from that specific user. – Michael Todd Jan 21 '12 at 06:32
  • 1
    Just curious: why do you have a tablename in a config file? – Mitch Wheat Jan 21 '12 at 06:55
  • 1
    See [Sanitize table/column name in Dynamic SQL in .NET? (Prevent SQL injection attacks)](http://stackoverflow.com/a/12629168/590956) – Sam May 29 '13 at 16:37

4 Answers4

3

Just create a query with a real param and check for the existence of the tablename - somthing like:

SELECT COUNT(*) FROM SYS.TABLES WHERE NAME = @pYOURTABLENAME

IF that returns 1 then you know that the table exists and thus can use it in the SELECT you showed in the question...

However I strongly recommend to try anything to get rid of the need for any code prone to SQL injection!

Yahia
  • 69,653
  • 9
  • 115
  • 144
1

I would ensure table name contains only these characters:

ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz[]. -_0123456789

E.g.,

Regex regex = new Regex(@"^[ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz\[\]. -_0123456789]{1,128}$");
if (!regex.IsMatch(tableName)) throw new ApplicationException("Invalid table name");

To do a more comprehensive job including non-English languages see this reference on what a valid table names: http://msdn.microsoft.com/en-us/library/ms175874.aspx

Jay Sullivan
  • 17,332
  • 11
  • 62
  • 86
mike nelson
  • 21,218
  • 14
  • 66
  • 75
0

You need to verify that tableName is appropriate. After some sanity checking (making sure it has no spaces, or other disallowed characters for table names, etc), I would then first query the database for the names of all tables, and programmatically verify that it is one of those table names. Then proceed to run the query you show.

Jonathon Reinhart
  • 132,704
  • 33
  • 254
  • 328
  • 1
    Like I said, query the database for the names of *all* tables, and then use C# to verify that the user-provided name is in that result. – Jonathon Reinhart Jan 21 '12 at 06:37
  • By doing sanity checks. Make sure only specific characters are in the table name. This is not about "does the table exist" but "is the string in the config really the name of a table or some other crapy oudont want o see the sql server". This is a beginner programmer exercise in string validation. – TomTom Jan 21 '12 at 06:39
  • @Michael Todd you shouldn't have deleted your comment that asked (paraphrased) "How can the user query the database for the table name without including the table name in *that* query" – Jonathon Reinhart Jan 21 '12 at 06:40
  • @Michael, I agree generally, but in that case, I thought it was a valid question that future readers may ask in their heads while reading. – Jonathon Reinhart Jan 22 '12 at 16:31
0

I'd look at moving the SQL to a stored proc and review this article by Erland Sommarskog as it has some great ideas for using dynamic SQL within stored procs. I'd certainly look into it. It discusses a lot of the issues around SQL injection and possible alternatives to dynamic SQL.

He also has another great article on ways to use arrays in stored procs. I know you didn't ask for that, but I often refer to these two articles as I think they are quite insightful and provide you with some useful ideas with regards to writing your procedures.

In addition to some of the suggestions linked above, I still have some basic parameter sanitisation mechanisms that I use if I am ever using dynamic SQL. An example of this is as follows;

        IF LEN(@TableName) < 5 OR LEN(@TableDisplayName) < 5
    BEGIN
        RAISERROR('Please ensure table name and display name are at least 5 characters long', 16, 1)
    END

    IF NOT (@TableName not like '%[^A-Z]%') 
    BEGIN
        RAISERROR('The TableName can only contain letters', 16, 1)
    END

    IF NOT (@TableDisplayName not like '%[^0-9A-Z ]%') 
    BEGIN
        RAISERROR('The TableDisplayName can only contain letters, numbers or spaces', 16, 1)
    END

This combined with using parameters within your dynamic sql and then executing using sp_executesql certainly help to minimise the possibility of a SQL injection attack.

Mr Moose
  • 5,946
  • 7
  • 34
  • 69