Allow create table (dynamic name) and insert but nothing else

February 14, 2022 Andy DBA Stack Exchange 0

clip art of a double-quote character

Question

I’m trying to find a way to allow an application to create tables and insert data into them on a SQL Server 2019 while protecting from injection attacks in case the app credentials would leak. My experience is limited when it comes to writing code that can run in parallel and writing dynamic sql that is protected from sql injection attacks.
The table name is based on input from the application, i.e. if the input is ‘nds’ the table name should be lake.nds_raw_log.
It is my understanding that there is no way to do this via directly granting permissions to the role for this application since creating tables is not separated from deleting or altering them.

What I’ve come up with is executing a stored procedure as dbo. Sure it’s not long but I have two issues with it:

  • it feels contrived which by my experience says that there is an easier way.
  • I believe that I need to run it as serializable to avoid orphan tables if I retrieve the wrong table when I query for my newly created table. This shouldn’t actually be that big of an issue since it won’t happen that often after the first start in production so maybe I shouldn’t care about it.
    create procedure [lake].[create_terminal_raw_log_table]
    (
        @terminal_name nvarchar(100)
    )
    with execute as 'dbo'
    as
        begin try
            set transaction isolation level serializable
            begin transaction
                --create table
                declare @dynamic_sql nvarchar(1000) =
                    'create table [lake].' + quotename(@terminal_name) + '
                    (
                        id bigint not null,
                        [timestamp] datetime2(3) not null,
                        cmd varbinary(max) not null
                    );'
                exec sp_executesql @dynamic_sql
    
                /*get name of new table, this is why I believe that I need serializable isolation
                since other tables can be created in parallel*/
                declare @table_name nvarchar(100) =
                (
                    select top 1
                        [name] as table_name
                    from sys.tables
                    order by create_date desc
                )
        
                --rename table
                declare
                    @old_name nvarchar(100) = '[lake].' + @table_name,
                    @new_name nvarchar(100) = @table_name + '_raw_log'
                begin try
                    exec sp_rename
                        @objname = @old_name,
                        @newname = @new_name
                end try
                begin catch
                    set @dynamic_sql = 'drop table ' + @old_name
                    exec sp_executesql @dynamic_sql
                    ;throw
                end catch
    
                --create primary key
                set @dynamic_sql = 'alter table [lake].' + @new_name + ' add constraint pk__' + @new_name + ' primary key(id)'
                exec sp_executesql @dynamic_sql
    
            commit transaction
        end try
        begin catch
            rollback --I thought a rollback would occur when I throw after dropping the table but that doesn't seem to be the case
            ;throw
        end catch

So I guess this boils down to 3 questions:

  • Is this stored procedure actually safe from SQL injection attacks?
  • Is there an easier way to do it?
  • Is it correct that setting the transaction level as serializable will protect the code from selecting the wrong table when selecting from sys.tables?

asked 2022-02-14 by Martin Riddar

Answer

Unless I am missing a business requirement, or a detail in your code, I think you’re making your procedure a whole lot more complicated than necessary.

There’s not a need to create the table, then look up the name of the table, then rename the table, then add the primary key constraint all as separate steps, wrapped in a transaction to ensure consistency. Instead you can do it all in one step.

A few other code-review type notes on your code:

  • You are using a nvarchar variables to support unicode, but doing the assignment using "regular" single quotes. To support unicode strings, you’ll need to use the N' prefix to quote unicode strings.
  • There is an inline constraint creation that is possible for CREATE TABLE
  • You should use the schema prefix whenever possible when referencing objects, including using the sys schema prefix on sp_executesql.
  • Never rely on the ordinal position of a stored procedure. Explicitly name the parameters as you pass them. Most developers (including Microsoft for system stored procedures) avoid changing the position of parameters–but if they do, it will break your code if you assume the position. Named parameters never has that problem.

Here’s my version of your procedure:

CREATE OR ALTER PROCEDURE [lake].[create_terminal_raw_log_table]
    (
        @terminal_name nvarchar(100)
    )
WITH EXECUTE AS 'dbo'
AS
    DECLARE @table_name nvarchar(128);
    DECLARE @dynamic_sql nvarchar(1000);
    
    -- We want the table name to be the input value with `_raw_log` appended:
    -- I could skip even using this variable, 
      -- and just use `@terminal_name + N'_raw_log'` 
      -- in the two spots I reference @table_name
      -- but if you use the table name a bunch of times, 
      -- this is easier.
    SET @table_name = @terminal_name + N'_raw_log';

    --Use dynamic SQL to create the table
        --With the PK Constraint created in-line.
    SET @dynamic_sql = N'create table [lake].' + QUOTENAME(@table_name) + N'
                    (
                        id bigint not null,
                        [timestamp] datetime2(3) not null,
                        cmd varbinary(max) not null,
                        CONSTRAINT ' + QUOTENAME(N'PK_' + @table_name) + N' 
                            PRIMARY KEY CLUSTERED (id)
                    );';
    EXEC sys.sp_executesql @stmt = @dynamic_sql;
GO

Make sure you test!

You’ll want to do some quick sanity tests to make sure that your procedure actually works. I like to make sure that I test with unicode characters (I always use emojis), and any other specific concerns (like SQL injection, white space in object names, min or max length, etc).

For example:

EXEC [lake].[create_terminal_raw_log_table] @terminal_name = N'nds';
EXEC [lake].[create_terminal_raw_log_table] @terminal_name = N'amtwo';
EXEC [lake].[create_terminal_raw_log_table] @terminal_name = N'; DROP PROCEDURE [lake].[create_terminal_raw_log_table];';
EXEC [lake].[create_terminal_raw_log_table] @terminal_name = N'It Works!! 🎉🎉';

SELECT 
    TableName = o.[name]
FROM sys.objects AS o 
JOIN sys.schemas AS s ON s.schema_id = o.schema_id
WHERE s.name = N'lake'
AND o.type = 'U';

Returns these results:

TableName
-------------------------------------------------------------------
nds_raw_log
amtwo_raw_log
; DROP PROCEDURE [lake].[create_terminal_raw_log_table];_raw_log
It Works!! 🎉🎉_raw_log

(4 rows affected)

answered 2022-02-14 by Andy Mallon