Tag: create temp table 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.

Just In Case

There are some circumstances where you can use this approach in a non-harmful way. In general it’s something you should avoid, though, especially if you’re not sure how to tell if it’s harmful or not.

This is one of those unfortunate query patterns that leads to you getting called nights and weekends because the server blew up. I’ve seen it do everything from generate a reliably bad query plan, to adding just enough of an element of cardinality uncertainty that slight plan variations felt like parameter sniffing — even with no parameters involved!

Uncertainty is something we’ve covered quite a bit in this series, because if you don’t know what you want, SQL Server’s optimizer won’t either.

Up The Bomb

Let’s start with a reasonable index on the Posts table:

CREATE INDEX p ON
    dbo.Posts
(
    PostTypeId,
    OwnerUserId
);

We don’t need a very complicated query to make things bad, even with that stunningly perfect index in place.

SELECT
    c = COUNT_BIG(*)
FROM dbo.Users AS u
JOIN dbo.Posts AS p
    ON 1 = CASE     
               WHEN p.PostTypeId = 1
               AND  p.OwnerUserId = u.Id
               THEN 1
               WHEN p.PostTypeId = 2
               AND  p.OwnerUserId = u.AccountId
               THEN 1
               ELSE -1
           END
WHERE u.Reputation > 500000;

This returns a count of 3,374, and runs for about a minute total.

Naughty Query Plan

The plan for this may look innocent enough, but it’s one of those cases where a Lazy Table Spool is a warning sign.

Operator times in query plans are generally a blessing, because they show you where execution time ratchets up. Unfortunately, it’s not always clear what you have to do to fix the problems they show you.

I suppose that’s what posts like this are for, eh?

Oh, what a feeling.

Rewriting The Query

A Useful Rewrite© of this query looks something like this, at least if you’re a fancy-pants blogger who cares a lot about formatting.

SELECT
    c = COUNT_BIG(*)
FROM
(
    SELECT
        u.Id,
        u.AccountId
    FROM dbo.Users AS u
    WHERE u.Reputation > 500000 
) AS u
CROSS APPLY
(

    SELECT
        p.*
    FROM dbo.Posts AS p
    WHERE p.PostTypeId = 1
    AND   p.OwnerUserId = u.Id

    UNION ALL

    SELECT
        p.*
    FROM dbo.Posts AS p
    WHERE p.PostTypeId = 2
    AND   p.OwnerUserId = u.AccountId
) AS c;

I’m using a derived table here because if I used a Common Table Expression, you’d think they have some ridiculous magic powers that they really don’t, and one wouldn’t make the query any more readable.

Caring about formatting makes queries more readable.

Good game.

Nice Query Plan

You’ll notice that this plan no longer features a Table Spool, no longer runs for over a minute, and makes me happy.

I’m not a math major, but 181 milliseconds seems like a great improvement over 60 seconds.

Suede Shoes

This is another case of more typing for us results in a faster query. Perhaps there’s some wisdom to learning how to clearly express oneself before starting a career talking to databases.

Database Communications Major, or something. Just don’t buy it from Sally Struthers.

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.