Introduction

Some time ago, one of my ex-colleagues told me about a terrible situation that had happened in his SQL Server production database. I immediately thought that it was very interesting. Of course, he did not give me his T-SQL code, so as soon as possible I built an example that fit with the situation he had to face. In my opinion, it is worth being shared because it demonstrates how much attention we have to put in our daily work and how a little carelessness and bad luck can lead to a disaster.

The Facts

One morning, my ex-colleague sat at his desk and the telephone rang immediately. It was one of his clients saying that every record of a particular table seemed to have the same wrong value in a particular column. The client aded that the in the afternoon of the day before, everything was correct. The client had several printed reports that demonstrated that.

What had happened and who was the culprit?

My ex-colleague and his team immediately got possession of the backups and restored a new database in order to get the right values of the column as they were the afternoon before. At the same time, they started analysing all the procedures carried out during the night. He finally they understood what had really happened. The culprit was a bad use of the system stored procedure sp_executesql and very bad luck.

The sample code

The following is the example that I wrote in order to reconstruct the scenario. It is very simple: using tempdb, there is a table with an identity as PK and a numeric column.

use tempdb
go
--Set up the table with 3 records
drop table if exists Table1
go
create table Table1 (Id int identity(1,1) primary key, NumericField int)
go
insert into Table1 values (10), (15), (20)
go

The following lines of code dynamically set up a string that is executed with sp_executesql. With RAISERROR the statement and the number of the updated rows are printed. The execution is inside a rolled back transaction so that we can run it several times with some modifications, always starting from the initial situation.

--The script to run
declare @SqlStmt nvarchar(4000),
      @SomeNumber int = 100,
    @NumberOfUpdatedRows int
set nocount on
set @SqlStmt = 'update Table1 set NumericField = 100 + ' + convert(varchar(10), @SomeNumber) + ' where Id = 1 '
begin tran
exec sp_executesql @SqlStmt
set @NumberOfUpdatedRows = @@ROWCOUNT
raiserror('Statement: %s', 10, 1, @SqlStmt) with nowait
raiserror('Updated rows: %d', 10, 1, @NumberOfUpdatedRows) with nowait
select * from Table1
rollback

If you run the script, you will find that the executed statement is the following and it updates one row, according to expectations:

update Table1 set NumericField = 100 + 100 where Id = 1

Now, apply the following modifications and then run the script again. In the @SqlStmt definition

1. remove the space between the symbol plus (+) and the single quotation mark
2. substitute the symbol plus (+) with the symbol minus (-)

Assign a negative value to the @SomeNumber variable. For example, let’s use “-100”.

Something totally unforeseen has just happened: the symbol minus (-) of the formula is put next to the minus of the negative value of @SomeNumber and for SQL Server “double minus” means a comment! Here is the statement that has been run:

update Table1 set NumericField = 100 --100 where Id = 1

Basically, it is an UPDATE without a WHERE clause and all the rows are updated with the same value! This was the culprit that caused some trouble to my ex-colleague, who was very unlucky because the unwanted comment popped up in a point that maintained the statement syntactically correct.

Code revision

Probably the author of the script was a junior SqQL developer that was not particularly skilled. The syntax of the procedure sp_executesql allows you to use parameters and this is definitely a better way to implement dynamic sql.

Here is the correct version of the script:

--The script to run
declare @SqlStmt nvarchar(4000),
    @NumberOfUpdatedRows int,
    @ParamDefinition nvarchar(100) = '@SomeNumber int, @Id int'
set nocount on
set @SqlStmt = 'update Table1 set NumericField = 100 + @SomeNumber where Id = @Id '
begin tran
exec sp_executesql @SqlStmt, @ParamDefinition, @SomeNumber = 100, @Id = 1
set @NumberOfUpdatedRows = @@ROWCOUNT
raiserror('Updated rows: %d', 10, 1, @NumberOfUpdatedRows) with nowait
select * from Table1
rollback

In this way, there is no risk of “self injection” or SQL injection in general.

Speaking about the performance, very often a parameterized execution of sp_executesql is faster than the corresponding ad-hoc T-SQL, but this is not the general rule. You can read more on this topic in this article by Grant Fritchey (https://dzone.com/articles/sp-executesql-is-not-faster-than-an-ad-hoc-query)

Conclusion

The system stored procedure sp_executesql is a great tool to execute T-SQL code composed as a string. Although it is possible to construct the complete ad-hoc statement without parameters, this is definitely the worst practice: it is dangerous because it leaves your system open to SQL injection and besides it is even more complicated to read and to maintain than the parameterized version.