3

I've found some exec(sql) statements buried in the code. They're there for good reason, because these statements couldn't be written directly, however they are an obvious attack vector.

Is there a safe alternative to exec(some sql)? Something that will be paremetrised correctly, including table names in statements?

8

For certain types of parameterization, it's not going to matter if you use EXEC() or sp_executesql, because some things can't be parameterized anyway. For example, you expressed in the comments (please update your question to be more specific about your requirements!) that you are parameterizing table names, but these can't be parameterized because they need to expressed literally to SQL Server (it can't tokenize that and swap at runtime).

To protect yourself from table name vulnerabilities, you can easily protect yourself like this:

DECLARE @tablename sysname; -- pretend this is a parameter
SET @tablename = N'this_is_not_a_table';

-- in procedure body:
IF NOT EXISTS (SELECT 1 FROM sys.tables WHERE name = @tablename)
BEGIN
  RAISERROR(N'Table does not exist.', 1, 11);
  RETURN;
END
SET @sql = N'SELECT ... FROM dbo.' + QUOTENAME(@tablename) + ...;
-- EXEC(@sql) or EXEC sys.sp_executesql @sql

Now, I prefer to use sp_executesql always, partly because it promotes using strongly-typed parameters (avoiding SQL injection issues as well as double-single-quote issues), but also because in some cases you will be passing parameters for both parameterizable and unparameterizable values (yes I made those words up). More info:

Now the only thing you have to worry about is if someone is able to create tables and can create a table named sys.objects; DROP TABLE foo; -- - but if you have someone you don't trust but has the ability to create tables in your database...

  • 2
    If you use quotename on that table, I think it is safe even with a name like drop. – jmoreno Oct 17 '16 at 22:18
  • Seems silly that you have to parameterize on table. You could just partition your data in the same table with a surrogate key instead.... – Aron Oct 18 '16 at 3:11
  • @jmoreno Of course, thanks, I harp on that all the time. – Aaron Bertrand Oct 18 '16 at 3:58
  • 1
    @Aron Sure, in an ideal world we would never need dynamic SQL at all. Then there's reality, where we don't have control over the table design and the set of tables/columns being queried. – Aaron Bertrand Oct 18 '16 at 3:59
  • That is why one should introduce the table designers to my friend Bobby Tables – Aron Oct 18 '16 at 4:06
3

Take a look at using sp_executeSQL instead. It is fully paramaterized and so provides a much greater sense of safety to dynamic queries (and using alongside EXECUTE AS USER within procedures with strict permission sets make it even better).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.