Improving my SQL skills through your questions!

OK, I know many of you have seen this before (an oldie, but a goodie!):

(image from xkcd.com, with “copy and share” license described here: License)

But, what can you do to prevent this? And, when would this even be possible?

This is possible when DSE (dynamic string execution) occurs. There are still some VERY relevant and important reasons to use DSE and some are performance related (ok, this is another post for another day) but suffice it to say – I use DSE but I also know how to prevent Little Bobby Tables from running amuk within my database.

(April 5 note: after a few comments about sp_executesql, I’ve modified the following paragraph to explain a bit more… I’m *very* aware of sp_executesql and while it does have some benefits, it also has some negatives! I promise, this is coming soon in another post. I really want this post to focus solely on how to minimize injection when you do NEED to use a dynamically constructed string and there are times when this is the ONLY way to solve certain performance problems. sp_executesql – while often compared to EXEC as a better choice just isn’t ALWAYS better and really it’s not a simple comparison as they are not equivalent!! In fact, there are some cases where I would specifically avoid sp_executesql… So, I promise… this will be coming up in a blog post soon!! Great comments! Thanks!! :))

So, while the discussion on when, where and why you might use DSE is another one - especially when compared with sp_executesql (yes, I hear another blog post coming – and, well, I’m kind of on a roll right now :)… I want to give you a couple of very cool tips/tricks to reduce the possibilities of problems if/when you do use it (and, there REALLY are places where it’s necessary!). It’s especially important as there are a few things that many folks just don’t even know exist. And, there are 3 parts to this article… I would expect that most apps need QUOTENAME() and/or REPLACE() but ALL apps should consider my recommendations/comments for EXECUTE AS – if/when you’re actually using DSE (through EXEC).

QUOTENAME()

REPLACE()

EXECUTE AS

Part I: QUOTENAME()

In a question/thread (this is many moons ago), the following question was asked (NOTE: I have “tweaked” the code to highlight ONLY the problem here AND I’ve made it so that it won’t execute):

I have a procedure that creates a database (below). When people use spaces in their database names it fails. How can I make it work if they supply spaces?

And, there was a reply to this question where the answer was (and I am NOT recommending this answer. Please do NOT stop reading here otherwise you are asking for a world of pain – and LOTS of SQL Injection):

Just go ahead and do the following. By putting brackets around the name you will solve this problem.

OK, this is a SCARY recommendation. Not just because it won’t always work (what if someone actually wants [don't get me wrong - I don't know WHY they'd want to do this but...] to put [brackets] in the name?) but because it’s prone to SQL Injection.

This won’t work:

EXEC dbo.CreateDBProc N'This is the most stupid :) name I can think of with [brackets] and all sorts of junk! in the name';
GO

Now, no matter what messed up name someone actually WANTS to create – this will protect it such that SQL Injection is not possible. You can try any of these combinations with this version of the proc and while creating a database actually named “[fakedbname] DROP DATABASE foo –” probably isn’t desirable, dropping a database isn’t either (and I would argue is MUCH worse).

Note: The first and second execute (if you actually execute instead of just SELECT @ExecStr) *and* create databases with these ugly names. The third and fourth don’t execute because some of the injected characters are not supported for file/directory names. However, the most important point is that *all* are protected from SQL Injection!

Part II: REPLACE()

But, what if the string you need to work with isn’t an identifier? Can you still use QUOTENAME()? And, the answer is YES, IFthe string is compatible with SYSNAME which is an nvarchar(128). However, if you need longer strings or if you are protecting a string which is not an identifier – what else can you do? Well… it’s definitely NOT as pretty but it works. The answer here is to use REPLACE(). If I had wanted to do this with the procedure above, I would have had to do the following:

However, this will NOT work here because an identifier must be properly quoted within the string and there are only two ways to “quote” identifiers. One is the [bracket] and the other is “double quotes” not single. Instead, this is what the procedure will look like (nope! we don’t need REPLACE()):

And, SQL only supports brackets by default (without any special settings). To allow double quotes, you can turn on QUOTED_IDENTIFIER (using SET QUOTED_IDENTIFIER ON) to allow double quotes instead of brackets but this has a few negative consequences (See SET Options That Affect Results) and personally, I always try to avoid making SET options changes in most of my stored procs.

Once again, REPLACE() isn’t needed. And, to be honest, this becomes a nightmare with ” [double quotes] or actual [brackets] in the name. QUOTENAME() deals with those properly. So, with an identifier you really need QUOTENAME(). So, where does replace come in? With actual strings in your DSE. Imagine that you’re wanting to pass in a first name but something about your application requires that this happen through DSE (and, this CAN definitely happen – mostly for performance purposes). Don’t get me wrong, I just don’t have the ability to do all of this within one post but simply put, because of how stored procedures create their execution plans there are OFTEN reasons for why you might want a string to be built dynamically. No, probably not one quite this simple but I’m trying to keep this particular example simple. This is probably what it would look like to start:

Darn. But, at least this is how you catch it. Instead of with Little Bobby Tables. What if this gets executed:

EXEC GetMembers N'''Robert''); DROP TABLE Students; --''';
GO

Luckily, I got:

Msg 3701, Level 11, State 5, Line 1
Cannot drop the table 'Students', because it does not exist or you do not have permission.

But, what if there had been a table called Students AND the context of the execution was such that the could execute this? Well, that’s what’s proposed by the cartoon and well… I’ve *unfortunately* heard of that happening!

So, what do you do? First, you need to protect the string being inserted! This is what you need:

And, this requires that the string be surrounded with quotes AND the string submitted have all single quotes replaced with ” [single quote followed immediately by a second single quote]). Yes, I realize that this is starting to look ugly but… what happens with Little Bobby Tables? We get his row. NOTE: We do have other potential performance problems here because the parameter (when it gets thrown back out into the string – is no longer an nvarchar, it’s only a varchar). So, we should also consider adding an N into the string like the following:

And, what if the string being built has numerics or other types – what should we do there? To be honest, I often EXPLICITLY type the variable in what looks like a completely unnecessary CONVERT. However, for performance purposes I *often* do this. Ugh, the future posts are REALLY starting to build from here! Again, I’m not hitting performance in this one so I’m not going to take the bait of the tangent. Another time for sure!

Finally, what if our strings are REALLY complex and we need to do some strange stuff and well… we’re not sure that the string can be completely protected? What if we’re passing in a much more complicated “piece” of a WHERE clause or a full and customized ORDER BY? What if it’s not just a SINGLE value? Ugh… this is much harder but you definitely need EXECUTE AS.

Part III: EXECUTE AS

This is the ultimate in protection for your DSE code. This is where you decide that you’re going to allow virtually anything in the WHERE clause or ORDER BY but you really want to protect your database. Here, what I need is EXECUTE AS. And I’m not a big fan of this kind of code – but, again, I’m not looking at perf or anything here… I’m *just* looking at the security and potential of the injection.

SQL Server 2005 introduced EXECUTE AS and it has a lot of options – in fact, 4 options:

EXECUTE AS Caller

EXECUTE AS User

EXECUTE AS Self

EXECUTE AS Owner

EXECUTE AS Caller is the default. This means that the user that executes the procedure must have the DIRECT rights associated with the Dynamically Executed String. And, so, this is good. This should protect MOST cases altogether. However, what if the users have higher privileges? Well, I’d say that there’s already something wrong with your database (because this should NEVER be the case) but… well… I’ve seen apps (and even RECENTLY) heard of apps that connect as SA. Yes, seriously. So… what can they do… Well, much worse than DROP TABLE. How about DROP DATABASE? Or, well, let’s just say that if I found vulnerabilities in that – I could possible get outside of their SQL Server and do even more damage – to files, to the network. So… this is REALLY REALLY REALLY REALLY BAD. I would STRONGLY suggest that anyone that connects as SA – IMMEDIATELY WORK HARD TO CHANGE THIS.

EXECUTE AS Owner is almost as bad (although nothing’s as bad as connecting as SA) – especially when the OWNER has elevated rights. So, I’d try to avoid EXECUTE AS Owner unless you SEVERELY restrict who has permissions to the procedure in general. But, don’t EXECUTE AS Owner for a stored procedure that’s granted EXEC to public. That completely defeats the purpose!

EXECUTE AS Self is a bit strange. But, when a stored procedure is created in a schema, the ownership chaining and privileges follow from the OWNER of the schema. So, what if the author of the stored procedure has rights that the owner doesn’t. No, I don’t really recommend this but that’s where “self” comes in. This is where the procedure can execute under the context of the actual creator (not the owner). However, often these are the same and often they’re DBO. This is even worse… Although still, not as bad as connecting as SA.

EXECUTE AS User (where the User is a low-privileged user with very few rights) is good. And, in fact, this will be the answer to this problem!

Yes, even if you connect as SA, I can STILL protect you. Yes, we can still protect this code even when run under the connection context of SA – with EXECUTE AS.

And, even if for some reason you can’t use REPLACE() this procedure will be limited in what they can do because the user (User_procedurename) has limited rights within the database. In fact, I would recommend that the user ONLY have the necessary rights for THAT stored procedure and I’d even recommend that you create one user for each stored procedure (that has DSE). Yes, that’s potentially a lot of users… but, better than the alternative(s)!

Summary/Resources

Finally, please be sure that you’re doing some form of auditing if you’re using EXECUTE AS. And, you should also do a quick check to see if anyone is ELEVATING user’s rights to a more privileged user. Check out these two blog posts for additional information:

Hey there Dave – Actually, I agree and disagree :) – which is why I’m trying *not* to do perf in this first one. sp_executesql has some benefits in that you have strongly typed variables and the plan is cached (sp_executesql is also known as "forced statement caching"). And, while having a *good* plan in cache (that’s getting reused) is good… often you might end up with a bad plan being FORCED into cache and subsequent users are then forced into a bad plan. So… there are still cases where EXEC is a *MUCH* better choice over sp_executesql.

I definitely need to do another post here… soon!!!

Cheers and thanks for your link. I’ll review that when I go down the perf path for sure. I’ve seen a lot of recommendations of sp_executesql instead of EXEC and I tend to disagree with a few of them.

Hey there Dave – Well… actually, yes. There are a couple of key points where sp_executesql doesn’t actually solve the *performance* problem. And, this has been on my ToDo list for quite some time as I’ve seen a lot of recommendations for sp_executesql (without any negatives).

Hey there Simon – While I agree that your sp_executesql example avoids SQL Injection it is WAY more prone to performance problems than mine (and EXEC – especially for more complex statements). I promise (yes, I really, really promise! :)) that I’m going to tackle this in another post – SOON – but I *really* don’t want people to think that EXEC and sp_executesql are interchangeable. They definitely aren’t and this is the point that I’m really planning to flesh out because a lot of people say to use sp_executesql instead of EXEC (to avoid injection) but completely miss the sometimes VERY negative performance problems when doing this. So, this can cause a lot of confusion! I was "trying" to avoid performance here and just focus on the syntax of when DSE is *really* necessary and how to avoid injection when you HAVE TO USE EXEC.

But, I’ll definitely use exactly this type of example as a starting point to show where sp_executesql FAILS (from a performance perspective) and where EXEC is an absolute requirement for certain types of strings. I have lots of good stuff here!!

[...] is where we're at – we've looked at some of the problems with DSE in my post titled: Little Bobby Tables, SQL Injection and EXECUTE AS. Then, we started to look at EXEC and sp_executesql – how are they different? in the [...]

[…] The end result is that I just can’t recommend using OPTION (RECOMPILE). In the past, if I’ve ever run into problems with OPTION (RECOMPILE), I always consider rewriting the statement using a dynamic string instead; this always works! Now, for the first tangent – yes, you have to be careful of SQL injection. But, there are ways to eliminate this. Check out this blog post if you want more information Little Bobby Tables, SQL Injection and EXECUTE AS. […]

It’s because you’re using a single double-quote before the single quote after the N. In my code that’s THREE single-quotes. Please copy / paste the code from the blog so that you use the exact same code. If you’re trying to type it in manually then you need to be really careful with those darn quotes – they get me every time as well.