CODEWITHSUNDEEP
javajdbcsql-injection
john
john

Reputation: 707

How to prevent SQL injection when generating DDL dynamically?

Goal: dynamically generate a PreparedStatement immune to SQL injection. 

    // This is a bad method. SQL injection danger . But it works 
    private PreparedStatement generateSQLBad(Connection connection, String tableName, 
        String columnName, String columnType) throws SQLException {
        String sql = "create table " + tableName + " (" + columnName + " " + columnType + ")";
        PreparedStatement create = connection.prepareStatement(sql);
        return create;
    }

    // I tried this. But it didn't work  
    private PreparedStatement generateSQLGood(Connection connection, String tableName, 
        String columnName, String columnType) throws SQLException {
        String sql = "create table ? (? ?)";
        PreparedStatement create = connection.prepareStatement(sql);
        create.setString(1, tableName);
        create.setString(2, columnName);
        create.setString(3, columnType);
        return create;
    }

How to dynamically generate PreparedStatement where user could choose tablename, columntype etc. and no danger of SQL injection?

Upvotes: 3

Views: 1592

Answers (1)

Bill Karwin
Bill Karwin

Reputation: 562270

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.

Upvotes: 3

Related Questions