Assigining value to parameter using variable in stored procedure in SQL Server

Question

I have a simple stored procedure with one parameter @Name which I want to replace with another variable.

I am actually looking for SQL injection character and if name contains -- then it should replace it with blank. The stored procedure shown below, it is executing without an error, but not replacing the string for example let is say user searches for EXEC John'''select * FROM TEST2 -- which has SQL injection statement in it

CREATE PROCEDURE GetStudentDetails
    @Name nvarchar(300)
AS
BEGIN
    SET NOCOUNT ON;

    SELECT @Name = REPLACE(@Name ,'--','');
    SET @Name = REPLACE(@Name ,'--','');

    SELECT * 
    FROM TABLENAME 
    WHERE Name LIKE N'%'+ @Name +'%'
END

Updated stored procedure:

CREATE PROCEDURE GetStudentDetails
    @Name nvarchar(300)
AS
BEGIN
    SET NOCOUNT ON;
    DECLARE @SafeSearchItem   nvarchar(30);

    SELECT  @SafeSearchItem = REPLACE(@Name ,N'--',N'')
    SET  @SafeSearchItem = REPLACE(@Name ,N'--',N'')

    SELECT * 
    FROM TABLENAME 
    WHERE Name LIKE N'%'+ @SafeSearchItem +'%'
END

EXEC

EXEC John'''select * FROM TEST2 --

In the second stored procedure, I am always able to inject SQL - not sure it is my system?

Answer 1

As it stands, we can't answer the question, as, well there isn't a question applicable for information we're been provided. There is no risk of injection in the SP we have, thus, there is not answer on how to avoid it.

Anyway, instead, what i'm going to do is show firstly why that SP isn't subject to injection and then change it so it would be, and how the limited "fix" in it could easily be avoided.

Firstly, let's start with a simple table and data (I strongly suggest running any following scripts in a Sandbox environment!):

USE Sandbox;
GO

CREATE TABLE InjectionReady (ID int IDENTITY(1,1), SomeText varchar(500));

INSERT INTO InjectionReady
VALUES ('Here is some text'),
       ('Life is like a box a chocolates'),
       ('Milk Chocolate is my favourite'),
       ('Cheese is dairy product'),
       ('Chocolate is a dairy product'),
       ('Cows say "moo"!'),
       ('English Cat says "Meow"'),
       ('Japanese Cat says "Nyaa"');
GO

OK, and now let's create your SP (amended for our object). and then do some tests:

CREATE PROCEDURE NonInjectionSearch @Wildcard nvarchar(100) AS


    SELECT @Wildcard = REPLACE(@Wildcard ,N'--',N'');
    SET @Wildcard = REPLACE(@Wildcard ,N'--',N'');

    SELECT *
    FROM InjectionReady
    WHERE SomeText LIKE N'%'+ @Wildcard +N'%';

GO
EXEC NonInjectionSearch 'Chocolate';
EXEC NonInjectionSearch '''; DROP TABLE InjectionReady;--';
EXEC NonInjectionSearch '''; DROP TABLE InjectionReady; SELECT ''';

No injection. Great! Ok, now for an SP that could suffer injection:

CREATE PROCEDURE InjectionSearch @Wildcard nvarchar(100) AS


    SELECT @Wildcard = REPLACE(@Wildcard ,N'--',N'');
    SET @Wildcard = REPLACE(@Wildcard ,N'--',N'');

    DECLARE @SQL nvarchar(MAX);
    SET @SQL = N'
    SELECT *
    FROM InjectionReady
    WHERE SomeText LIKE N''%'+ @Wildcard + N'%'';'; --Yes, intentional non parametrisation
    PRINT @SQL;
    EXEC (@SQL);

GO
EXEC InjectionSearch 'Chocolate';
GO
EXEC InjectionSearch '''; CREATE TABLE Injection1(ID int);--'; --This'll fail
GO
EXEC InjectionSearch '''; CREATE TABLE Injection2(ID int); SELECT '''; --Oh! This worked!

GO

So, how could you avoid this? Well, Parametrise your dynamic SQL:

CREATE PROCEDURE ParamSearch @Wildcard nvarchar(100) AS

    DECLARE @SQL nvarchar(MAX);
    SET @SQL = N'
    SELECT *
    FROM InjectionReady
    WHERE SomeText LIKE N''%'' + @pWildCard +''%'';'; --Yes, intentional non parametrisation
    PRINT @SQL;
    EXEC sp_executesql @SQL, N'@pWildcard nvarchar(500)', @pWildCard = @Wildcard;
GO

EXEC ParamSearch 'Chocolate';
GO
EXEC ParamSearch '''; CREATE TABLE Injection1(ID int);--'; --Won't inject
GO
EXEC ParamSearch '''; CREATE TABLE Injection2(ID int); SELECT '''; --Oh! this didn't inject either

Dynamic objects bring another level to this, however, I'll only cover this if required; as it stands (like I said at the start) the question asked can't happen for the scenario we have.

Clean up:

DROP TABLE Injection2;
DROP PROC ParamSearch;
DROP PROC InjectionSearch;
DROP PROC NonInjectionSearch;
DROP TABLE InjectionReady;