2
\$\begingroup\$

I've implemented a stored procedure for a search process SQL Server 2008. I'm not sure if everything I created is correct. Also I'm wondering if this code has any leaks or vulnerability on SQL Injection. Here is my stored procedure:

USE [TestDB]
GO
/****** Object:  StoredProcedure [dbo].[Search_Dictionary]    Script Date: 08/09/2018 19:17:48 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[Search_Dictionary]
  @FilterBy int = NULL,
  @Name varchar(50) = NULL,
  @Code char(2) = NULL
AS
BEGIN
    SET NOCOUNT ON;

    SELECT RecID, Status, Code, Name
    FROM Dictionary
    WHERE 
        (@FilterBy = 1 AND Name LIKE '%'+@Name+'%')
        OR 
        (@FilterBy = 2 AND Code = @Code)
        OR
        (@FilterBy = 3 AND @Name IS NULL AND @Code IS NULL);
END

Here is example on how I call this procedure:

EXEC Search_Dictionary @FilterBy = 1, @Name = "Grant", @Code = NULL;

I just want to prevent, if for example Filter By is 2 that should search query by Code column returns any result if user pass word Grant. In that case should return 0 records.

Also if anyone have any suggestions on how to improve the code please let me know.

\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

A user could cause a dos attack by passing in complex like filters, you might want to sanitize the @name parameter. https://www.owasp.org/index.php/Testing_for_SQL_Wildcard_Attacks_(OWASP-DS-001)

\$\endgroup\$
0
\$\begingroup\$

Vulnerabilities

There's no way this stored procedure can be used for SQL injection. The only way a stored procedure can allow SQL injection is if it allows an unsanitized user-entered string to be used to build a dynamic query using sp_executesql (see docs). You're all safe on the vulnerabilities side.

Functionality

The intended functionality with regards to the FilterBys is not very clear in your posts so I'll try to explain how I think they'll behave:

Your procedure has three "modes" depending on what number is entered in to FilterBy.

  1. For FilterBy = 1, the procedure would return any rows that have the @Name parameter inside the Name column. It ignores whatever is entered in to @Code in this instance. Note, if @Name is an empty string, all records will be returned. Is that something you want?*
  2. For FilterBy = 2, the procedure returns any rows where the @Code parameter matches the Code column entirely. In this instance, if @Code is null or empty, no records are returned. Whatever is passed in as @Name is ignored – whether it's null or not.
  3. For FilterBy = 3, the procedure returns all rows, but only if @Name and @Code are null. If either of these parameters not null, no records are returned.

* If you don't want an empty string to return all rows, change the line to something like:

(@FilterBy = 1 AND Name LIKE '%'+@Name+'%' AND @Name <> '')

Performance

Performance of this query largely depends on the size of the Dictionary table (obviously), and indexes. If you can afford the disk space and slower write speed, you could add an index to Dictionary with the Code column as the first indexed field. Make sure you include RecID, Status, and Name columns somewhere in that index. This would increase performance when using FilterBy = 2.

You could try adding another index with the Name field first, however this wouldn't do much for performance since you're using wildcards on the left side of the parameter when searching the Name field. Here's a quick little explanation of why with a nice analogy.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.