views:

57

answers:

2

Hi guys,

simple problem, but perhaps no simple solution, at least I can't think of one of the top of my head but then I'm not the best at finding the best solutions.

I have a stored proc, this stored proc does (in a basic form) a select on a table, envision this:

SELECT * FROM myTable

okay, simple enough, except the table name it needs to search on isn't known, so we ended up with something pretty similiar to this:

-- Just to give some context to the variables I'll be using
DECLARE @metaInfoID AS INT
SET @metaInfoID = 1

DECLARE @metaInfoTable AS VARCHAR(200)

SELECT @metaInfoTable = MetaInfoTableName FROM MetaInfos WHERE MetaInfoID = @MetaInfoID

DECLARE @sql AS VARCHAR(200)
SET @sql = 'SELECT * FROM ' + @metaInfoTable

EXEC @sql

So, I, recognize this is ultimately bad, and can see immediately where I can perform a sql injection attack. So, the question is, is there a way to achieve the same results without the construction of the dynamic sql? or am I going to have to be super, super careful in my client code?

A: 

Given the constraints described, I'd suggest 2 ways, with slight variations in performance an architecture.

Choose At the Client & Re-Architect

I'd suggest that you should consider a small re-architecture as much as possible to force the caller/client to decide which table to get its data from. It's a code smell to hold table names in another table.

I am taking an assumption here that @MetaInfoID is being passed from a webapp, data access block, etc. That's where the logic of which table to perform the SELECT on should be housed. I'd say that the client should know which stored procedure (GetCustomers or GetProducts) to call based on that @MetaInfoID. Create new method in your DAL like GetCustomersMetaInfo() and GetProductsMetaInfo() and GetInvoicesMetaInfo() which call into their appropriate sprocs (with no dynamic SQL needed, and no maintenance of a meta table in the DB).

Perhaps try to re-architect the system a little bit.

In SQL Server

If you absolutely have to do this lookup in the DB, and depending on the number of tables that you have, you could perform a handful of IF statements (as many as needed) like:

IF @MetaInfoID = 1
 SELECT * FROM Customers

IF @MetaInfoID =2 
 SELECT * FROM Products
-- etc

That would probably become to be a nightmare to maintain.

Perhaps you could write a stored procedure for each MetaInfo. In this way, you gain the advantage of pre-compilation, and no SQL injection can occur here. (imagine if someone sabotaged the MetaInfoTableName column)

IF @MetaInfoID = 1
 EXEC GetAllCustomers
IF @MetaInfoID = 2
 EXEC GetAllProducts
p.campbell
Ah, unfortunately this wouldn't be practical, not with the system we have. Users effectively create the Meta Info Tables from the client, which are used to store extra, user definable, information against other items in the system. The current codebase creates a sql table under the hood matching the names of the meta info table they've created, so we do have to keep a track of these in another table. The system is also now quite big (due to it's several years of age) so a huge major change with it's architecture is currently not on the cards.....
Sekhat
... the sql injection problem is something I've come across as I now wish to expose the searching capabilities across these meta tags through a web service, which unfortunately means exposing the "MetaTableName" variable as a free string client side. Now sql injection may not happen at all with the above code, as it -- should -- fail if the metatable name doesn't exist in the metaInfoTables table, however I don't really wish to leave it upto chance.
Sekhat
+1  A: 

You have to use dynamic sql if you don't know the table name up front. But yes, you should validate the value before attempting to use it in an SQL statement.

e.g.

IF EXISTS(SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME=@metaInfoTable)
    BEGIN
        -- Execute the SELECT * FROM @metaInfoTable dynamic sql
    END

This will make sure a table with that name exists. There is an overhead to doing this obviously as you're querying INFORMATION_SCHEMA. You could instead validate the @metaInfoTable contains only certain characters:

-- only run dynamic sql if table name value contains 0-9,a-z,A-Z, underscores or spaces (enclose table name in square brackets, in case it does contain spaces)
IF NOT @metaInfoTable LIKE '%^[0-9a-zA-Z_ ]%'
    BEGIN
        -- Execute the SELECT * FROM @metaInfoTable dynamic sql
    END
AdaTheDev
Well it does seem the best thing to do. Many thanks :)
Sekhat