4

While parameter is the best way to guard against Sql injection, there are times which we can't use it while building dynamic query. For example Table/Column/Index names cannot be passed in as parameter but only plain Text.

It seems like

SqlCommandBuilder.QuoteIdentifier

is the only option that I can find. Is calling this method enough to protect ourselves?

MSDN DOC:

Given an unquoted identifier in the correct catalog case, returns the correct quoted form of that identifier. This includes correctly escaping any embedded quotes in the identifier.

For example is

"Select * FROM " + SqlCommandBuilder.QuoteIdentifier("CustomTable" + userInputText);

safe to do?

Edit: The query is just an example. I am interested in finding out if Sql injection is ever possible.

Steve
  • 11,696
  • 7
  • 43
  • 81

5 Answers5

2

That may not be not safe to do.

To protect against any possible security problem with the SqlCommandBuilder.QuoteIdentifier method all that you need to do is get a list of the available table names etc. from the database and validate the user input against them.

Edited to add: I have reason to doubt if QuoteIdentifier is completely safe: the documentation for the SqlCommandBuilder.QuoteIdentifier Method says (as you previously quoted):

Given an unquoted identifier in the correct catalog case, returns the correct quoted form of that identifier. This includes correctly escaping any embedded quotes in the identifier.

Nowhere in that documentation does it state what happens if it is given an unquoted identifier in the wrong catalog case (whatever a "catalog case" is). Or what happens if the identifier is longer than the maximum allowed. Of course, undefined behaviour cannot be relied on.

Community
  • 1
  • 1
Andrew Morton
  • 24,203
  • 9
  • 60
  • 84
  • can you provide an example showing how injection can be done against this? – Steve Nov 16 '16 at 20:39
  • @Steve No, I can't, but there are some intricate examples of SQL injection attacks out there. I edited my answer to mention a fool-proof method. – Andrew Morton Nov 16 '16 at 20:45
2

It won't protect you from the attacker going to tables you don't want them to.

such as SQL system tables...

johng
  • 171
  • 9
  • that was just an example. I am just trying to find out if sql injection is possible at all – Steve Nov 16 '16 at 20:46
  • Isn't that what permissions are for? For that matter, any SQL query doesn't protect against the user accessing tables he should see (again permissions...) – Developer Webs Feb 05 '20 at 15:07
0

Seems safe to do to me. Also, this might be a good reference:

Sanitize table/column name in Dynamic SQL in .NET? (Prevent SQL injection attacks)

Community
  • 1
  • 1
Boggy
  • 11
  • 1
  • table name might contain special characters so I don't think sanitizing is a good option – Steve Nov 16 '16 at 20:35
0

Using user input in table name fields is never safe no matter how much you try to check it (unless you restrict the entries to a limited set of names or do some other kind of sorcery).

Even removing quotes, the user could type: TableName; DROP DATABASE db; or (SELECT * FROM <sensible table).

Possible solutions would be:

  • Using a ComboBox or an equivalent where the user can't modify the options
  • Check the input against a String[] with all allowed table names (where they should be identical to one of the entries)

and if the user input as a table name was just an example but you're going to use the input as part of a WHERE clause of a SELECT, then you should check Bobby Tables.

Quoted from the website:

From the C# Online wiki page ASP.NET Security Hacks--Avoiding SQL Injection

SqlCommand userInfoQuery = new SqlCommand(
    "SELECT id, name, email FROM users WHERE id = @UserName",
    someSqlConnection);

SqlParameter userNameParam = userInfoQuery.Parameters.Add("@UserName",
    SqlDbType.VarChar, 25 /* max length of field */ );

// userName is some string valued user input variable
userNameParam.Value = userName;

Or simpler:

String username = "joe.bloggs";
SqlCommand sqlQuery = new SqlCommand("SELECT user_id, first_name,last_name FROM users WHERE username = ?username",  sqlConnection);
sqlQuery.Parameters.AddWithValue("?username", username);
GGG
  • 640
  • 1
  • 9
  • 27
  • If a ComboBox was used it would still be vital to validate the selected value as there are ways to modify it. – Andrew Morton Nov 16 '16 at 20:59
  • @AndrewMorton That's why I suggested also using a `String[]` with allowed values, but if you start thinking that way, it could still be broken if the user tampered with the Assembly and removed the validation. – GGG Nov 16 '16 at 21:00
0

Have you considered using OData? you can pass in text, selecting tables, indexes and so on. But with OData you select what tables you want to publish this way, and it can't be injection attacked as you have to explicitly allow update and insert operations.

http://www.odata.org/documentation/odata-version-2-0/uri-conventions/ https://www.asp.net/web-api/overview/odata-support-in-aspnet-web-api

johng
  • 171
  • 9