Software Vendor Mistakes With SQL Server: Writing Unsafe Dynamic SQL

SQL Injection


The first thing that comes to mind when you mention dynamic SQL is SQL injection. That makes sense! It’s a scary thing that has messed up a lot of people, places, and things.

Imagine how scary it would be to wake up one day and find there had been a data breach, defacement, server takeover, or ransomeware-d.

All of these things are possible if you write the bad kind of dynamic SQL, or think that typing square brackets will save you.

If I had my way, there would be more way more invested in making safe dynamic SQL easy to write, debug, and maintain. But I don’t have my way, and Microsoft is more focused on defrauding you with shoddy cloud performance than anything else.

To define the problem:

  • You write code that accepts user input
  • The input is a string with a non-arbitrary length
  • Which is concatenated into another string for execution
  • And not parameterized, properly quoted, etc.

Building A String


This can be done in the application, in SQL Server, or a mix of the two.

Since I’m not an application developer, I’m going to scope this to how it looks in a T-SQL stored procedure.

CREATE OR ALTER PROCEDURE
    dbo.haxer
(
    @injectable nvarchar(100)
)
AS
BEGIN
   SET NOCOUNT, XACT_ABORT ON;

/*Our placeholder variable for the string we'll build*/
DECLARE    
    @sql nvarchar(MAX) = N'';

/*The start of our query-building*/
SELECT
    @sql = N'
SELECT
    H.*
FROM dbo.hax AS H
WHERE 1 = 1
';

/*
This is where things first go wrong,
concatenating input into the string
*/
IF @injectable IS NOT NULL
BEGIN
    SELECT
        @sql += N'AND H.user_name = ''' + @injectable + ''';';
END;

PRINT @sql;

/*
This is where things continue to go wrong, 
using EXEC against the variable
*/
EXEC(@sql);

END;
GO 

There’s not much complicated here. The comments tell the story:

  • Concatenating the @injectable parameter into the string means it can bring anything the user wants into your query
  • Using EXEC directly against the @sql variable means it can’t be parameterized

Before we can run this, though, we need a table and some dummy data.

Dummy Data And String Things


Here’s a simple table, with a single row in it. I don’t think this needs much explanation.

CREATE TABLE
    dbo.hax
(
    id int PRIMARY KEY,
    user_name nvarchar(100),
    points bigint,
    location nvarchar(250),
    creation_date datetime,
    last_login datetime
);

INSERT 
    dbo.hax
(
    id,
    user_name,
    points,
    location,
    creation_date,
    last_login
)
VALUES
(
    1,   
    N'Rick Ross',
    400000000,
    'Port Of Miami',
    '20060808',
    '20211210' 
);

As a note here, this is another good reason to reasonably scope string column lengths. The shorter they are, the shorter search parameters are, and that limits the amount of potentially malicious code that can be stuffed into them. A 10-byte length string is a lot harder to cause problems with than a string with a 50-byte length, and so on. I’ve oversized a couple columns here as examples.

Is there any value in a 100 character user name, or a 250 character location? I sort of doubt it.

Executed Normally


When we execute the stored procedure as-is, searching for our lone user, we get a single result back:

EXEC dbo.haxer
    @injectable = N'Rick Ross';

The current query result isn’t important, but the result of the PRINT statement is.

SELECT
    H.*
FROM dbo.hax AS H
WHERE 1 = 1
AND H.user_name = 'Rick Ross';

There are two issues, here because of the lack of parameterization

  • It can lead to generating multiple execution plans for the same query
  • Users can substitute the legitimate search with a malicious query

Unfortunately, neither the Optimize For Ad Hoc Workloads nor the Forced Parameterization settings can protect you from SQL injection. Forced parameterization can help you with the second problem, though.

Executed Abnormally


When we execute the stored procedure with a slightly different value for the parameter, we get into trouble:

EXEC dbo.haxer
    @injectable = N'%'' UNION ALL SELECT object_id,name,schema_id,type_desc,create_date,modify_date FROM sys.tables --';

We’re not trying to do anything too crazy here, like drop a table or a database, change a setting, or other thing that you might need a lot of control of the server, database, or schema to do. We’re querying a pretty standard system view.

Now, I’m not big on security, permissions, or any of that stuff. It has never captured my fancy, so I don’t have any great advice about it. I will make a few points though:

  • Most applications have a single login
  • A lot of the time that login is given sa-level permissions
  • Even when it’s not, that login may need to do routine database things:
    • Check if databases, tables, columns, or indexes exist (especially during upgrades)
    • Create or drop all of those things
    • Execute database maintenance procedures
    • Mess with settings or Agent jobs

Without really tight control of a whole bunch of permissions, you’re gonna end up with applications that can do a ton of crazy stuff, including a list of tables in the database from this query.

SELECT
    H.*
FROM dbo.hax AS H
WHERE 1 = 1
AND H.user_name = '%' UNION ALL SELECT object_id,name,schema_id,type_desc,create_date,modify_date FROM sys.tables--';

All of this fits well within the 100 characters available in the search parameter, and the entire query is executed as one.

From here, a mean person could take one of the tables names and list out the columns (using the sys.columns view), and then use that to grab data they wouldn’t normally be allowed to see.

This how user data ends up on the dark web. Passwords, credit cards, social security numbers, phone numbers, addresses, childhood fears. You name it.

Protecting Yourself


Sure, you can steam yourself up and encrypt the crap out of everything, not store certain personal or processing details in the database, or a number of other things.

But before you go and dump out the bag of golden hammers, let’s start by eliminating the risks of dynamic SQL. We’re gonna need to rewrite our stored procedure a bit to do that.

CREATE OR ALTER PROCEDURE
    dbo.no_hax
(
    @injectable nvarchar(100)
)
AS
BEGIN
   SET NOCOUNT, XACT_ABORT ON;

/*Our placeholder variable for the string we'll build*/
DECLARE    
    @sql nvarchar(MAX) = N'';

/*The start of our query-building*/
SELECT
    @sql = N'
SELECT
    H.*
FROM dbo.hax AS H
WHERE 1 = 1
';

/*
This changed: the parameter is embedded in the string
instead of being concatenated into it
*/
IF @injectable IS NOT NULL
BEGIN
    SELECT
        @sql += N'AND H.user_name = @injectable;';
END;

PRINT @sql;

/*
This also changed: we're using EXEC against
the system stored procedure sp_executesql,
and feeding it parameters 
*/
EXEC sys.sp_executesql
    @sql,
  N'@injectable nvarchar(100)',
    @injectable;

END;
GO

In this version, here’s what we’re doing different:

  • The parameter is inside of the string, instead of concatenated in from outside
  • We’re using the system stored procedure sp_executesql to execute the string we’ve built up

It’s important to note that just using sp_executesql won’t protect you from SQL injection attacks unless the strings are parameterized like they are here.

Any Execution


No matter how we execute this, the query that gets generated and executed will look the same.

EXEC dbo.no_hax
    @injectable = N'Rick Ross';

EXEC dbo.no_hax
    @injectable = N'%'' UNION ALL SELECT object_id,name,schema_id,type_desc,create_date,modify_date FROM sys.tables--';
SELECT
    H.*
FROM dbo.hax AS H
WHERE 1 = 1
AND H.user_name = @injectable;

The important thing here is that the second execution does not result in successful SQL injection, but the first query returns correct results.

The second query returns no results this time, because the entire search string is parameterized, and no names match the supplied value.

Thanks for reading!

Going Further


If this is the kind of SQL Server stuff you love learning about, you’ll love my training. I’m offering a 75% discount to my blog readers if you click from here. I’m also available for consulting if you just don’t have time for that and need to solve performance problems quickly.



3 thoughts on “Software Vendor Mistakes With SQL Server: Writing Unsafe Dynamic SQL

  1. Anyone know of a script or tool that will look at tables and parameters used in a Dynamic SQL statement and create the sp_executesql statement with all its parameters? Sometimes that can be a pain staking process to get right.

    1. Yep! sp_GRUDGen (http://spcrudgen.org). It is an open-source project that creates stored procedures based on your tables. (https://github.com/kevinmartintech/sp_CRUDGen).

      If you use the @GenerateSearch=1 parameter it will create a dynamic search stored procedure for @SchemaTableName=N’dbo.MyTableName’. It will turn the columns from your table into parameters in the stored procedure.
      In about two weeks, there will be 8-9 upgrades including temporal table and view support you can check out in the Issues tab on the GitHub project site.

Comments are closed.