The Old Joke Goes
A security engineer walks into a bar and then
- Runs into a bar.
- Crawls into a bar.
- Dances into a bar.
- Flies into a bar.
- Jumps into a bar.
And orders:
- a beer.
- 2 beers.
- 0 beers.
- 99999999 beers.
- a lizard in a beer glass.
- -1 beer.
When you’re designing and testing SQL Server stored procedures (or views or functions or queries), you need to do the same thing.
While most of it isn’t a security concern, though it may be if you’re using Row Level Security, Dynamic Data Masking, or Encrypted Columns, you should try executing it as other users to make sure access is correct.
When I’m writing stored procedures for myself or for clients, here’s what I do.
Try To Reveal Bad Parameter Sniffing
Sometimes it’s easier than others, but here’s what you should try:
- For equality predicates, run some count queries against those columns to find data skew
- For range predicates (like dates) try small and large ranges to see if the plan changes
- Try passing in NULL or blank values, especially for any string parameters
When you do this, grab and compare the execution plans. If you get crazy different plans, see what happens when you don’t recompile, and plans get shared across executions.
If performance is bad, think about these things:
- Can you improve indexing?
- Do you need a temp table?
- How many CTEs do you need to throw in the garbage?
- Should you use dynamic SQL to generate different plans?
- Maybe a recompile hint would be good for a specific query?
This is at the top of my list for new code, because I don’t want it to get called in a weird way and go haywire. That’s not what people pay consultants for.
Speaking Of Dynamic SQL
If you’re not sure if yours is safe from SQL injection, now is the time to find out.
Especially for long-ish string parameters, try passing in some nefarious commands. In general, what you don’t want to see is dynamic SQL like this:
DECLARE @s nvarchar(MAX) = N'', @d nvarchar(40) = N'Jon Skeet'; SELECT @s += N' SELECT c = COUNT_BIG(*) FROM dbo.Users AS u WHERE u.DisplayName = ''' + @d + N''';' EXEC sys.sp_executesql @s;
This is unsafe dynamic SQL, because it accepts user input and concatenates it into a string.
There are safe ways to accept user input, as long as either:
- The user input dictates a static string to append to the dynamic SQL
- The user input is parameterized within the dynamic SQL
Something like this is an example of taking user input and having it dictate a static string:
DECLARE @s nvarchar(MAX) = N'', @t nvarchar(40) = N'Votes'; IF @t = N'Users' BEGIN SELECT @s += N' SELECT c = COUNT_BIG(*) FROM dbo.Users AS u;' END; IF @t = N'Votes' BEGIN SELECT @s += N' SELECT c = COUNT_BIG(*) FROM dbo.Votes AS v;' END; EXEC sys.sp_executesql @s;
But this is a case where you should see what happens when you pass a lizard in a beer glass.
And of course, parameterized dynamic SQL looks like this:
DECLARE @s nvarchar(MAX) = N'', @d nvarchar(40) = N'Jon Skeet'; SELECT @s += N' SELECT c = COUNT_BIG(*) FROM dbo.Users AS u WHERE u.DisplayName = @d;' EXEC sys.sp_executesql @s, N'@d nvarchar(40)', @d;
Run It From The Application
In SQL Server, there are these things called ANSI settings, and they can really screw with performance and execution plans.
Even if you’re not using indexed views, computed columns, or filtered indexes, you may see oddball things if you’re testing in SSMS and running code from somewhere else.
This is what SSMS uses, and what SQL Server needs to effectively use those indexed views, computed columns, and filtered indexes.
+-------------------------+----------------+ | SET options | Required value | +-------------------------+----------------+ | ANSI_NULLS | ON | | ANSI_PADDING | ON | | ANSI_WARNINGS 1 | ON | | ARITHABORT | ON | | CONCAT_NULL_YIELDS_NULL | ON | | NUMERIC_ROUNDABORT | OFF | | QUOTED_IDENTIFIER | ON | +-------------------------+----------------+
You’ll need to check your application(s) to see what they’re using, and make adjustments where necessary.
Bonus points if you’re using the Python or JDBC drivers, and you turn off those pesky implicit transactions.
This Is A Good Start
You may have other things to test, like if you’re offloading reads in an Availability Group, using security features, or other logical constructs.
If your code has a lot of variable assignment in it, you may want to override the lookups with static values to see what happens if some out of band values (especially NULLs) come around.
I do this a lot in the SQL Server Troubleshooting stored procedures that I write to make sure things go down the right path and the right things happen.
You should too.
No one expects the unexpected.
Thanks for reading!
Going Further
If this is the kind of SQL Server stuff you love learning about, you’ll love my training. I’m offering a 75% discount to my blog readers if you click from here. I’m also available for consulting if you just don’t have time for that, and need to solve database performance problems quickly. You can also get a quick, low cost health check with no phone time required.
You forgot NULL beers 😀
Blame the lizard.
It just so happens that I spent the afternoon tracking down an obscure failure, tracing it to a statement:
UPDATE dbo.Person SET DoctorPhone = ‘?’+DoctorPhone WHERE 1=1
DoctorPhone is defined as NVARCHAR(128) to allow for international customers and obscure internal phone systems. Adding ‘?’ to all records in a test clone database ought to prevent the field from being used inadvertently by phone-blast software. The UPDATE statement failed with “String or binary data would be truncated” – why, one asks?
Because the customer had entered the following value for DoctorPhone…
https://www.google.com/search?q=pediatrics+abcdefg+al&client=safari&hl=en-us&ei=EhkyY6qcG5apqtsP6e652Ak&oq=pediatrics+abcdefg&gs
…a perfectly valid URL of exactly 128 characters, which left no room for me to prefix the value with ‘?’
This is now my go-to response to the question: “Do you really have to test against redacted client data?”
Oh that’s spicy! Nice work! Man, I can’t imagine how lousy that was to track down.