SQL Injection and sp_executesql

Question

SQL Injection and sp_executesql

Lexa

Hall of Fame

Points: 3314
More actions
October 1, 2011 at 7:33 am

#244755

Hi,
Are SQL Injection Attacks possible with the usage of sp_executesql without parameter check for malicious code?
Thanks

Viewing 14 posts - 1 through 13 (of 13 total)

You must be logged in to reply to this topic. Login to reply

Gail Shaw SSC Guru Points: 1004504 More actions · Answer 1

Yes. If you don't parameterise the statement. You can still write un-parameterised dynamic SQL with sp_executesql. The use of sp_executesql alone won't protect you, proper parameterisation and the use of sp_executesql will.

Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

We walk in the dark places no others will enter
We stand on the bridge and no one may pass

Andre Guerreiro SSCertifiable Points: 7319 More actions · Answer 2

Plus you won't need any parameter checks for SQL injection at the beginning of your code when you use 'sp_executesql' properly as Gail advised.

'sp_executesql' will do that for you.

Best regards,

Andre Guerreiro Neto

Database Analyst
http://www.softplan.com.br
MCITPx1/MCTSx2/MCSE/MCSA

Lexa Hall of Fame Points: 3314 More actions · Answer 3

codebyo (10/1/2011)
'sp_executesql' will do that for you.

codebyo, what exactly do you mean by it will do that for you? If a '; drop table --' command is passed, it won't execute since it is treated as a parameter, right?

Thanks.

Andre Guerreiro SSCertifiable Points: 7319 More actions · Answer 4

Lexa (10/1/2011)
codebyo (10/1/2011)
'sp_executesql' will do that for you.
codebyo, what exactly do you mean by it will do that for you? If a '; drop table --' command is passed, it won't execute since it is treated as a parameter, right?
Thanks.

Exactly what I meant. 😉

It's just that your first post was asking if it's possible without parameter check and I see many procedures that have checks for every parameter passed instead of doing as advised here in this topic. 🙂

Best regards,

Andre Guerreiro Neto

Database Analyst
http://www.softplan.com.br
MCITPx1/MCTSx2/MCSE/MCSA

Gail Shaw SSC Guru Points: 1004504 More actions · Answer 5

Lexa (10/1/2011)
codebyo (10/1/2011)
'sp_executesql' will do that for you.
codebyo, what exactly do you mean by it will do that for you? If a '; drop table --' command is passed, it won't execute since it is treated as a parameter, right?

If you've parameterised the query and passed parameters, yes. If you've concatenated the parameters into the string, no.

Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

We walk in the dark places no others will enter
We stand on the bridge and no one may pass

Gail Shaw SSC Guru Points: 1004504 More actions · Answer 6

codebyo (10/1/2011)
I see many procedures that have checks for every parameter passed instead of doing as advised here in this topic. 🙂

Defend in depth. Check parameters and use parameterisation properly.

Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

We walk in the dark places no others will enter
We stand on the bridge and no one may pass

Andre Guerreiro SSCertifiable Points: 7319 More actions · Answer 7

GilaMonster (10/1/2011)
codebyo (10/1/2011)
I see many procedures that have checks for every parameter passed instead of doing as advised here in this topic. 🙂
Defend in depth. Check parameters and use parameterisation properly.

But do you think parameter check is necessary even when using proper parameterisation in 'sp_executesql'?

IF @p1 < '0' OR @p1 > '99'

BEGIN

-- Do somenthing here

RETURN;

END;

But if you're checking every parameter for valid values it would be safe to execute the code even from a concatenated string instead of real parameters, right?

Best regards,

Andre Guerreiro Neto

Database Analyst
http://www.softplan.com.br
MCITPx1/MCTSx2/MCSE/MCSA

Andre Guerreiro SSCertifiable Points: 7319 More actions · Answer 8

Or maybe I'm just making a big confusion or haven't understood clearly. Thank you for your advices. 😀

Best regards,

Andre Guerreiro Neto

Database Analyst
http://www.softplan.com.br
MCITPx1/MCTSx2/MCSE/MCSA

Lexa Hall of Fame Points: 3314 More actions · Answer 9

GilaMonster (10/1/2011)
Lexa (10/1/2011)
codebyo (10/1/2011)
'sp_executesql' will do that for you.
codebyo, what exactly do you mean by it will do that for you? If a '; drop table --' command is passed, it won't execute since it is treated as a parameter, right?
If you've parameterised the query and passed parameters, yes. If you've concatenated the parameters into the string, no.

This is what I was looking for.. I found some procs with concatenated parameters which do not prevent from passing ' ; drop table --' code. Thanks all.

Gail Shaw SSC Guru Points: 1004504 More actions · Answer 10

codebyo (10/1/2011)
GilaMonster (10/1/2011)
codebyo (10/1/2011)
I see many procedures that have checks for every parameter passed instead of doing as advised here in this topic. 🙂
Defend in depth. Check parameters and use parameterisation properly.
But do you think parameter check is necessary even when using proper parameterisation in 'sp_executesql'?

Yes. Absolutely. Always check parameters for illegal values, check ranges. Never assume. Check in multiple places so if one piece of code is in error or changed you're not completely vulnerable.

This is what the security principal (and principal of writing secure and safe code) called Defend in Depth is concerned with. Don't assume the layer above has checked the values. Don't assume they're safe because of how the layer below works.

https://buildsecurityin.us-cert.gov/bsi/articles/knowledge/principles/347-BSI.html

Or page 59-60 of "Writing Secure Code" 2nd Edition by Howard and LeBlank.

Let's say, while you're on vacation the junior developer has to make a change and he can't figure out how to get that sp_executesql working again, so he changes it to concatenated SQL and EXEC. If the parameterisation was your only level of protection, you're in trouble. If you had parameter checking on the front end, and in the procedure, and the principal of least privileged was followed then the removal of that one layer doesn't leave you completely vulnerable.

Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

We walk in the dark places no others will enter
We stand on the bridge and no one may pass

Andre Guerreiro SSCertifiable Points: 7319 More actions · Answer 11

Lexa (10/1/2011)
GilaMonster (10/1/2011)
Lexa (10/1/2011)
codebyo (10/1/2011)
'sp_executesql' will do that for you.
codebyo, what exactly do you mean by it will do that for you? If a '; drop table --' command is passed, it won't execute since it is treated as a parameter, right?
If you've parameterised the query and passed parameters, yes. If you've concatenated the parameters into the string, no.
This is what I was looking for.. I found some procs with concatenated parameters which do not prevent from passing ' ; drop table --' code. Thanks all.

I find many sp like that in my company. Too late to change everything now. But as soon as I see something like that when I'm maintaining code I change it to the right way.

My point was that it isn't necessary to "check for SQL injection" using proper parameterisation but it's maybe useful to "check for valid values" before passing the parameters to execution.

I've got confused and I have a hangover to take care of. 😀

Best regards,

Andre Guerreiro Neto

Database Analyst
http://www.softplan.com.br
MCITPx1/MCTSx2/MCSE/MCSA

Andre Guerreiro SSCertifiable Points: 7319 More actions · Answer 12

GilaMonster (10/1/2011)
Let's say, while you're on vacation the junior developer has to make a change and he can't figure out how to get that sp_executesql working again, so he changes it to concatenated SQL and EXEC. If the parameterisation was your only level of protection, you're in trouble. If you had parameter checking on the front end, and in the procedure, and the principal of least privileged was followed then the removal of that one layer doesn't leave you completely vulnerable.

I see what you mean. I wan't thinking that something like that could ever happen. That's an interesting situation... Sometimes we must defend our work from other people indeed.

Thank you for the link. Much appreciated.

Best regards,

Andre Guerreiro Neto

Database Analyst
http://www.softplan.com.br
MCITPx1/MCTSx2/MCSE/MCSA

Gail Shaw SSC Guru Points: 1004504 More actions · Answer 13

Lexa (10/1/2011)
GilaMonster (10/1/2011)
Lexa (10/1/2011)
codebyo (10/1/2011)
'sp_executesql' will do that for you.
codebyo, what exactly do you mean by it will do that for you? If a '; drop table --' command is passed, it won't execute since it is treated as a parameter, right?
If you've parameterised the query and passed parameters, yes. If you've concatenated the parameters into the string, no.
This is what I was looking for.. I found some procs with concatenated parameters which do not prevent from passing ' ; drop table --' code. Thanks all.

Yup. Very common.

On that point, I forgot that I wrote this: http://sqlinthewild.co.za/index.php/2009/04/03/dynamic-sql-and-sql-injection/

Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

We walk in the dark places no others will enter
We stand on the bridge and no one may pass