CODEWITHSUNDEEP
sqlsql-servert-sqldynamic-sqlsp-executesql
Hyjrt6534
Hyjrt6534

Reputation: 21

Complex sp_executesql - vulnerable to injection?

Just started a new job, and I have found an insanely complex call to sp_executesql (SQL Server 2008 R2). I can't decide whether it is vulnerable to SQL injection or not.

The following is defined as a single C# string (so no additional escaping):

declare @temp table(keyid int IDENTITY(1,1) PRIMARY KEY, name nvarchar(50));
insert into @temp (name) select distinct name from SOMEWHERE where code=@code;

declare @names nvarchar(max); 
declare @dyn nvarchar(max); 

-- dynamically build the columns
select @names = Stuff ((select '],['+name from @temp order by name for XML PATH('') ),1,2,'')+']'; 

-- dynamically build the pivot query
set @dyn = N'select '+ @names +' from (select name, score from TABLE where code='''+@code+''') as p PIVOT
    (max(score) for name in ('+ @names +')) as pivtab';

execute sp_executesql @dyn

And then the whole lot is executed as so

exec sp_executesql @query, N'@code nvarchar(500)',@code=N'..something..'

So @code is both used properly (in the insert statement) but also used badly in the construction of @dyn (and @names too, one of those name fields could contain malicious script).

It seems like a bad code smell, but I can't seem to write a value for @code that will execute arbitrary SQL. And I lack the authority to just force the issue.

Does anyone know if this type of thing is safe or not? Thanks

Upvotes: 2

Views: 385

Answers (1)

Lukasz Szozda
Lukasz Szozda

Reputation: 176264

This looks like classic dynamic PIVOT.

declare @temp table(keyid int IDENTITY(1,1) PRIMARY KEY, name nvarchar(50));

insert into @temp (name) 
select distinct name from SOMEWHERE where code=@code;

declare @names nvarchar(max) 
       ,@dyn nvarchar(max); 

-- dynamically build the columns
select @names = Stuff ((select ','+ QUOTENAME(name) 
                        from @temp 
                        order by name 
                        for XML PATH('') ),1,1,''); 

-- dynamically build the pivot query
set @dyn = 
   N'select '+ @names +' from (select name, score from TABLE where code=@code) 
    as p PIVOT
    (max(score) for name in ('+ @names +')) as pivtab';

execute dbo.sp_executesql 
        @dyn,
        N'@code DATATYPE',
        @code;

I would:

  1. Use QUOTENAME instead of concatenating ],[ (just as good practice)
  2. Parametrize @code (possible vector of attack, user can pass malicious code)

If you can do PIVOT in application layer do it.

I recommed also to read The Curse and Blessings of Dynamic SQL

Upvotes: 2

Related Questions