-2

I have a stored procedure as below:

CREATE PROCEDURE sp_GetName(@Name NVARCHAR(50))
AS
BEGIN
    DECLARE @sqlcmd NVARCHAR(MAX);
    DECLARE @params NVARCHAR(MAX);
    SET @sqlcmd = N'SELECT * FROM [dbo].[Employee] WHERE Name like ''%' + @Name + '%''';

    PRINT @sqlcmd;

    SET @params = N'@Name NVARCHAR(50)';
    EXECUTE sp_executesql @sqlcmd, @params, @Name;
END

EXEC sp_GetName '';WAITFOR DELAY '00:00:10'

Whenever I execute above statement, it always delays the response.

How can I write my procedure so that it will handle this SQL Injection attack.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
user3125433
  • 973
  • 1
  • 8
  • 7
  • 1
    Side note: you should **not** use the `sp_` prefix for your stored procedures. Microsoft has [reserved that prefix for its own use (see *Naming Stored Procedures*)](http://msdn.microsoft.com/en-us/library/ms190669%28v=sql.105%29.aspx), and you do run the risk of a name clash sometime in the future. [It's also bad for your stored procedure performance](http://www.sqlperformance.com/2012/10/t-sql-queries/sp_prefix). It's best to just simply avoid `sp_` and use something else as a prefix - or no prefix at all! – marc_s Mar 29 '19 at 18:25

3 Answers3

2

Don't use dynamic SQL if you want to avoid SQL injection attacks. It doesn't matter whether you wrap the string concatenation code in a stored procedure or not.

In this case there's no reason to use dynamic SQL. You can simply write :

SELECT * FROM [dbo].[Employee] WHERE Name like '%' + @Name + '%'

to search for employees with a specific string in their name.

This query is immune to SQL injection whether you execute it as a parameterized query from a client or as a stored procedure. Clients would call both of them the same way.

The stored procedure should be just :

CREATE PROCEDURE getName(@Name nvarchar(50)
AS
SELECT * FROM [dbo].[Employee] WHERE Name like '%' + @Name + '%'

I removed the sp_ prefix because it's used for system stored procedures.

The problem with this code is that %something% will have to search the entire table. It can't use any index on Name. Only prefix searches can use indexes, ie @Name + '%' :

SELECT * FROM [dbo].[Employee] WHERE Name like @Name + '%'
Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
  • Even If I use this and run this query, exec sp_GetName '';WAITFOR DELAY '00:00:10', it will still provide the response after delay – user3125433 Mar 29 '19 at 18:10
  • @user3125433 that's what you asked the server to do. Execute a stored procedure and then wait for 10 seconds. What did you expect it to do? This has nothing to do with the question - how to avoid SQL Injection. – Panagiotis Kanavos Apr 01 '19 at 06:54
0

You just need to parametrize the dynamic code.

CREATE PROCEDURE sp_GetName(@Name NVARCHAR(50))
AS
BEGIN
DECLARE @sqlcmd NVARCHAR(MAX);
    DECLARE @params NVARCHAR(MAX);
    SET @sqlcmd = N'SELECT * FROM [dbo].[Employee] WHERE Name like ''%'' + @Name + ''%''';
    print @sqlcmd;
    SET @params = N'@Name NVARCHAR(50)';
    EXECUTE sp_executesql @sqlcmd, @params, @Name;
End

exec sp_GetName '';

However, the code has nothing that would justify the need of making it dynamic. It could be simply rewritten as this.

CREATE PROCEDURE sp_GetName(@Name NVARCHAR(50))
AS
    SELECT * FROM [dbo].[Employee] WHERE Name like '%' + @Name + '%';

Why are you making your application slow by design?

Luis Cazares
  • 3,495
  • 8
  • 22
  • Even If I use this and run this query, exec sp_GetName '';WAITFOR DELAY '00:00:10', it will still provide the response after delay – user3125433 Mar 29 '19 at 18:09
  • Why are you using `WAITFOR DELAY '00:00:10'`? – Luis Cazares Mar 29 '19 at 18:12
  • Actually vulnerability scan is done for my site and this blind sql injection has been detected. They provide inputs like this in textboxes and see if this induces the delay in response. I need to make my stored procedure in such a way so that it does not have any impact. – user3125433 Mar 29 '19 at 18:17
  • 1
    You've been shown how to prevent SQL injection. The code in the answers is not vulnerable to injection. If you run `exec sp_GetName '';WAITFOR DELAY '00:00:10'` you're not testing for injection, you're just running the procedure and adding a delay after it. – Luis Cazares Mar 29 '19 at 18:21
0

You can just embed the SQL in you're stored procedure without using sp_executesql

CREATE PROCEDURE sp_GetName(@Name NVARCHAR(50))
AS
BEGIN
    SELECT * FROM [dbo].[Employee] WHERE Name like '%' + @Name + '%'
End

Your example doesn't actually demonstrate a SQL injection attack; you've just got 2 sql statements seperated by a semi-colon, the second of which is a "wait"

 -- Also demonstrates wait behaviour
 exec sp_who;WAITFOR DELAY '00:00:10'
Andrew Skirrow
  • 3,402
  • 18
  • 41
  • Even If I use this and run this query, exec sp_GetName '';WAITFOR DELAY '00:00:10', it will still provide the response after delay – user3125433 Mar 29 '19 at 18:08
  • I think that's just because your executing the stored procedure then waiting for 10 seconds. The query results aren't rendered until the entire batch is finished. – Andrew Skirrow Apr 01 '19 at 10:19