Is this function vulnerable to SQL injection?

Question

I built a function to check if a table exists on a database using PDO, but I'm not sure if I've secured it properly.

public function tableExists($table){
    try{
        $this->query('SELECT 1 FROM `'.str_replace('`', '', $table).'` LIMIT 1');
    }catch(\PDOException $e){
        if($e->errorInfo[1] == 1146){
            return false;
        }
        throw $e;
    }
    return true;
}

Is it possible for an attacker to compromise the query if $table is provided directly from user input? (extreme case)

Answer 1

NO, your code is not vulnerable to SQL injection attack

However, that is perhaps more by fluke than anything else.

To handle user-provided values correctly, one would normally wish to escape any characters that have special meaning (rather than simply remove them). As documented under Schema Object Names:

Identifier quote characters can be included within an identifier if you quote the identifier. If the character to be included within the identifier is the same as that used to quote the identifier itself, then you need to double the character. The following statement creates a table named a`b that contains a column named c"d:

mysql> CREATE TABLE `a``b` (`c"d` INT);

So how can we permit any table name, including those that contain backtick characters?

Permissive escaping—attempt 1

You might be tempted to modify your function to use the following:

$this->query('SELECT 1 FROM `'.str_replace('`', '``', $table).'` LIMIT 1');

DON'T! The above code is vulnerable (in obscure edge cases).

str_replace() is a naive, byte-wise function (it is not character-set aware, and therefore is not safe to use with string encodings that have multi-byte characters).

If your database connection uses a multibyte character set, such as GBK, then a malicious table name would not get escaped properly:

// malicious user-provided value
$_POST['tableName'] = "\x8c`; DROP TABLE users; -- ";

// call your function with that value
tableExists($_POST['tableName']);

The above would result in query() being called with the following string argument:

SELECT 1 FROM `宍`; DROP TABLE users; -- ` LIMIT 1

This is because when str_replace() goes through the inputted string byte-by-byte—naively replacing any occurrence of the '`' character, i.e. byte 0x60—it does so without understanding that MySQL will consider the string to be encoded using GBK, in which 0x8c60 is the single character '宍'; and therefore it turns such characters into 0x8c6060 which represent '宍`'. That is str_replace() has introduced a terminating backtick character that was not there before!

Permissive escaping—attempt 2

The problem can however be fixed by using a character-set aware replacement function. PHP doesn't have one by default, although some optional extensions like Multibyte String are fairly commonplace in typical hosting environments.

If following this approach, you must ensure that you perform the replacement using the character encoding of the database connection.

Permissive escaping—attempt 3

As of MySQL v5.7.6, you can use mysql_real_escape_string_quote() to properly escape SQL identifiers using the database connection's character set. Sadly however, the PDO API does not (yet?) provide an interface to this C function…

What about whitelisting?

Whitelisting is often considered to be more secure and reliable than escaping. However (unless one knows in advance that they do not contain special characters), one must still escape the whitelisted values—so this doesn't really help progress the most general cases, although it does limit the damage that can be done should the escaping prove erroneous.

So what's the conclusion?

It's actually quite difficult to use arbitrary, user-provided, SQL identifiers in a safe way.

Fortunately, however, it's not a common requirement. As a general rule, one's schema should both be static (absent any changes to one's codebase that necessitate schema modification) and compliant with the principle of orthogonal design: if both of those conditions are fulfilled, SQL identifiers will always be part of one's static code and there will not be any need to use user input in their place.

Answer 2

No. You have removed the only harmful character in the given context.

So it's safe.

As for reasonable, well... no. Table names are generally something that should never come from user input.