Allow create table (dynamic name) and insert but nothing else
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?
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
nvarcharvariables 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
- https://sqlblog.org/2019/09/12/bad-habits-to-kick-avoiding-the-schema-prefix , including using the
sysschema prefix on
- 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;
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).
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!! ';
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)
- You are using a