How do I protect this function from SQL injection?

Question

public static bool TruncateTable(string dbAlias, string tableName)
{
    string sqlStatement = string.Format("TRUNCATE TABLE {0}", tableName);
    return ExecuteNonQuery(dbAlias, sqlStatement) > 0;
}

Answer 1

The most common recommendation to fight SQL injection is to use an SQL query parameter (several people on this thread have suggested it).

This is the wrong answer in this case. You can't use an SQL query parameter for a table name in a DDL statement.

SQL query parameters can be used only in place of a literal value in an SQL expression. This is standard in every implementation of SQL.

My recommendation for protecting against SQL injection when you have a table name is to validate the input string against a list of known table names.

You can get a list of valid table names from the INFORMATION_SCHEMA:

SELECT table_name 
FROM INFORMATION_SCHEMA.Tables 
WHERE table_type = 'BASE TABLE'
  AND table_name = @tableName

Now you can pass your input variable to this query as an SQL parameter. If the query returns no rows, you know that the input is not valid to use as a table. If the query returns a row, it matched, so you have more assurance you can use it safely.

You could also validate the table name against a list of specific tables you define as okay for your app to truncate, as @John Buchanan suggests.

Even after validating that tableName exists as a table name in your RDBMS, I would also suggest delimiting the table name, just in case you use table names with spaces or special characters. In Microsoft SQL Server, the default identifier delimiters are square brackets:

string sqlStatement = string.Format("TRUNCATE TABLE [{0}]", tableName);

Now you're only at risk for SQL injection if tableName matches a real table, and you actually use square brackets in the names of your tables!

Answer 2

As far as I know, you can't use parameterized queries to perform DDL statements/ specify table names, at least not in Oracle or Sql Server. What I would do, if I had to have a crazy TruncateTable function, that had to be safe from sql injection would be to make a stored procedure that checks that the input is a table that is safe to truncate.

-- Sql Server specific!
CREATE TABLE TruncableTables (TableName varchar(50))
Insert into TruncableTables values ('MyTable')

go

CREATE PROCEDURE MyTrunc @tableName varchar(50)
AS
BEGIN

declare @IsValidTable int
declare @SqlString nvarchar(50)
select @IsValidTable = Count(*) from TruncableTables where TableName = @tableName

if @IsValidTable > 0
begin
 select @SqlString = 'truncate table ' + @tableName
 EXECUTE sp_executesql @SqlString
end
END

Answer 3

If you're allowing user-defined input to creep into this function via the tablename variable, I don't think SQL Injection is your only problem.

A better option would be to run this command via its own secure connection and give it no SELECT rights at all. All TRUNCATE needs to run is the ALTER TABLE permission. If you're on SQL 2005 upwards, you could also try using a stored procedure with EXECUTE AS inside.

Answer 4

CREATE OR REPLACE PROCEDURE truncate(ptbl_name IN VARCHAR2) IS
  stmt VARCHAR2(100);
BEGIN
  stmt := 'TRUNCATE TABLE '||DBMS_ASSERT.SIMPLE_SQL_NAME(ptbl_name);
  dbms_output.put_line('<'||stmt||'>');
  EXECUTE IMMEDIATE stmt;
END;

Answer 5

Use a stored procedure. Any decent db library (MS Enterprise Library is what I use) will handle escaping string parameters correctly.

Also, re:parameterized queries: I prefer to NOT have to redeploy my app to fix a db issue. Storing queries as literal strings in your source increases maintenance complexity.

Answer 6

Have a look at this link

Does this code prevent SQL injection?

Remove the unwanted from the tableName string.

I do not think you can use param query for a table name.

Answer 7

There are some other posts which will help with the SQL injection, so I'll upvote those, but another thing to consider is how you will be handling permissions for this. If you're granting users db+owner or db_ddladmin roles so that they can truncate tables then simply avoiding standard SQL injection attacks isn't sufficient. A hacker can send in other table names which might be valid, but which you wouldn't want truncated.

If you're giving ALTER TABLE permissions to the users on the specific tables that you will allow to be truncated then you're in a bit better shape, but it's still more than I like to allow in a normal environment.

Usually TRUNCATE TABLE isn't used in normal day-to-day application use. It's used for ETL scenarios or during database maintenance. The only situation where I might imagine it would be used in a front-facing application would be if you allowed users to load a table which is specific for that user for loading purposes, but even then I would probably use a different solution.

Of course, without knowing the specifics around why you're using it, I can't categorically say that you should redesign, but if I got a request for this as a DBA I'd be asking the developer a lot of questions.

Answer 8

Use parameterized queries.

Answer 9

In this concrete example you need protection from SQL injection only if table name comes from external source.

Why would you ever allow this to happen? If you are allowing some external entity (end user, other system, what?) to name a table to be dropped, why won't you just give them admin rights.

If you are creating and removing tables to provide some functionality for end user, don't let them provide names for database objects directly. Apart from SQL injection, you'll have problems with name clashes etc. Instead generate real table names yourself (e.g DYNTABLE_00001, DYNTABLE_00002, ...) and keep a table that connects them to the names provided by user.

---

Some notes on generating dynamic SQL for DDL operations:

• In most RDBMS-s you'll have to use dynamic SQL and insert table names as text. Be extra careful.

• Use quoted identifiers ([] in MS SQL Server, "" in all ANSI compliant RDBMS). This will make avoiding errors caused by invalid names easier.

• Do it in stored procedures and check if all referenced objects are valid.

• Do not do anything irreversible. E.g. don't drop tables automatically. You can flag them to be dropped and e-mail your DBA. She'll drop them after the backup.

• Avoid it if you can. If you can't, do what you can to minimize rights to other (non-dynamic) tables that normal users will have.

Answer 10

You could use SQLParameter to pass in tableName value. As far as I know and tested, SQLParameter takes care of all parameter checking and thus disables possibility of injection.

Answer 11

If you can't use parameterized queries (and you should) ... a simple replace of all instances of ' with '' should work.

string sqlStatement = string.Format("TRUNCATE TABLE {0}", tableName.Replace("'", "''"));