Short answer, no. The quoting trick is easily defeated by including your own closing quote and then a comment symbol to eliminate the final concatenated quote, precisely as in your example.
To protect yourself from SQL injection you must use bind variables. Changing your example to SELECT * FROM table WHERE ID = :X and then binding the user's input to X solves the problem instantly and completely. It is impossible to over-emphasize how important this practice is!
There is still a potential for injection here. Let's assume that:
- 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:
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:
Best Answer
you better use prepared statement from your programming source code, e.g. for PHP use PDO's prepare statement!
Check this - https://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php