Feature Restrictions in SQL Server 2019 are Worse Than Useless: a False Sense of Security And Wasted Opportunity

(last updated: 2019-08-05 @ 19:10 EST / 2019-08-05 @ 23:10 UTC )

First, let me say how much I love some of the new functionality in SQL Server 2019:

1. Using In-Memory tables for [tempdb]
2. Adaptive inlining of T-SQL scalar functions
3. Optimizing indexes for sequential inserts (i.e. IDENTITY)
4. Better table variable statistics
5. and quite a few others…

However, nobody’s perfect and occasionally the SQL Server team makes a mistake. A new feature introduced in SQL Server 2019 CTP 3.2, “Feature Restrictions”, is an unfortunate example of such a mistake. It’s a tragically misguided attempt at improving security that not only increases the chances of SQL Injection, but it also prevented useful changes from being made. “Misguided” because it doesn’t even accomplish it’s stated goal, and “tragic” because it a) most likely increases the chances of SQL Injection, and b) used up the time that could have been spent on implementing useful changes.

What is “Feature Restrictions”?

Feature Restrictions accepts that Dynamic SQL exists and that code exists in your system that concatenates unchecked (or inadequately checked) user input into the query to be executed (as opposed to using parameters and/or thorough validations). The idea of this feature is to disable certain features used by hackers to gain knowledge of the system.

Ideally, application code is developed so it does not allow for SQL injection. However, in large code-bases that include legacy and external code, one can never be sure that all cases have been addressed, so SQL injections are a fact of life that we have to protect against.

The current two features available to restrict are: “ErrorMessages”, and “WaitFor”. Restricting “ErrorMessages” will mask system details in the error messages (e.g. table names, datatypes, etc), and “WaitFor” will simply disable the WAITFOR statement from causing any amount of waiting.

Unfortunately, the new “Feature Restrictions” feature is not going to protect any database against SQL Injection. Not only is “Feature Restrictions” absolutely worthless in that it doesn’t even do what it claims to do, but because it does not truly protect against these types of SQL Injection attacks, it gives a false sense of security to those who only read the official documentation and implement this feature thinking that they are then secure. That makes this feature actually quite dangerous, and the end result could actually be an increase in SQL Injection attacks.

Problems

1. The feature is meant to protect against SQL Injection, which can only happen in ad hoc query batches (i.e. Dynamic SQL), yet disabled features are disabled even in compiled contexts, such as stored procedures, where this is no chance of SQL Injection. I had originally thought that this was a problem, but then I got the following to work, so perhaps it cannot simply be ad hoc batches:

   SELECT * FROM sys.objects WHERE [is_ms_shipped] = 0; EXEC(N'CREATE PROCEDURE #Temp AS SELECT DB_NAME(); WAITFOR DELAY ''00:00:05'';'); EXEC(N'EXEC #Temp;');
   

2. While it is likely that this feature is enabled in Production only, sometimes it is hard to reproduce errors based on user descriptions of what happened, and so that error message might be quite useful to QA and/or support.
3. The “error messages” with masked internal information are only system generated error message. The output of RAISERROR and THROW is not masked! (example below)
4. Hiding error messages helps for scenarios where the error message is displayed, but isn’t it more common for the application to simply indicate that an error happened without showing any details? Either using a customer error page or even just a standard “500 Internal Server Error” page? In these scenarios, an attacker can infer from differences in behavior, between error and no error cases. And you cannot prevent all errors entirely (well, you could, I suppose, but that would be quite dangerous and foolish). Hence, the following cannot be prevented by this feature:

   IF (USER = 'dbo') SELECT 1 / 0;
   

   or this:

   IF (USER = 'dbo') SELECT * FROM dbo.[NoSuchTable];
   

5. WAITFOR is not the only means of causing a delay. It is possible to inject a simple loop that ends after a specified number of seconds, and does not cause any error. For example:

   IF (USER = 'dbo') BEGIN DECLARE @Now DATETIME = DATEADD(SECOND, 5, GETDATE()); WHILE (GETDATE() < @Now) DECLARE @Dummy INT; END;
   

   (example below)

6. EXEUTE AS 'dbo' or Login is member of sysadmin Fixed Server Role will prevent restriction:
   1. The WITH EXEUTE AS 'dbo' clause is often used when creating a stored procedure or function to grant rights to perform operations such as TRUNCATE TABLE. It is also sometimes used to get Dynamic SQL working since Dynamic SQL breaks ownership chaining, and we don't want to give direct table DML access to the application account. (examples below)
   2. Hopefully the application account isn't logging in as "sa" or anyone in the "sysadmin" fixed server role. But this does happen, and feature restrictions do not apply to any Login that is a member of the "sysadmin" fixed server role. This is important to note because the example given for the WaitFor restriction (in the documentation) is:

      IF (SYSTEM_USER = 'sa') WAITFOR DELAY '00:00:05' ...
      

      There are two significant problems with this example:

      1. "sa" is likely not the only account with "sysadmin" privileges. Anyone looking will be checking the following:

         SELECT IS_SRVROLEMEMBER(N'sysadmin', SYSTEM_USER)
         

      2. Any Login in the "sysadmin" role will not have either of these features restricted. Hence, if this condition is actually true, then the WAITFOR will behave normally, and will indicate that the current security context has "sysadmin" privileges.

   These are both yet more reasons to use Module Signing instead of EXECUTE AS!

7. There is a minor bug with the WaitFor restriction: security context gets cached the first time a query is executed that checks the current user. But, whatever is cached can be dropped automatically by the system (probably for a variety of reasons), or manually by executing:

   DBCC FREESYSTEMCACHE('ALL', 'default');
   

   I also tested with DBCC FREESYSTEMCACHE('ALL', 'internal'); and that had no effect.

   This is a minor bug because the scenarious that would be using this feature would almost certainly already have a query being executed before the WAITFOR is introduced. Still, a good security feature wouldn't rely so much on luck. (example below)

Examples

Setup

General

IF (SUSER_ID(N'_TestDbOwner') IS NULL)
BEGIN
    CREATE LOGIN [_TestDbOwner] WITH PASSWORD = 'NotVerySecure',
                                     CHECK_POLICY = OFF;
END;
IF (DB_ID(N'FeatureRestrictionTest') IS NULL)
BEGIN
    CREATE DATABASE [FeatureRestrictionTest]
        COLLATE Latin1_General_100_BIN2_UTF8;
    ALTER DATABASE [FeatureRestrictionTest] SET RECOVERY SIMPLE;
    ALTER AUTHORIZATION ON DATABASE::[FeatureRestrictionTest]
                        TO [_TestDbOwner];
END;
GO
USE [FeatureRestrictionTest];

CREATE LOGIN [_TestAppAccount] WITH PASSWORD = 'NotVerySecure',
                                    CHECK_POLICY = OFF;
CREATE USER [_TestAppAccount] FOR LOGIN [_TestAppAccount];

GO
CREATE OR ALTER PROCEDURE dbo.[ShowSecurityContext]
AS
SET NOCOUNT ON;
SELECT SYSTEM_USER AS [Login], [name], [type], [usage]
FROM   sys.login_token;
SELECT USER AS [User], [name], [type], [usage]
FROM   sys.user_token;
GO
GRANT EXECUTE ON dbo.[ShowSecurityContext] TO [public];

Add Restrictions

Please note that the capitalization of "waitfoR" and "useR" is intentional: I am testing in a DB using a binary collation to make sure that everything behaves as expected.

EXEC sp_add_feature_restriction
          N'waitfoR', N'useR', N'_TestAppAccount';
EXEC sp_add_feature_restriction
          N'ErrormessageS', N'useR', N'_TestAppAccount';

SELECT * FROM sys.sql_feature_restrictions;
/*
class    object             feature
User     _TestAppAccount    WaitFor
User     _TestAppAccount    ErrorMessages
*/

Test: "ErrorMessages" — RAISERROR and THROW aren't blocked

EXECUTE AS LOGIN = N'_TestAppAccount';
EXEC dbo.[ShowSecurityContext];

SELECT * FROM sys.objects WHERE [is_ms_shipped] = 0 AND CAST(DB_NAME() AS INT) = 0;
/*
Msg 245, Level 16, State 1, Line XXXXX
Conversion failed when converting the ****** value '******' to data type ******.
*/
SELECT * FROM sys.objects WHERE [is_ms_shipped] = 0; DECLARE @Stuff NVARCHAR(4000) = DB_NAME() + NCHAR(42) + ORIGINAL_LOGIN(); RAISERROR(@Stuff, 16, 1);
/*
(2 rows affected)
Msg 50000, Level 16, State 1, Line XXXXX
FeatureRestrictionTest*ALBRIGHTSolomon
*/
SELECT * FROM sys.objects WHERE [is_ms_shipped] = 0; DECLARE @XML NVARCHAR(4000) = (SELECT [name], [type], [usage] FROM sys.user_token FOR XML RAW('r')); RAISERROR(@XML, 16, 1);
/*
(2 rows affected)
Msg 50000, Level 16, State 1, Line XXXXX
<r name="Mr.HasLogin" type="SQL USER" usage="GRANT OR DENY"/><r name="public" type="ROLE" usage="GRANT OR DENY"/>
*/
DECLARE @Err NVARCHAR(4000) = DB_NAME();
;THROW 50505, @Err, 16;
/*
Msg 50505, Level 16, State 16, Line XXXXX
FeatureRestrictionTest
*/
REVERT;
EXEC dbo.[ShowSecurityContext];

Test: "ErrorMessages" — EXECUTE AS 'dbo' prevents restriction

Create Stored Procedures

GO
CREATE OR ALTER PROCEDURE dbo.[ErrorMessages]
AS
SET NOCOUNT ON;
EXEC dbo.[ShowSecurityContext];
SELECT [name]
FROM   sys.objects
WHERE  [is_ms_shipped] = 0
AND    CAST(DB_NAME() AS INT) = 0;
GO
CREATE OR ALTER PROCEDURE dbo.[ErrorMessages_ExecAsDBO]
WITH EXECUTE AS N'dbo'
AS
SET NOCOUNT ON;
EXEC dbo.[ShowSecurityContext];
SELECT [name]
FROM   sys.objects
WHERE  [is_ms_shipped] = 0
AND    CAST(DB_NAME() AS INT) = 0;
GO

GRANT EXECUTE ON dbo.[ErrorMessages] TO [_TestAppAccount];
GRANT EXECUTE ON dbo.[ErrorMessages_ExecAsDBO] TO [_TestAppAccount];

Execute Stored Procedures

EXECUTE AS LOGIN = N'_TestAppAccount';
EXEC dbo.[ShowSecurityContext];

EXEC dbo.[ErrorMessages]; -- masked
EXEC dbo.[ErrorMessages_ExecAsDBO]; -- not masked

REVERT;
EXEC dbo.[ShowSecurityContext];

Test: "WaitFor" — Simple WHILE loop to force a delay

IF (1 = 1)
BEGIN
    DECLARE @Now DATETIME = DATEADD(SECOND, 5, GETDATE());
    WHILE (GETDATE() < @Now)
    BEGIN
        DECLARE @Dummy INT;
    END;
END;

Test: "WaitFor" — EXECUTE AS 'dbo' prevents restriction

Create Stored Procedures

GO
CREATE OR ALTER PROCEDURE dbo.[WaitFor]
AS
SET NOCOUNT ON;
EXEC dbo.[ShowSecurityContext];
WAITFOR DELAY '00:00:03.000'; -- 3 seconds
GO
CREATE OR ALTER PROCEDURE dbo.[WaitFor_ExecAsDBO]
WITH EXECUTE AS N'dbo'
AS
SET NOCOUNT ON;
EXEC dbo.[ShowSecurityContext];
WAITFOR DELAY '00:00:03.000'; -- 3 seconds
GO

GRANT EXECUTE ON dbo.[WaitFor] TO [_TestAppAccount];
GRANT EXECUTE ON dbo.[WaitFor_ExecAsDBO] TO [_TestAppAccount];

Execute Stored Procedures

EXECUTE AS LOGIN = N'_TestAppAccount';
EXEC dbo.[ShowSecurityContext];

EXEC dbo.[WaitFor]; -- no delay
EXEC dbo.[WaitFor_ExecAsDBO]; -- 3-second delay

REVERT;
EXEC dbo.[ShowSecurityContext];

Test: "WaitFor" — Bug allows WAITFOR to cause delay again

EXECUTE AS LOGIN = N'_TestAppAccount';
EXEC dbo.[ShowSecurityContext];

-- Execute the following in another Query tab:
-- USE [master]; DBCC FREESYSTEMCACHE('ALL', 'default') WITH NO_INFOMSGS;
WAITFOR DELAY '00:00:03'; -- 3-second delay (NOT expected)

SELECT TOP (1) [name] FROM sys.objects;
WAITFOR DELAY '00:00:03'; -- no delay (expected)
-- If the above command did cause a delay, re-run
-- the "SELECT TOP (1)" just above it and then
-- re-execute this WAITFOR.

-- Execute the following in another Query tab:
-- USE [master]; DBCC FREESYSTEMCACHE('ALL', 'default') WITH NO_INFOMSGS;
WAITFOR DELAY '00:00:03'; -- 3-second delay again (NOT expected)

REVERT;
EXEC dbo.[ShowSecurityContext];

Clean-up / Tear-down

USE [master];
DROP DATABASE [FeatureRestrictionTest];
DROP LOGIN [_TestAppAccount];
DROP LOGIN [_TestDbOwner];

Fun Facts That Are Not Officially Documented Yet

Here are some behaviors I found in testing that are not yet in the documentation:

1. sp_drop_feature_restriction is not yet in IntelliSense.
2. "Feature Restrictions" cannot be used in three system databases: [master], [model], and [tempdb]:

   USE [master];
   EXEC sp_add_feature_restriction N'ErrorMessages', N'User', N'MyUser';
   
   USE [tempdb];
   EXEC sp_add_feature_restriction N'ErrorMessages', N'User', N'MyUser';
   
   USE [model];
   EXEC sp_add_feature_restriction N'ErrorMessages', N'User', N'MyUser';
   /*
   Msg 16305, Level 16, State 0, Procedure sp_add_feature_restriction, Line XXXXX [Batch Start Line YYYYY]
   The database does not support feature restrictions.
   */

3. "Feature Restrictions" can be used in one system database: [msdb]:

   USE [msdb];
   EXEC sp_add_feature_restriction N'ErrorMessages', N'User', N'MyUser';
   /*
   Msg 16301, Level 16, State 0, Procedure sp_add_feature_restriction, Line XXXXX [Batch Start Line YYYYY]
   User 'MyUser' not found.
   */

4. Cannot add "dbo" User to "Feature Restrictions":

   EXEC sp_add_feature_restriction N'waitfoR', N'useR', N'dbo';
   /*
   Msg 16309, Level 16, State 0, Procedure sp_add_feature_restriction, Line XXXXX [Batch Start Line YYYYY]
   Feature restrictions are not allowed for the dbo user.
   */

5. Restrictions do not apply to either of the following scenarios:
   1. module is created using WITH EXECUTE AS 'dbo' clause —
   2. Login is member of sysadmin Fixed Server Role —
6. "ErrorMessages" option does not mask custom error messages sent from:
   1. RAISERROR
   2. THROW

Can This Feature Be Fixed?

Sadly, no.

1. You can’t prevent all error output entirely (i.e. the entire message returned by RAISERROR or THROW) as that would make debugging extremely difficult, if not impossible.
2. You can’t prevent forcing an error, either a simple “divide by zero”, or selecting from an object that doesn’t exist. Even if error messages are not shown to the end user (this is usually the case, right?), isn’t there at least some indication of there at least being an error (again, this is usually the case, right?)?
3. You can’t prevent WHILE loops.
4. Hackers are more clever than this feature gives them credit for. If I was able to find all of these holes in this feature, you better believe hackers will find even more.

Clearly this feature was neither planned out well nor thoroughly tested.

Conclusion

This is a wasted opportunity in that I am guessing at least 50 – 100 developer hours were spent on adding this feature, between planning, development, testing, etc. Unfortunately, all of that time was wasted delivering a feature that will never do anything. And by “wasted” I mean that those hours could have been spent delivering real value to the SQL Server community. In that same amount of time, both of the following updates could have been made which would have benefited many thousands of users:

• Allow Asymmetric Key to be created from binary hex bytes string just like CREATE CERTIFICATE — this is merely a request to get the public key bytes from a VARBINARY literal string (or variable) instead of from a file or Assembly. There is nothing different to interpret as the bytes read from the 0x... string or variable is identical to whatever is in the file or Assembly (except that executable files and assemblies have extra bytes of program code to skip over, but the internal logic is all the same). No private key handling is necessary; we just need the Asymmetric Key created from the public key only.
• Allow SQLCLR to Connect to LocalDB instance using “(LocalDB)InstanceName” connection string in EXTERNAL_ACCESS instead of requiring UNSAFE — this is just a few minutes of work to add a single line of code to what eventually gets compiled as the sqlaccess.dll assembly:

  appDomain.SetData("ALLOW_LOCALDB_IN_PARTIAL_TRUST", true);
  

  That’s it! Just copy and paste that in and change “appDomain” to whatever it should be.

I do not want to be harsh as, again, I do love SQL Server and it has many more positive and great features than failings. But, this is still quite frustrating knowing that development time required to implement one or more very needed and definitely beneficial features was available but squandered, leaving us with nothing (or worse: a mess of a feature that will only serve to confuse and/or give the false sense of having security) when we could have easily had something.

This feature needs to be removed before people start using it and get a false sense of security, thinking that they are being protected against certain forms of SQL Injection.

Also See

• sys.sql_feature_restrictions
• sp_add_feature_restriction (no documentation page yet)
• sp_drop_feature_restriction (no documentation page yet)