0

I've developed a stored procedure that utilizes dynamic SQL to search for specific entities (this is a procedure for catch all logic). Is this procedure prone to SQL injection, and how do I get the final query that is being executed sp_executesql i.e after the values are substituted?

Here when I use print @sql - it just gives me the query with parameter names but not the actual values. Thanks for your time!

CREATE OR ALTER PROCEDURE [dbo].[usp_SearchEntities]
    (@customerId      UNIQUEIDENTIFIER = NULL,
     @startRowIndex   BIGINT = 0,
     @pageSize        BIGINT = 20,
     @orderByColumn   NVARCHAR(100) = NULL,
     @orderDirection  NVARCHAR(10) = 'DESC',
     @statusCode      TINYINT = NULL,
     @regionId        UNIQUEIDENTIFIER = NULL,
     @entityName      NVARCHAR(254) = NULL,
     @identifier      NVARCHAR(25) = NULL,
     @outResultStatus NVARCHAR(100) = NULL OUTPUT)
AS 
BEGIN
    /* Turn off row counts */
    SET NOCOUNT ON;

    DECLARE @sql NVARCHAR(MAX);

    SET @sql = '
    SELECT e.Id, ev.EntityName
    FROM dbo.CustomEntities e 
    INNER JOIN dbo.CustomEntityVersions ev 
        ON e.ActiveEntityVersionId = ev.Id
    INNER JOIN dbo.CustomLocations l 
        ON e.LocationId = l.Id
    INNER JOIN dbo.CustomRegions r 
        ON l.RegionId = r.Id
    WHERE e.CustomerId = @customerId 
        AND e.IsDisabled = 0';

    -- Conditionally append WHERE clauses based on parameters
    IF @entityName IS NOT NULL
        SET @sql = @sql + N' AND ev.EntityName LIKE ''%'' + @entityName + ''%''';

    IF @regionId IS NOT NULL
        SET @sql = @sql + N' AND l.RegionId = @regionId';

    IF @statusCode IS NOT NULL
        SET @sql = @sql + N' AND ev.StatusCode = @statusCode';

    IF @identifier IS NOT NULL
        SET @sql = @sql + ' AND ev.Identifier LIKE ''%'' + @identifier + ''%''';

    -- Construct ORDER BY clause based on specific mappings
    IF      @orderByColumn = 'LASTUPDATED'       SET @sql = @sql + ' ORDER BY ev.LastUpdatedDate ' + CASE WHEN @orderDirection = 'ASC' THEN 'ASC' ELSE 'DESC' END
    ELSE IF @orderByColumn = 'REGIONID'          SET @sql = @sql + ' ORDER BY r.RegionName ' + CASE WHEN @orderDirection = 'ASC' THEN 'ASC' ELSE 'DESC' END      
    ELSE IF @orderByColumn = 'LOCATIONID'        SET @sql = @sql + ' ORDER BY l.LocationCode ' + CASE WHEN @orderDirection = 'ASC' THEN 'ASC' ELSE 'DESC' END
    ELSE IF @orderByColumn = 'STATUSCODE'        SET @sql = @sql + ' ORDER BY ev.StatusCode ' + CASE WHEN @orderDirection = 'ASC' THEN 'ASC' ELSE 'DESC' END   
    ELSE IF @orderByColumn = 'IDENTIFIER'        SET @sql = @sql + ' ORDER BY ev.Identifier ' + CASE WHEN @orderDirection = 'ASC' THEN 'ASC' ELSE 'DESC' END   
    ELSE IF @orderByColumn = 'RELEASEDATE'       SET @sql = @sql + ' ORDER BY ev.ReleaseDate ' + CASE WHEN @orderDirection = 'ASC' THEN 'ASC' ELSE 'DESC' END
    ELSE IF @orderByColumn = 'ENTITYNAME'        SET @sql = @sql + ' ORDER BY ev.EntityName '       
    ELSE SET @sql = @sql + ' ORDER BY ev.LastUpdatedDate ' + CASE WHEN @orderDirection = 'ASC' THEN 'ASC' ELSE 'DESC' END

    -- Append ORDER BY, OFFSET, and FETCH
    SET @sql = @sql + ' OFFSET @startRowIndex ROWS FETCH NEXT @pageSize ROWS ONLY';

    -- Execute the SQL
    EXEC sp_executesql 
        @sql, 
        N'@customerId UNIQUEIDENTIFIER, @regionId UNIQUEIDENTIFIER, @statusCode TINYINT, @entityName NVARCHAR(254), @identifier NVARCHAR(25), @startRowIndex INT, @pageSize INT, @orderDirection NVARCHAR(10)',
        @customerId       = @customerId,
        @regionId         = @regionId,
        @statusCode       = @statusCode,
        @entityName       = @entityName,
        @identifier       = @identifier,
        @startRowIndex    = @startRowIndex,
        @pageSize         = @pageSize,
        @orderDirection   = @orderDirection;

    -- Print the generated SQL for debugging
    PRINT @sql;

    RETURN 0;
END 
7
  • 2
    It looks good to me, well done! Commented Jun 17, 2024 at 15:08
  • 2
    The fact that you can't print out the final SQL is a good sign. Essentially this gets submitted in two chunks. The first submission is the SQL with the parameters in it (the @sql print out). The database gets that and creates an execution plan, just like any sql statement. Then the second chunk is sent with values for the parameters and the execution is ran. So you shouldn't ever see SQL with the actual values present inside of it; if you did, then that would be cause for sql-injection concern. Commented Jun 17, 2024 at 15:09
  • 3
    @lifeisajourney the final SQL is whatever the @sql variable contains. Parameters are never part of the query itself. The database compiles the query into an execution plan and executes that using the parameter values Commented Jun 17, 2024 at 15:13
  • 1
    You could have an output variable to spit the result-set out after execution, like described here Commented Jun 17, 2024 at 15:14
  • 2
    There is, in truth, no injection here. I jection is when you injecy values from the outer scope into the inner (dynamic) scope, such as N'... AND ' + @MyVariable + N' = 123'; that isn't what you are doing here, you're parameterising. Commented Jun 17, 2024 at 15:19

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.