CODEWITHSUNDEEP
sqlsql-servert-sql
AWinter
AWinter

Reputation: 81

Am I vulnerable to an SQL injection?

I am making an SSRS report, with the stored procedure listed below. My users need a search tool that will return all possible results in the name field such that a name of 'AD' should return 'ADAM' and 'MADELYN'.

I am worried that since I am using string concatenation for my where clause, is it possible that this stored procedure could fall victim to an SQL injection attack:

BEGIN
    @location varchar(20),@name varchar(20) 
    SELECT location, name 
    FROM   table 
    WHERE  (location LIKE @location+'%') AND (name LIKE '%'+@name+'%')
END

Is this code vulnerable? And, if so, how can I fix it to be safe?

Upvotes: 3

Views: 275

Answers (2)

Allan
Allan

Reputation: 17429

The code you posted is not vulnerable to SQL injection. Concatenating string values that you use in the query is fine, it's only when you construct the query by concatenating strings together that you are vulnerable.

For code that resides in T-SQL, that means unless you're using either EXEC or sp_executeSQL you are unlikely to be vulnerable.

An example that is equivalent to your code and vulnerable to SQL injection:

BEGIN --Don't do this!
    @location varchar(20),@name varchar(20) 
    sp_executesql('
    SELECT location, name 
    FROM   table 
    WHERE  (location LIKE ' + @location + '%'') AND (name LIKE ''%' + @name + '%''')
END

Upvotes: 5

Joel Coehoorn
Joel Coehoorn

Reputation: 416049

This part of the code, at least, is okay. Yes, you are using string concatenation, but the concatenation happens at runtime for the query, after it was compiled. The execution plan is already determined, and the result of the concatenation is only itself used as value; never as code. There is no way an extra ' character or anything else could cause malicious elements of that string to leak out and be interpreted as sql code.

String concatenation is a problem for SQL strings with user data when it happens before compile time... either at the client code level, or on the server ahead of an exec() or sp_executesql() or similar, because the result the of the concatenation has the potential to be interpreted as code.

Of course, there may be other things in this procedure we haven't seen that still have a problem.

Upvotes: 2

Related Questions