Is this Vulnerable for SQL injection?..

  • Hi Everyone

    I hope everyone is having a nice day.

    This is my Code

    set ANSI_NULLS ON

    set QUOTED_IDENTIFIER ON

    go

    -- =============================================

    -- Author:<Author,,Name>

    -- Create date: <Create Date,,>

    -- Description:<Description,,>

    -- =============================================

    ALTER PROCEDURE [dbo].[SearchBiography]

    @firstname varchar(50),

    @middlename varchar(50),

    @lastname varchar(50),

    @sex varchar(50),

    @status varchar(50),

    @sexID int,

    @statusID int

    AS

    BEGIN

    SET NOCOUNT ON;

    DECLARE @sqlquery varchar(max) , @SqlQueryFirstName varchar(max),@SqlQueryMiddleName varchar(max), @SqlQueryLastName varchar(max), @SqlQuerySex varchar(max), @SqlQueryStatus varchar(max)

    SET @sqlquery = ''

    IF LEN(@sex) > 0

    SET @SqlQuerySex = ' AND sex like ''%' + @sex + '%'''

    ELSE

    SET @SqlQuerySex = ''

    IF LEN(@status) > 0

    SET @SqlQueryStatus = ' AND status like ''%' + @status + '%'''

    ELSE

    SET @SqlQueryStatus = ''

    IF LEN(@firstname) > 0

    SET @SqlQueryFirstName = ' AND firstname like ''%' + @firstname + '%'''

    ELSE

    SET @SqlQueryFirstName = ''

    IF LEN(@middlename) > 0

    SET @SqlQueryMiddleName = ' AND middlename like ''%' + @middlename + '%'''

    ELSE

    SET @SqlQueryMiddleName = ''

    IF LEN(@lastname) > 0

    SET @SqlQueryLastName =' AND lastname like ''%' + @lastname + '%'''

    ELSE

    SET @SqlQueryLastName = ''

    SET @sqlquery = 'SELECT * FROM TestMyView WHERE sexID = ' + convert(varchar(20), @sexID) + ' AND statusID = ' + convert(varchar(20), @statusID)

    SET @sqlquery = @sqlquery + @SqlQuerySex + @SqlQueryStatus + @SqlQueryFirstName + @SqlQueryMiddleName + @SqlQueryLastName

    EXEC (@SqlQuery) /* Should i need a parameter here??? */ please tell me :-(

    PRINT(@SqlQuery)

    END

  • YES!

    Anything that is Dynamic can be. Plus as everyone else has said, it is very poor practice. And poor performing.

    I wish that it were illegal for a front-end programmer to even open SQL Server. It isn't illegal for one to perform surgery on themselves. Why don't you try that next time you are ill?

    And you wonder why so many SQL DBAs are not being paid nearly enough, while having to deal with crap code that makes a SQL Server come to a crawl. And also given code like this to try and troubleshoot or change, all because the front-end programmer was fired.

    Andrew SQLDBA

  • AndrewSQLDBA (9/26/2013)


    YES!

    Anything that is Dynamic can be. Plus as everyone else has said, it is very poor practice. And poor performing.

    I wish that it were illegal for a front-end programmer to even open SQL Server. It isn't illegal for one to perform surgery on themselves. Why don't you try that next time you are ill?

    And you wonder why so many SQL DBAs are not being paid nearly enough, while having to deal with crap code that makes a SQL Server come to a crawl. And also given code like this to try and troubleshoot or change, all because the front-end programmer was fired.

    Andrew SQLDBA

    HAHAHAH..thanks andrew..

    can you "Re-Code" the above Code??...so that it will not be prone to sql injection...thanks andrew..

  • You should use sp_executesql with parameters.

    Have a look at The Curse and Blessings of Dynamic SQL for a bit more info.

  • The book by Denny Cherry on Securing SQL Server has a chapter on SQL Injection, it might be worth checking it out.

    Securing SQL Server, Second Edition: Protecting Your Database from Attackers

    Need an answer? No, you need a question
    My blog at https://sqlkover.com.
    MCSE Business Intelligence - Microsoft Data Platform MVP

Viewing 5 posts - 1 through 4 (of 4 total)

You must be logged in to reply to this topic. Login to reply