0

I'm using a stored procedure to generate a create table script on the fly based on parameters passed into it. I am validating that the server exists, and then validating if the table exists in the database on said server.

So, first I check if the server exists:

Exists(SELECT 1 FROM sys.servers WHERE name = @Server) 

If it passes this check I go on to build a string that points to Information_schema:

set @InformationSchemaPath = @Server + '.' + @DataBaseName + '.' +  'information_schema' 

and then use that string in a dynamic SQL statement that gets put into a string:

set @DynamicSQL = ('SELECT count(*) 
             FROM ' +  @InformationSchemaPath + '.Tables
             WHERE  TABLE_NAME = ''' + @TableName + '''')

The results get inserted into a temp table:

INSERT INTO @receiver Exec (@DynamicSQL )

I am unable to use SP_EXECUTESQL. I am stuck using Exec.

migrated from codereview.stackexchange.com Jul 10 '14 at 16:40

This question came from our site for peer programmer code reviews.

2

Yes, this is unsafe. Try passing this:

@TableName = 'FakeTableName'';DROP TABLE Foo;PRINT''You have been Pwned'

One way to avoid this is to pass @TableName as a parameter to the dynamic SQL. Like so:

SET @DynamicSQL = '... WHERE TABLE_NAME = @TableName'

And then do this:

INSERT INTO @Receiver EXEC sp_executesql @DynamicSQL, @TableName

From the wording of your question, I am not sure if you are saying that you can't run sp_executesql or that you are not sure about the syntax. If you have to run the statement using only EXEC there is a method that works, but only on a Linked Server. You do do this:

SET @DynamicSQL = '... WHERE TABLE_NAME = ?'

Followed by this:

INSERT INTO @Receiver EXEC (@DynamicSQL, @TableName) AT YourLinkedServer
1

Yes it is still vulnerable. The @DatabaseName parameter can include code that terminates one command and starts another. You are creating a string based on @DatabaseName and then executing it. That is a classic SQL injection vulnerability.

  • I neglected to include that that statement is wrapped in the Exist statement. Does this effect running more than one command? – Elias Jul 10 '14 at 16:49
  • I am not positive, but I would think that given enough effort a malicious person could abuse it. I will run some tests. – datagod Jul 10 '14 at 17:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.