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
PHP $_Post Array Empty Upon Form Submission
I Have 2 Dates in PHP, How to Run a Foreach Loop to Go Through All of Those Days
MySQL Connection Not Working: 2002 No Such File or Directory
Working With Large Numbers in PHP
PHP Preg_Match With Working Regex
How to Choose an Authentication Library For Codeigniter
Creating One Array from Another Array in PHP
PHP File_Get_Contents() and Setting Request Headers
Best Way to Avoid Duplicate Entry into MySQL Database
How to Create a Comma-separated List from an Array in PHP
Convert Time in Hh:Mm:Ss Format to Seconds Only
Continue Processing PHP After Sending Http Response
Access Array Returned by a Function in PHP
Is There a "Nullsafe Operator" in PHP
Why Can't I Run Two MySQLi Queries? The Second One Fails
How to Get the Query Builder to Output Its Raw SQL Query as a String