Osprey
Osprey

Reputation: 1545

Safely execute a dynamic sql query

Consider the dangerous stored procedure below:

ALTER PROCEDURE [dbo].[ExecDynamicSQL] 
    @sqlToExec nvarchar(2000)
AS
BEGIN
    SET NOCOUNT ON;
    exec sp_sqlexec @sqlToExec;
END

I understand that this is very dangerous because it is very prone to SQL injection and that people could run malicious commands. However, I need to execute INSERT or UPDATE statements for which I don't have a fixed set of parameters and therefore I cannot pass individual parameters to the procedure.

Is there a way to somehow pass an array of name value pairs as a single parameter and then have the stored procedure safely build the query and execute it?

Is there an alternative and safe way to achieve this? I have considered splitting the query into Tablename, SET clause and WHERE clause sections (for update commands) and pass on 3 parameters accordingly, but I don't know if that would remove the risk of SQL Injection.

Upvotes: 0

Views: 2298

Answers (2)

Thom A
Thom A

Reputation: 95564

OK, so I wanted to do something "dumb" here, and I mean truly dumb. I wanted to show how crazy such an implementation would look to try and achieve what you really want; and this does.

A few notes about this:

  • Never use this in production.
  • Never use this in any environment other than a Sandbox, to try and understand it, and how stupid it is
  • I have only written a version for an INSERT. I have no interest in writing an UPDATE/Upsert version.
  • It only handles inserting 1 row at a time, no more, no less.
  • Never use this in production.
  • No, I won't be writing a version to support multiple rows.
  • This uses sql_variant and we all know you should never really use that.
  • If you don't understand this don't use it.
  • NEVER USE THIS IN PRODUCTION.
  • I am not explaining this works apart what the comments state in the answer.
  • I had to also create a function to get properly quoted object names
    • As such it should support user defined scalar data types
    • I have not tested if it does support user defined scalar data types
  • It used FOR XML PATH so that users on older versions can "test" it.
  • Did I mention, never use this in production?

So, there you have it. I won't be supporting this, I have no interest in supporting it, because you should NOT be using it. This was just something I felt like proving how dumb the idea is. It is.

CREATE DATABASE Demo;
GO
--Creating a new database for an easy "clean up"
USE Demo;
GO
--Single sample table
CREATE TABLE dbo.YourTable (SomeID int IDENTITY(1,1) NOT NULL,
                            SomeDate date NOT NULL,
                            SomeName nvarchar(30),
                            SomeNumber decimal(12,2),
                            EntryDate datetime2(1) NOT NULL DEFAULT SYSUTCDATETIME());

GO
--Create a type for inserting the data into
CREATE TYPE dbo.DataTable AS table (ColumnName sysname NOT NULL,
                                    ColumnValue sql_variant); --Yeah, you saw that right! sql_variant...
GO

--Create a function to return a delimit identified version of a sql_variant's data type
CREATE FUNCTION dbo.QuoteSqlvariant (@SQLVariant sql_variant) 
RETURNS nvarchar(258)
AS 
BEGIN
    RETURN QUOTENAME(CONVERT(sysname,SQL_VARIANT_PROPERTY(@SQLVariant,'BaseType'))) +
           CASE WHEN CONVERT(sysname,SQL_VARIANT_PROPERTY(@SQLVariant,'BaseType')) IN (N'char',N'varchar') THEN CONCAT(N'(',CONVERT(int,SQL_VARIANT_PROPERTY(@SQLVariant,'MaxLength')),N')')
                WHEN CONVERT(sysname,SQL_VARIANT_PROPERTY(@SQLVariant,'BaseType')) IN (N'nchar',N'nvarchar') THEN CONCAT(N'(',CONVERT(int,SQL_VARIANT_PROPERTY(@SQLVariant,'MaxLength'))/2,N')')
                WHEN CONVERT(sysname,SQL_VARIANT_PROPERTY(@SQLVariant,'BaseType')) IN (N'datetime2',N'datetimeoffset',N'time') THEN CONCAT(N'(',CONVERT(int,SQL_VARIANT_PROPERTY(@SQLVariant,'Scale')),N')')
                WHEN CONVERT(sysname,SQL_VARIANT_PROPERTY(@SQLVariant,'BaseType')) IN (N'decimal',N'numeric',N'time') THEN CONCAT(N'(',CONVERT(int,SQL_VARIANT_PROPERTY(@SQLVariant,'Precision')),N',',CONVERT(int,SQL_VARIANT_PROPERTY(@SQLVariant,'Scale')),N')')
                WHEN CONVERT(sysname,SQL_VARIANT_PROPERTY(@SQLVariant,'BaseType')) IN (N'varbinary') THEN CONCAT(N'(',CONVERT(int,SQL_VARIANT_PROPERTY(@SQLVariant,'TotalBytes'))-4,N')')
                ELSE N''
           END;
END
GO
--Sample outputs of the function for varying data types
SELECT dbo.QuoteSqlvariant(CONVERT(sql_variant,GETDATE())),
       dbo.QuoteSqlvariant(CONVERT(sql_variant,N'Hello')),
       dbo.QuoteSqlvariant(CONVERT(sql_variant,'Goodbye')),
       dbo.QuoteSqlvariant(CONVERT(sql_variant,CONVERT(varbinary(10),N'Hello'))),
       dbo.QuoteSqlvariant(CONVERT(sql_variant,CONVERT(varbinary(7),'Goodbye'))),
       dbo.QuoteSqlvariant(CONVERT(sql_variant,SYSDATETIME())),
       dbo.QuoteSqlvariant(CONVERT(sql_variant,SYSDATETIMEOFFSET())),
       dbo.QuoteSqlvariant(CONVERT(sql_variant,1.23)),
       dbo.QuoteSqlvariant(CONVERT(sql_variant,CONVERT(decimal(3,2),1.23)));
GO
--The "solution"
CREATE PROC dbo.CompletelyDynamicInsert @Schema sysname, @Table sysname, @Data dbo.DataTable READONLY, @EXEC nvarchar(MAX) = NULL OUTPUT, @SQL nvarchar(MAX) = NULL OUTPUT AS
BEGIN

    --Let the madness begin
    SET NOCOUNT ON;
    --First we need to create the initial INSERT INTO. This is the "Easy" part...
    DECLARE @CRLF nchar(2) = NCHAR(13) + NCHAR(10);
    SET @SQL = N'INSERT INTO ' + QUOTENAME(@Schema) + N'.' + QUOTENAME(@Table) + N' (' +
               STUFF((SELECT N',' + QUOTENAME(ColumnName)
                      FROM @Data
                      ORDER BY ColumnName ASC--Ordering is VERY important
                      FOR XML PATH(N''),TYPE).value('(./text())[1]','nvarchar(MAX)'),1,1,N'') + N')' + @CRLF +
               N'VALUES(';

    --Now for the VALUES clause
    SET @SQL = @SQL +
               STUFF((SELECT CONCAT(N',CONVERT(',dbo.QuoteSqlvariant(ColumnValue), N',@p', ROW_NUMBER() OVER (ORDER BY ColumnName ASC),N')',N' COLLATE ' + CONVERT(sysname,SQL_VARIANT_PROPERTY(ColumnValue,'Collation')))
                      FROM @Data
                      ORDER BY ColumnName ASC
                      FOR XML PATH(N''),TYPE).value('(./text())[1]','nvarchar(MAX)'),1,1,N'') + N');'
    --But we need to parmetrise this, so we need to generate a parmeters parameter
    DECLARE @Params nvarchar(MAX);
    SET @Params = STUFF((SELECT CONCAT(N',@p', ROW_NUMBER() OVER (ORDER BY ColumnName ASC), ' ', dbo.QuoteSqlvariant(ColumnValue))
                         FROM @Data
                         ORDER BY ColumnName ASC
                         FOR XML PATH(N''),TYPE).value('(./text())[1]','nvarchar(MAX)'),1,1,N'');
    
    --But, we can't just pass the values from @Data, no... Now we need a dynamic dynamic statement. Oh yay..?
    
    SET @EXEC = N'DECLARE ' + STUFF((SELECT CONCAT(N',@p', ROW_NUMBER() OVER (ORDER BY ColumnName ASC), ' ', dbo.QuoteSqlvariant(ColumnValue))
                                    FROM @Data
                                    ORDER BY ColumnName ASC
                                    FOR XML PATH(N''),TYPE).value('(./text())[1]','nvarchar(MAX)'),1,1,N'') + N';';
    SET @EXEC = @EXEC + @CRLF +
                STUFF((SELECT @CRLF + 
                              CONCAT(N'SET ',N'@p', ROW_NUMBER() OVER (ORDER BY ColumnName ASC),N' = (SELECT MAX(CASE WHEN ColumnName = N',QUOTENAME(ColumnName,''''),N' THEN CONVERT(',dbo.QuoteSqlvariant(ColumnValue), N',ColumnValue) END) FROM @Data);')
                       FROM @Data
                       ORDER BY ColumnName ASC
                       FOR XML PATH(N''),TYPE).value('(./text())[1]','nvarchar(MAX)'),1,2,N'');

    SET @EXEC = @EXEC + @CRLF + 
                N'EXEC sys.sp_executesql @SQL, @Params,' + 
                STUFF((SELECT CONCAT(N', @p', ROW_NUMBER() OVER (ORDER BY ColumnName ASC))
                       FROM @Data
                       ORDER BY ColumnName ASC
                       FOR XML PATH(N''),TYPE).value('(./text())[1]','nvarchar(MAX)'),1,1,N'') + N';';
    
    EXEC sys.sp_executesql @EXEC, N'@SQL nvarchar(MAX), @Params nvarchar(MAX), @Data dbo.DataTable READONLY', @SQL, @Params, @Data;

END;
GO

DECLARE @Data dbo.DataTable;
INSERT INTO @Data (ColumnName,ColumnValue)
VALUES(N'SomeDate',CONVERT(sql_variant,CONVERT(date,'20210101'))), --yes, the insert into this will look dumb like this. YOu need to explicitly convert them all to a sql_variant
      (N'SomeName',CONVERT(sql_variant,N'Larnu')),
      (N'SomeNumber',CONVERT(sql_variant,CONVERT(decimal(12,2),1732.12)));


DECLARE @EXEC nvarchar(MAX),
        @SQL nvarchar(MAX);
EXEC dbo.CompletelyDynamicInsert N'dbo',N'YourTable', @Data, @EXEC OUTPUT, @SQL OUTPUT;

PRINT @EXEC;
PRINT @SQL;
GO

SELECT *
FROM dbo.YourTable;
GO

USE master;
GO

DROP DATABASE Demo;

db<>fiddle

Upvotes: 0

Thom A
Thom A

Reputation: 95564

Although I have covered much of this in the comments, I felt it worthwhile giving an answer to give more of an explanation.

Firstly, as I have mentioned, this isn't a route you should be going down. Yes, you can have procedures that do use dynamic SQL, but these shouldn't be handling such basic things as inserting data into a table, or updating said data.

When using dynamic SQL, you need to first ensure that you are properly quoting your dynamic objects. For this that isn't too hard, you can just have a parameter for the object's schema and name and then when you inject them wrap them in QUOTENAME. The real problem comes from the latter, the "dynamic" columns.

Firstly, you seem to want a dynamic number of parameters; this is a major problem. You can't trivially, or even easily, parametrise dynamic parameters. You won't be able to pass these parameters as their correct type either; you wouldn't be able to pass a date as a date for example. I can imagine a solution that uses dynamic dynamic SQL (yes, I said dynamic twice), and the sql_variant object type, but should you be doing that? No. If you understood how to maintain such a solution I don't for one second think you would have asked the question you have; you would have something that is on it's way there, but needed some help.

So, what is the solution? Well, again, like I said in the comments, you should have separate procedures for each table. You might also want separate ones for INSERT and UPDATE operations, but you could also use a single one and implement "UPSERT" logic; there is plenty of good articles on how to do this so I won't cover it here.

As I mentioned in the comments as well, that means updating your procedures when you update your objects. That is normal. I routinely update procedures when an underlying table is updated to have more columns.

At the same time your application developers will then need to update their application code to ensure that pass the new parameters to your procedure. Good devops and relationships between your DBAs, SQL Developers and Application Developers is key, but that's all. Keep those communication channels open and active. When you or your DBA alters the table, adding the new column(s) and amended the objects indexes (if needed) in your development environment, and has informed you the SQL developer, you can then ALTER the needed procedures. You can then inform the Application Developer, and they can update the application code.

After that, complete your internal testing, fixe any bugs/unexpected behaviour/performance issues and then progress to the test environment. Get your users to confirm it works as required, and then off it goes to production. In other words, follow the basics of a good develop cycle.


TL;DR: The route you want is wrong, and is never going to scale. Stick to a normal development cycle, and update your database and application code in sync so that new functionality can be provided.

Upvotes: 3

Related Questions