SQL DDL code to avoid SQL Injection Attacks

Question

The following piece of code is being highlighted as a security vulnerability to SQL injection attacks.

StringBuilder sb = new StringBuilder();
sb.Append("DROP DATABASE IF EXISTS " + dbname);

String **sqlCommText** = sb.ToString();

using (SqlCommand command = new SqlCommand(**sqlCommText**, connection))
{

   connection.Open();

Namely the sqlCommText

I'm aware of creating prepared statements on DML sql like insert and updates but i dont think this works on DDL sql - i cant parameterize the dbname into the sql.

Any suggestions how this should be fixed?

Don't trust dbname - use a dictionary of sorts of predefined/known database names. Otherwise, select a list of databases and match the name in memory and then delete — Camilo Terevinto
– Camilo Terevinto, Commented Aug 4, 2021 at 14:25
Executing DDL in general is a security concern and needs to be done only from trusted code and data sources. — Dan Guzman
– Dan Guzman, Commented Aug 4, 2021 at 14:26
Well you are dropping a database, what's the worse thing that a SQL Injector could do with this? Dropping an entire database is already potentially pretty bad. They don't have to inject it, just changing the name to the wrong database is pretty disastrous on its own. — RBarryYoung
– RBarryYoung, Commented Aug 4, 2021 at 14:30
Considering that something with the ability to DROP a database tends to have quite high permissions, @RBarryYoung , I can think of a few other things they could do. — Thom A
– Thom A ♦, Commented Aug 4, 2021 at 14:32
@Larnu Dropping the wrong database is already a disaster. Preventing injection isn't nearly sufficient to make this code "safe". My point being that preventing injection here is pretty easy. Preventing dropping the wrong database is another story and requires more information than we have. — RBarryYoung
– RBarryYoung, Commented Aug 4, 2021 at 14:35

RBarryYoung · Accepted Answer · 2021-08-04 15:45:34Z

5

Preventing SQL Injection here is pretty easy: query the names of all of the databases on the SQL Server like this:

SELECT name, QUOTENAME(name) as QName 
FROM sys.databases 
WHERE database_id > 4;

Then, before you execute the rest of your SQL code, just check that your dbname variable is in that list of names returned by the query and if it is, then use the corresponding QName in your query (this protects against odd characters in the database name and also against something called latent injection).

However, as I mentioned in the comments, stopping injection is the easy problem. The hard problem here is to make sure that that legitimate users do not accidentally drop the wrong database, and worse, that bad actors do not intentionally drop the wrong database(s).

edited Aug 4, 2021 at 15:45

answered Aug 4, 2021 at 14:49

RBarryYoung

57k15 gold badges99 silver badges142 bronze badges

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

3 Comments

ChrisBD Over a year ago

Couldn't you restrict the application to a role that only allows for execution of stored procedures and thus restrict which database could be dropped as it could be specified within the stored procedure and not passed in by the code?

RBarryYoung Over a year ago

@ChrisBD Yes, exactly. However, the problem still remains of how to specify which DBs can and cannot be dropped. It's not a simple or easy question and definitely requires more context/information than we currently have.

m_finn Over a year ago

Thanks all for your input. This isnt my code, I've been asked to investigate the security concerns. I think it will be best to go back and question why this is done this way in the first place and maybe consider an alternative solution. Excellent answer and insight @RBarryYoung

Collectives™ on Stack Overflow

SQL DDL code to avoid SQL Injection Attacks

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related