I've always been concerned with security and I've always stressed the importance of auditing the REAL user context not just the current user (see this post on EXECUTE AS and auditing). So, I generally try to avoid using dynamic string execution and if necessary create well tested/protected parameters (fyi – using QUOTENAME can be a fantasic solution to protectng identifiers as input parameters but it can't protect more complex strings).

Having said that, what if I'm looking at a database for the first time… just poking around trying to see if there's anything that needs further attention? I've come up with a quick query… And, while it's not going to "solve" your problem (as that's going to take some re-writing of code) or even truly verify if you're vulnerable, it gives you a "quick list" of where you should look first! If your code uses dynamic strings AND it's elevated – then start there!

SELECT OBJECT_NAME(object_id) AS [Procedure Name], CASE WHEN sm.definition LIKE '%EXEC (%' OR sm.definition LIKE '%EXEC(%' THEN 'WARNING: code contains EXEC' WHEN sm.definition LIKE '%EXECUTE (%' OR sm.definition LIKE '%EXECUTE(%' THEN 'WARNING: code contains EXECUTE' END AS [Dynamic Strings], CASEWHEN execute_as_principal_id IS NOT NULL THEN N'WARNING: EXECUTE AS ' + user_name(execute_as_principal_id) ELSE 'Code to run as caller – check connection context' END AS [Execution Context Status]FROM sys.sql_modules AS smORDER BY [Procedure Name]

16 Responses to Looking for security vulnerabilities in database code

Hmm, this code, as-is, is going to find all nested stored proc calls. Not sure if that’s a very good thing since in many cases it will return almost all of the stored procedures in the system (a lot of times I see central config procs that need to be called at the start of every other proc to get values to use). I think you could get around this easily enough. Search for either of the following:

On further thought, it should also ignore white space, so that it can catch, e.g.:

EXEC
(
‘some dynamic sql here’
)

… that’s the way I tend to format my code, so I would escape scrutiny right away – and we wouldn’t want that!

I think the easiest way to handle that is to do REPLACE on the ‘definition’ column and change chracters 9, 10, 13, and 32 to empty strings before doing the LIKE comparison. Probably want to encapsulate that in a CTE or derived table?

Todd – I was trying to differentiate between a single space and no space. However, I didn’t want to put % or _ there because I didn’t want to get other words that begin with EXEC.

Adam – Yeah, you’re right… but, I can’t come up with a better way to look for that unless I do the above (using %). But, I do like your idea of changing tabs and spaces… maybe I can come up with something better that replaces all of that with a space (just for comparison) and then checks against that. hmmm.

Oh, and as for sp_executesql – this only allows parameters where parameters would normally be allowed (i.e. it’s not *AS* prone to SQL Injection unless it’s combined with EXEC) so, I can certainly add that BUT, I probably wouldn’t be as concerned about that…

SELECT OBJECT_NAME(object_id) AS [Procedure Name],
CASE
WHEN sm.definition LIKE ‘%EXEC%(%’ THEN ‘WARNING: code contains EXEC’
WHEN sm.definition LIKE ‘%EXECUTE%(%’ THEN ‘WARNING: code contains EXECUTE’
WHEN sm.definition LIKE ‘%sp_executesql%’ THEN ‘WARNING: code contains sp_executesql’
END AS [Dynamic Strings],
CASE
WHEN execute_as_principal_id IS NOT NULL THEN N’WARNING: EXECUTE AS ‘ + user_name(execute_as_principal_id)
ELSE ‘Code to run as caller – check connection context’
END AS [Execution Context Status]
FROM sys.sql_modules AS sm
ORDER BY [Procedure Name]

Here’s the latest (changing spaces to % so that I cut it down to one expression) and adding sp_executesql.

I’ve been trying to think of how I can use ^ but I can’t use it like % where I say any number of characters but NOT a-z or A-Z. But, I’m about to give up for today anyway. ;-) Your day has just started so maybe you’ll have this figured out before I get back up again?! ;-)