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

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?

• anon: You create your table using @terminal_name as the table name. Isn't @terminal_name always going to be the same as @table_name? I don't see why the second query of sys.tables to do the assignment to @table_name is necessary?
• Martin Riddar: The table name is supposed to be @terminal_name + '_raw_log'. First I tried to just do 'create table [lake].' + quotename(@terminal_name) + '_raw_log' but that just threw errors. I guess the name becomes [lake].[nds]_raw_log which isn't valid.
• Erik Reasonable Rates Darling: Why not change the code to + quotename(@terminal_name + '_raw_log') then?

Answer 1

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)

• Martin Riddar: For some reason I got hung up on the fact that I wasn't able to do quotename(@terminal_name) + '_raw_log for the table name and constraint instead of realizing I could just put it in variable first.
• anon: You can do it without the extra variable, too. Just by doing the concatenation INSIDE the parens when you call QUOTENAME(). Notice how I concatenate the PK_ to the @table_name
• Martin Riddar: Well that's why I asked the question here, couldn't see the forrest for the trees ;)