Sql-server – Preventing SQL Injection

sql serversql-injection

I know that this is very dangerous:

EXEC sp_executesql 'SELECT * FROM ' + @Table

But, is this safe from SQL injection?

IF EXISTS(SELECT * FROM information_schema.tables WHERE TABLE_NAME = @Table) BEGIN
    EXEC sp_executesql 'SELECT * FROM ' + @Table
END

Best Answer

There is still a potential for injection here. Let's assume that:

  1. someone has the ability to create tables in this database, but not drop them
  2. that someone has the ability to call this procedure
  3. the procedure runs as an elevated user

That person could call this procedure to get results from foo, but in the meantime create a table called [foo;drop table bar], then pass foo;drop table bar in as the parameter value. It would pass your check, then you would blindly execute it. They get the results from foo but they also drop table bar.

Now, that is a pretty contrived scenario, and you probably aren't worried about internal people dropping tables, but they could also use this to exploit data they don't have access to, for example they could create a table called [foo;SELECT name,salary FROM dbo.Employees;] and end up with two result sets, one with data you probably shouldn't be letting them see.

There is also a potential for getting data from the wrong table, in the event that there are tables with the same names in different schemas. Always, always, always use the schema prefix when creating or referencing objects:

Finally, don't use INFORMATION_SCHEMA. The catalog views and metadata functions are much more complete, current and reliable:

I would probably do it this way (and I'm going to assume for simplicity that all of your tables are in dbo):

DECLARE @table SYSNAME; -- procedure parameter, right?

DECLARE @sql NVARCHAR(MAX);
IF OBJECT_ID(N'dbo.' + QUOTENAME(@table), N'U') IS NOT NULL
BEGIN
  SET @sql = N'SELECT * FROM dbo.' + QUOTENAME(@table) + ';';
  EXEC sp_executesql @sql;
END

This way, even if they pass foo;drop table bar or foo;select name,salary from dbo.employees as the table name, that whole string gets surrounded in square brackets, thus the worst thing that can happen is they'll get the results from the table they created, rather than any other table or worse.

And I would ensure that the procedure runs as that user, rather than escalate using EXECUTE AS (a common bypass for proper permissions configuration). Also see:

Related Question