How to Prevent SQL Injection With Dynamic Tablenames

How to prevent SQL injection when the statement has a dynamic table name?

JDBC, sort of unfortunately, does not allow you to make the table name a bound variable inside statements. (It has its reasons for this).

So you can not write, or achieve this kind of functionnality :

connection.prepareStatement("SELECT * FROM ? where id=?", "TUSERS", 123);

And have TUSER be bound to the table name of the statement.

Therefore, your only safe way forward is to validate the user input. The safest way, though, is not to validate it and allow user-input go through the DB, because from a security point of view, you can always count on a user being smarter than your validation.
Never trust a dynamic, user generated String, concatenated inside your statement.

So what is a safe validation pattern ?

Pattern 1 : prebuild safe queries

1) Create all your valid statements once and for all, in code.

Map<String, String> statementByTableName = new HashMap<>();
statementByTableName.put("table_1", "DELETE FROM table_1 where name= ?");
statementByTableName.put("table_2", "DELETE FROM table_2 where name= ?");

If need be, this creation itself can be made dynamic, with a select * from ALL_TABLES; statement. ALL_TABLES will return all the tables your SQL user has access to, and you can also get the table name, and schema name from this.

2) Select the statement inside the map

String unsafeUserContent = ...
String safeStatement = statementByTableName.get(usafeUserContent);
conn.prepareStatement(safeStatement, name);

See how the unsafeUserContent variable never reaches the DB.

3) Make some kind of policy, or unit test, that checks that all you statementByTableName are valid against your schemas for future evolutions of it, and that no table is missing.

Pattern 2 : double check

You can 1) validate that the user input is indeed a table name, using an injection free query (I'm typing pseudo sql code here, you'd have to adapt it to make it work cause I have no Oracle instance to actually check it works) :

select * FROM 
(select schema_name || '.' || table_name as fullName FROM all_tables)
WHERE fullName = ?

And bind your fullName as a prepared statement variable here. If you have a result, then it is a valid table name. Then you can use this result to build a safe query.

Pattern 3

It's sort of a mix between 1 and 2.
You create a table that is named, e.g., "TABLES_ALLOWED_FOR_DELETION", and you statically populate it with all tables that are fit for deletion.

Then you make your validation step be

conn.prepareStatement(SELECT safe_table_name FROM TABLES_ALLOWED_FOR_DELETION WHERE table_name = ?", unsafeDynamicString);

If this has a result, then you execute the safe_table_name. For extra safety, this table should not be writable by the standard application user.

I somehow feel the first pattern is better.

SQLinjection concerns when dynamic table variable is needed in Java query string

I'll preface what I'm about to say by pointing out that I am not a security analyst.

I don't think what you've done here is either effective or necessary. First, SQL injection depends on the parser reading to the end of the first valid SQL statement and executing it before continuing. So if the above code were vulnerable at all, (and I don't think it is, but I'll cover that in a moment,) your query

select '"+t+"' as table_name, data, date from " + t +" where data = ?

could be attacked by setting t equal to

1' from pg_users; drop table pg_users;

and the parser would execute the SELECT, then execute the DROP, then choke on the AS, but by then it would be too late. That's how SQL injection works.

The fact that you're using a prepared statement, though, means that the entire query string has to be parsed from one end to the other before any of it is executed, and any attempt to inject through t will almost certainly leave invalid syntax at the end of the query which would choke the parser before any of it could be executed. Even if that were not the case, your table name is coming from your own code, picked from a preset list, and if that's been compromised you have bigger problems to worry about than SQL injection.

Writing secure code is good, but armour-plating your code makes it slow and difficult to maintain and can rarely be justified.

One alternative you might consider, though, was used on a commercial product I worked on a few years ago. That alternative is to store the queries for the 50 tables in another table, perhaps indexed by the table name. The code might look something like this:

String stm = "SELECT query FROM table_queries WHERE table_id = ?";
pst = con.prepareStatement(stm);
pst.setInt(1, tableID);
rs = pst.executeQuery();
stm = rs.first().getString("query");
pst = con.prepareStatement(stm);
pst.setInt(1, 100);
rs = pst.executeQuery();

Avoid SQL Injections on query with tablename

I question why you are doing this, but you can look at sys.tables for a conclusive whitelist.

DECLARE @TableName VARCHAR(100) = 'Table to Look for';
DECLARE @Exists BIT = ( SELECT CAST( COUNT(1) AS BIT ) FROM sys.tables WHERE name = @TableName AND type = 'U' );

You could parameterize the initial input, but the whitelist approach is still important. Otherwise, a malicious user could pass any valid table name in the entire database and the query would run against it (assuming they had SELECT permissions).

How can I prevent SQL injection with dynamic tablenames?

Your advice is indeed incorrect.

mysql_real_escape_string() will not work for dynamic table names; it is designed to escape string data, delimited by quotes, only. It will not escape the backtick character. It's a small but crucial distinction.

So I could insert a SQL injection in this, I would just have to use a closing backtick.

PDO does not provide sanitation for dynamic table names, either.

This is why it is good not to use dynamic table names, or if one has to, comparing them against a list of valid values, like a list of tables from a SHOW TABLES command.

I wasn't really fully aware of this either, and probably guilty of repeating the same bad advice, until it was pointed out to me here on SO, also by Col. Shrapnel.

How to prevent SQL injection when generating DDL dynamically?

You can't use ? parameter placeholders for identifiers (table names and column names). Nor can they be used for SQL keywords, like data types. Preparing a query needs to be able to validate the syntax, and validate that your table names and so on are legal. This must be done at prepare time, not at execute time. SQL doesn't allow parameters to contain syntax. They are always treated as scalar values. That's how they protect against SQL injection.

So parameters can only be used in place of scalar literals, like quoted strings or dates, or numeric values.

What to do for dynamic identifiers? As the comments suggest, the best you can do is filter the inputs so they're not going to introduce SQL injection. In a way, dynamic SQL based partially on user input is SQL injection. You just need to allow it in a controlled way.

All SQL implementations allow you to use special characters in your table names if you delimit the identifier. Standard SQL uses double-quotes for delimiters. MySQL uses back-ticks, and Microsoft SQL Server uses square brackets.

The point is that you can make strange-looking table names this way, like table names containing spaces, or punctuation, or international characters, or SQL reserved words.

CREATE TABLE "my table" ( col1 VARCHAR(20) );

CREATE TABLE "order" ( col1 VARCHAR(20) );

See also my answer to https://stackoverflow.com/a/214344/20860

But what if the table name itself contains a literal double-quote character? Then you must escape that character. Either use a double character or a backslash:

CREATE TABLE "Dwayne ""The Rock"" Johnson" ( col1 VARCHAR(20) );

CREATE TABLE "Dwayne \"The Rock\" Johnson" ( col1 VARCHAR(20) );

You could alternatively design your function to check the dynamic table name for such characters, and either strip them out or throw an exception.

But even if you make the statement safe by carefully filtering the input, this might not satisfy the checkmarx warning. SQL injection testers have no way of analyzing your custom code to be sure it filters input reliably.

You may just have to do your best to make the dynamic SQL safe, knowing that checkmarx will always complain about it. Write comments in your code explaining your safety measures to future developers who read your code.

Also write unit tests to ensure that dangerous inputs result in either safe DDL statements, or else exceptions.



Related Topics



Leave a reply



Submit