Sql-server – SQL Server Extend dynamic SQL query with where part fetched from Stored Procedure

dynamic-sqlextended-stored-proceduresql serverstored-procedureswhere

I have a stored procedure that creates a dynamic SQL Query. I use sp_executesql, to have it SQL injection safe.

I have a second stored procedure that generates a where logic.
For example if I run the second procedure EXECUTE dbo.getWhereLogic @FilterID, @parOutput = @WhereLogic OUTPUT it will return as output (1 = 1)
I want to extend my first stored procedure with this where logic.

Let's assume that @SQLQuery = "Select * FROM Blabla Where 1 = 1" and @WhereLogic = (1=1)

If I do it like this:
SET @SQLQuery = @SQLQuery + ' AND ' + @WhereLogic
It will work, but it is not SQL injection safe.

If I do it like this:
SET @SQLQuery = @SQLQuery + ' AND @WhereLogic'

I get an error that there is a non-boolean type defined. There is a comparison missing. I've tried a couple of possibilities.

I think running the second procedure with sp_executesql will have no effect. That procedure returns the WhereLogic as varchar without problems, even if I simulate an SQL injection attack.

The two procedures are too long and complex to post it here. The main problem is also without code example clear.

To put the second procedure in the first and have a logical where query would be a possibility, but then I have to re write the second stored procedure and it was a lot of work 🙁 This should be the last option.

If someone has faced this problem already, I would appreciate an answer.
Links to other posts would be helpfull too.
Thanks in advance!

EDIT:
I try to create later a dbfiddle example.

Best Answer

Typical things you can make SQL injection safe:

  1. Values

    DECLARE @UserSuppliedValue varchar(32) = 'foo';
    
    DECLARE @sql nvarchar(max) = N'SELECT ... FROM dbo.bar
      WHERE value = @value;';
    EXEC sys.sp_executesql @sql, N'@value', @UserSuppliedValue;
    
  2. Entities, like table/column names

    DECLARE @UserSuppliedTable  = N'dbo.bar',
            @UserSuppliedColumn = N'foo';
    
    IF EXISTS -- make sure table exists
    (
      SELECT 1 FROM sys.tables
        WHERE [object_id] = OBJECT_ID(@UserSuppliedTable)
    )
    AND EXISTS -- or make sure both exist
    (
      SELECT 1 FROM sys.columns
        WHERE [object_id] = OBJECT_ID(@UserSuppliedTable)
          AND name = @UserSuppliedColumn
    )
    BEGIN
      DECLARE @sql nvarchar(max) = N'SELECT '
        + QUOTENAME(@UserSuppliedColumn) + N' FROM '
        + @UserSuppliedTable + N';';
      EXEC sys.sp_executesql @sql;
    END
    

I listed both there for illustration but you would only need one or the other in most scenarios.

If you're trying to pass an entire WHERE clause to a dynamic SQL statement, there's little you can do to make that injection safe, except maybe try to execute it with PARSEONLY to make sure it's not garbage. That doesn't help prevent valid SQL injection commands but it does at least help prevent utter garbage.

DECLARE @where nvarchar(max) = N' WHERE junk;'

DECLARE @sql  nvarchar(max) = N'SELECT * FROM sys.objects ' + @where;
DECLARE @test nvarchar(max) = N'SET PARSEONLY ON;' + @sql;

DECLARE @hr int;
BEGIN TRY
  EXEC @hr = sys.sp_executesql @test;
  EXEC sys.sp_executesql @sql;
END TRY
BEGIN CATCH
  PRINT N'Statement was garbage.';
END CATCH;

If @where were the following...

DECLARE @where nvarchar(max) = N' WHERE 1=1; DROP TABLE dbo.bobby;';

...then this parsing trick doesn't work (since parsing just validates syntax, not object existence or permissions).

So you need to put proper logic into the stored procedure that produces @where and make sure there are appropriate controls, permissions, and auditing around that stored procedure, as well as reducing the permissions of the user/group executing the final @sql - this should not be sa, for example.

I've written a little about this topic: