CODEWITHSUNDEEP
sql-servert-sqlstored-proceduressql-injection
Steven Kuan
Steven Kuan

Reputation: 63

Stored procedure SQL Injection check

I wrote the following stored procedure for querying the database. Can someone tell me if this dynamic query stored procedure is vulnerable to a SQL injection attack?

If it is, how to modify the following code to prevent SQL injection attacks?

The second question is OPTION (RECOMPILE) at the end of the WHERE cause, is it necessary with every execution? 

CREATE PROCEDURE DataMapMainQuery
    (@DataMapID VARCHAR(MAX),
     @DataMapIDName VARCHAR(MAX),
     @StartIndex INT,
     @MaximumRows INT,
     @sortExpression VARCHAR(MAX))
AS
BEGIN   
    DECLARE @FilteredTotalRows AS INT
    DECLARE @SqlString NVARCHAR(MAX)
    DECLARE @WhereString1 NVARCHAR(MAX)
    DECLARE @WhereString2 NVARCHAR(MAX)

    IF (@DataMapID IS NULL)
        SET @WhereString1 = ' AND (DataMapID LIKE ' + '''%%''' + ' OR NULL IS NULL)'
    ELSE
        SET @WhereString1 = ' AND (DataMapID LIKE ' + '''%' + @DataMapID + '%''' + ' OR ''' + @DataMapID + ''' IS NULL)'

    IF (@DataMapIDName IS NULL)
        SET @WhereString2 = ' AND (DataMapIDName LIKE ' + '''%%''' + ' OR NULL IS NULL)'
    ELSE
        SET @WhereString2 = ' AND (DataMapIDName LIKE ' + '''%' + @DataMapIDName + '%''' + ' OR ''' + @DataMapIDName + ''' IS NULL)'

    IF (@sortExpression IS NULL)
        SET @sortExpression = 'DataMapID'

    SELECT
        @FilteredTotalRows = COUNT(*) 
    FROM
        DataMapMain
    WHERE
        1 = 1
        AND (DataMapID LIKE '%' + @DataMapID + '%' OR @DataMapID IS NULL)
        AND (DataMapIDName LIKE '%' + @DataMapIDName + '%' OR @DataMapIDName IS NULL)

    IF (@FilteredTotalRows < @StartIndex + 1)
    BEGIN
        SET @SqlString = '
                SELECT 
                    DataMapID, DataMapIDName,
                    DataMapGroup, DataMapGroupRemark,
                    CONVERT(BIGINT, TimeStamp) AS TimeStamp
                FROM 
                    (SELECT
                         ROW_NUMBER() OVER (ORDER BY ' + @sortExpression + ') AS RowNumber,
                         DataMapID, DataMapIDName,
                         DataMapGroup, DataMapGroupRemark,
                         TimeStamp
                     FROM
                         DataMapMain
                     WHERE
                         1 = 1' 
                    + @WhereString1
                    + @WhereString2
                    + ') DataMapMain
                WHERE
                     RowNumber >= 1 
                     AND RowNumber < (1 + ' + CONVERT(NVARCHAR(10), @MaximumRows) + ') 
                OPTION (RECOMPILE)'
    END
    ELSE
    BEGIN
        SET @SqlString = '
                SELECT 
                    DataMapID
                    ,DataMapIDName
                    ,DataMapGroup
                    ,DataMapGroupRemark
                    ,CONVERT(bigint, TimeStamp) as TimeStamp
                FROM 
                     (
                     Select ROW_NUMBER() over (order by '  + @sortExpression + ') as RowNumber
                    ,DataMapID
                    ,DataMapIDName
                    ,DataMapGroup
                    ,DataMapGroupRemark
                    ,TimeStamp
                     From DataMapMain
                     WHERE
                        1 = 1'
                    + @WhereString1
                    + @WhereString2

                        + ') DataMapMain
                WHERE
                    RowNumber >= (' +  CONVERT(nvarchar(10),@StartIndex) + ' + 1) and RowNumber < (' + CONVERT(nvarchar(10),@StartIndex) + ' + 1 + ' + CONVERT(nvarchar(10),@MaximumRows) + ' )
                    OPTION (RECOMPILE)'
            END 
            PRINT @SqlString
            PRINT @FilteredTotalRows
        EXEC sp_executesql @SqlString
    END

Upvotes: 2

Views: 609

Answers (4)

Steven Kuan
Steven Kuan

Reputation: 63

Thanks for all of your helps, I rewrote the code below. Please let me know if it is not OK. thank you all!

CREATE PROCEDURE DataMapMainQuery
    (@DataMapID VARCHAR(MAX),
     @DataMapIDName VARCHAR(MAX),
     @StartIndex INT,
     @MaximumRows INT,
     @sortExpression VARCHAR(MAX))
AS
BEGIN   
    DECLARE @FilteredTotalRows AS INT
    DECLARE @SqlString NVARCHAR(MAX)
    DECLARE @params NVARCHAR(MAX);
    DECLARE @WhereString1 NVARCHAR(MAX)
    DECLARE @WhereString2 NVARCHAR(MAX)

    IF (@DataMapID IS NULL)
        SET @WhereString1 = ' AND (DataMapID LIKE ' + '''%%''' + ' OR NULL IS NULL)'
    ELSE
        SET @WhereString1 = ' AND (DataMapID LIKE ' + '''%' + @DataMapID + '%''' + ' OR ''' + @DataMapID + ''' IS NULL)'

    IF (@DataMapIDName IS NULL)
        SET @WhereString2 = ' AND (DataMapIDName LIKE ' + '''%%''' + ' OR NULL IS NULL)'
    ELSE
        SET @WhereString2 = ' AND (DataMapIDName LIKE ' + '''%' + @DataMapIDName + '%''' + ' OR ''' + @DataMapIDName + ''' IS NULL)'

    IF (@sortExpression IS NULL)
        SET @sortExpression = 'DataMapID'

    SELECT
        @FilteredTotalRows = COUNT(*) 
    FROM
        DataMapMain
    WHERE
        1 = 1
        AND (DataMapID LIKE '%' + @DataMapID + '%' OR @DataMapID IS NULL)
        AND (DataMapIDName LIKE '%' + @DataMapIDName + '%' OR @DataMapIDName IS NULL)

    IF (@FilteredTotalRows < @StartIndex + 1)
    BEGIN
        SET @SqlString = '
                SELECT 
                    DataMapID, DataMapIDName,
                    DataMapGroup, DataMapGroupRemark,
                    CONVERT(BIGINT, TimeStamp) AS TimeStamp
                FROM 
                    (SELECT
                         ROW_NUMBER() OVER (ORDER BY ' + @sortExpression + ') AS RowNumber,
                         DataMapID, DataMapIDName,
                         DataMapGroup, DataMapGroupRemark,
                         TimeStamp
                     FROM
                         DataMapMain
                     WHERE
                         1 = 1' 
                    + @WhereString1
                    + @WhereString2
                    + ') DataMapMain
                WHERE
                     RowNumber >= 1 
                     AND RowNumber < (1 + ' + CONVERT(NVARCHAR(10), @MaximumRows) + ')'
    END
    ELSE
    BEGIN
        SET @SqlString = '
                SELECT 
                    DataMapID
                    ,DataMapIDName
                    ,DataMapGroup
                    ,DataMapGroupRemark
                    ,CONVERT(bigint, TimeStamp) as TimeStamp
                FROM 
                     (
                     Select ROW_NUMBER() over (order by '  + @sortExpression + ') as RowNumber
                    ,DataMapID
                    ,DataMapIDName
                    ,DataMapGroup
                    ,DataMapGroupRemark
                    ,TimeStamp
                     From DataMapMain
                     WHERE
                        1 = 1'
                    + @WhereString1
                    + @WhereString2

                        + ') DataMapMain
                WHERE
                    RowNumber >= (' +  CONVERT(nvarchar(10),@StartIndex) + ' + 1) and RowNumber < (' + CONVERT(nvarchar(10),@StartIndex) + ' + 1 + ' + CONVERT(nvarchar(10),@MaximumRows) + ' )'
            END

    SET @params = '
        @DataMapID      VARCHAR(MAX)
        ,@DataMapIDName     VARCHAR(MAX)
        ,@StartIndex    INT
        ,@MaximumRows   INT
        ,@sortExpression VARCHAR(MAX)';

    EXEC sp_executesql 
        @SqlString
        ,@params
        ,@DataMapID
        ,@DataMapIDName
        ,@StartIndex
        ,@MaximumRows
        ,@sortExpression;
    END

Upvotes: 0

Esat Erkec
Esat Erkec

Reputation: 1724

Adding OPTION(RECOMPILE) hint provides to rebuild a new execution plan for the query execution for every execution. Under some circumstances it can help to improve the performance. However the recompile operation uses memory and CPU resources in order to generate new execution plan. As a result, if you are not sure about the effects of the performance you don't use it

Upvotes: 0

gotqn
gotqn

Reputation: 43636

Just use sp_executesql with parameters. Build your dynamic T-SQL statements, but instead the value add @parameter_name. Then call the routine like this:

EXEC sp_executesql @sql
                  ,N'@parameter_name1 INT, @parameter_name2 VARCHAR(128), @parameter_name3 BIT'
                  ,@parameter_name1, @parameter_name2, @parameter_name3;

Upvotes: 1

Ed Bangga
Ed Bangga

Reputation: 13006

So far your @DataMapID and @DataMapName are safe because your building it first before applying in your main sql query. I would suggest adding these lines to check proper values of your sort expression, maxrows and start index

    IF (@sortExpression NOT IN ('ASC', 'DESC'))
    BEGIN
            RAISERROR('invalid order expression', 16,1);
            RETURN;
    END;


    IF (TRY_CAST(@StartIndex as int) = null or TRY_CAST(@MaximumRows as int) = null)
    BEGIN
            RAISERROR('invalid startindex or maximum rows', 16,1);
            RETURN;
    END;

Upvotes: 0

Related Questions