Stored procedures are generally a good way of preventing SQL injection as they encourage parameterisation. A big exception to this rule is if exec\execute\sp_executesql is used within a stored procedure, these may run a string built up from component parts. These component parts can have malicious code injected into them.

The contents of your stored procedure may look something like below.

EXEC(@SQL)

or

EXECUTE [sys].[sp_execute] @SQL

This is major risk. How has @SQL been constructed? If it is by appended strings (and why else would you use this technique) you have a high risk item.

A better solution is sp_executesql with parameters. The example below has a query containing parameters – @SQLToRun. The parameters are passed in using the next two arguments. If constructed correctly this can encourage query plan reuse and prevent sql injection. Also just like calling a stored procedure from code all parameters are double quoted automatically preventing injection attacks.

DECLARE @ParmDefinition  AS NVARCHAR(200)
SET @ParmDefinition = N’@param1  NVARCHAR(40)’;

EXECUTE [sys].[sp_executesql]
@SQLToRun,
@ParmDefinition,
@param1 = param1In

The @SQLToRun is text from somewhere and can be prone to injection if appended together from different strings rather than using a fixed string and so is still a risk. Ideally @SQLToRun should be a fixed string in the stored procedure, something like

set @SQLToRun = “select mycolumn from mytable where myothercolumn = @myparam1”

However you often use this technique concatenate strings to execute

set @SQLToRun = mystring1 + mystring2; <- High risk

You need to examine carefully how the @SQLToRun is constructed. The strings should be selected by case of if statements depending on the a parameter. What I mean by this is that no user input should end up in @SQLToRun rather it should be build up like below

set @SQLToRun = “select mycolumn from mytable”

IF @param == 1

BEGIN

set @SQLToRun = @SQLToRun +”inner join othertable on mytable.Id = othertable.id

END

set @SQLWhere = “Where ”

IF @param == 1

BEGIN

set @SQLWhere = @SQLWhere +”othertable.id = @userparameter”

END

 

Then called as below. This way any user input only arrives in @userparameterIn as a parameter to a statement

EXECUTE [sys].[sp_executesql]
@SQLToRun + @SQLWhere ,
@ParmDefinition,
@userparameter = @userparameterIn

 

A strong sign something is wrong is if the parameters are missing I.e.

EXECUTE [sys].[sp_executesql]  @SQLToRun

or

EXEC(@SQLToRun)

As the stored procedure is trusted it will be able to perform actions that the service account connecting to the db may not be able to do directly. What I mean is the service account may be limited in its permissions on the database i.e. it may only have permission to read 3 tables. When calling the stored procedure it is trusted and code within it can probably do a lot more than read a few tables. That is why this approach needs to be crafted with care.