CODEWITHSUNDEEP
c#sqlsql-serveroracle-database
JMarsch
JMarsch

Reputation: 21753

Defending vs an Insert SQL Injection attack

So we have a custom logger (really wanted to use Log4Net, just wouldn't work for this case). From a config file, you can configure the name of the table that will be inserted into.

I am parameterizing the insert statement, but the configurable table name will be used in an insert statement, and it's a potential vector for attack. Here's an example of the statement that will be built:

"insert into " + theTableName + " (static column list) values(parameterized list of values)"

So my inserted values are parameterized, and pretty safe, it's theTableName that could contain nasties.

My question is, what can I do to sanitize the table name? I think the nature of the code an attacker would inject would have to be a little different from the garden-variety. -- instead of a tick, you would potentially close off the statment with "table name() values(); do something bad --

(or something like that, I suppose). To that end, I was thinking about checking for the ";", and the "--".

Can anyone suggest a better way to sanitize this? (this will be used with Oracle and SQL server)

Upvotes: 0

Views: 222

Answers (5)

Allan
Allan

Reputation: 17429

The obvious solution to me would be to validate the table name that you're going to use in the dynamic SQL using static SQL first:

select * from sys.tables where name = @thetablename

Since this query binds the variable rather than concatenating it, it should be immune to SQL injection. If the query returns a record then continue, otherwise handle it as an error.

Upvotes: 1

KM.
KM.

Reputation: 103587

why not use a theTableID numeric ID value to a row where you store a table name theTableName. You can store this numeric ID value in the config file on the user's PC and pass it into the application. Then you can look it up, via a parametrized query, and then concatenate the actual string table name into the query. That will be safe.

Upvotes: 1

Ryan Roper
Ryan Roper

Reputation: 333

I guess the main question is why theTableName variable is even being exposed to the person using the page that the logging is for.

Barring that, a whitelist check either against a set array of values or against a set that you create on the fly by querying the system catalog for table names and dumping them into an array and thing just doing a .Contains(theTableName) on the array or whatever the equivalent for your language of choice is probably the safest way to go.

Upvotes: 0

Larry Lustig
Larry Lustig

Reputation: 50970

I think you have a potentially bigger problem, which is that the user could specify a syntactically safe and valid table name that simply doesn't exist in the database.

Therefore, I'd do this:

  1. Build the INSERT string (including the table name) only once, when you load the config file.

  2. Verify the table name at that time by checking the system metadata to ensure that it's a real, valid table.

At that time you can warn the user that logging is off because the table "theTableName" does not exist in the database.

Upvotes: 1

Jens Erat
Jens Erat

Reputation: 38682

That still doesn't keep you safe from UNION-attacks (and several more). Best do some whitelist-check for latin letters or similiar if you know what's allowed.

Other characters to check for would be 

()`"'

and whitespace.

Upvotes: 2

Related Questions