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.
