patternsqlMinor
Preventing SQL Injection
Viewed 0 times
sqlpreventinginjection
Problem
I know that this is very dangerous:
But, is this safe from SQL injection?
EXEC sp_executesql 'SELECT * FROM ' + @TableBut, is this safe from SQL injection?
IF EXISTS(SELECT * FROM information_schema.tables WHERE TABLE_NAME = @Table) BEGIN
EXEC sp_executesql 'SELECT * FROM ' + @Table
ENDSolution
There is still a potential for injection here. Let's assume that:
That person could call this procedure to get results from
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
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
I would probably do it this way (and I'm going to assume for simplicity that all of your tables are in dbo):
This way, even if they pass
And I would ensure that the procedure runs as that user, rather than escalate using
- someone has the ability to create tables in this database, but not drop them
- that someone has the ability to call this procedure
- 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:
- Bad habits to kick : Avoiding the schema prefix
Finally, don't use
INFORMATION_SCHEMA. The catalog views and metadata functions are much more complete, current and reliable:- The case against INFORMATION_SCHEMA views
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;
ENDThis 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:- Bad habits to kick : Using SELECT *
Code Snippets
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;
ENDContext
StackExchange Database Administrators Q#71616, answer score: 5
Revisions (0)
No revisions yet.