1

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.

  • 1
    Could you try to build a reduced and simplified version of your procedure? Use some fiddle like dbfiddle.uk – McNets Jan 15 '18 at 9:40
  • Lets assume the first quers is "SELECT * FROM Blabla Where 1 = 1" and the WhereLogic is (1=1) – 1yunuz Jan 15 '18 at 9:48
  • No, I think you don't understand the problem. How can I pass SQL Injection safe a "Where Logic" that is in a variable. That is the question. There is nothing else. Please read my text carefully. – 1yunuz Jan 15 '18 at 13:39
  • 2
    You can make a value SQL injection safe, and even an entity, but you can't make a clause SQL injection safe, without putting all kinds of controls around where the clause is generated. If you have a black box stored procedure that just outputs a where clause, your protection is the logic inside the stored procedure, because all you can do on your end is decide whether or not to append that clause to your query. – Aaron Bertrand Jan 15 '18 at 14:32
3

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:

  • Thank you for your detailed answer. I will take a deeper look into users and groups sections. For a quick and dirty solution I will skip the @where part, if there is some suspicious character in there. – 1yunuz Jan 15 '18 at 19:50
0

Given the constraints, I think the only solution is to use SQL Security to protect against SQL injection attacks. You only need to prevent SQL injection from untrusted users.

So long as the procedures that accepts the WHERE clause can't be called by regular users, and you properly handle user-supplied strings, it should be safe.

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.