25
votes

I am currently building a query where both the field/column and value parts possibly consist of user inputted data.

The problem is escaping the fieldnames. I'm using prepared statements in order to properly escape and quote the values but when escaping the fieldnames i run into trouble.

  • mysql_real_escape_string requires a mysql connection resource in order to us so that is ruled out
  • PDO::quote adds quotes around the fieldnames which renders them useless in a query too
  • addslashes works but isn't really safe

Anyone has an idea on what the best way is to properly insert the fieldnames into the query before passing it to PDO::prepare?

5

5 Answers

27
votes

The ANSI standard way of doing a delimited identifier is:

SELECT "field1" ...

and if there's a " in the name, double it:

SELECT "some""thing" ...

Unfortunately this doesn't work in MySQL with the default settings, because MySQL prefers to think double quotes are an alternative to single quotes for string literals. In this case you have to use backticks (as outlined by Björn) and backslash-escaping.

To do backslash escaping correctly, you would need mysql_real_escape_string, because it's character-set-dependent. But the point is moot, because neither mysql_real_escape_string nor addslashes escape the backquote character. If you can be sure there will never be non-ASCII characters in the column names you can get away with just manually backslash-escaping the ` and \ characters.

Either way, this isn't compatible with other databases. You can tell MySQL to allow the ANSI syntax by setting the config option ANSI_QUOTES. Similarly, SQL Server also chokes on double quotes by default; it uses yet another syntax, namely square brackets. Again, you can configure it to support the ANSI syntax with the ‘quoted_identifier’ option.

Summary: if you only need MySQL compatibility:

a. use backquotes and disallow the backquote, backslash and nul character in names because escaping them is unreliable

If you need cross-DBMS compatibility, either:

b. use double quotes and require MySQL/SQL-Server users to change the configuration appropriately. Disallow double-quote characters in the name (as Oracle can't handle them even escaped). Or,

c. have a setting for MySQL vs SQL Server vs Others, and produce either the backquote, square bracket, or double-quote syntax depending on that. Disallow both double-quotes and backslash/backquote/nul.

This is something you'd hope the data access layer would have a function for, but PDO doesn't.

Summary of the summary: arbitrary column names are a problem, best avoided if you can help it.

Summary of the summary of the summary: gnnnnnnnnnnnh.

13
votes

The correct answer, is

str_replace("`", "``", $fieldname)

Wrong:

mysql> SELECT `col\"umn` FROM user;
ERROR 1054 (42S22): Unknown column 'col\"umn' in 'field list'

Right:

mysql> SELECT `kid``s` FROM user;
ERROR 1054 (42S22): Unknown column 'kid`s' in 'field list'
mysql> SELECT ```column``name``` FROM user;
ERROR 1054 (42S22): Unknown column '`column`name`' in 'field list'

(Note that in last example, the column name has 3 (three) extra back-ticks in it, just to show an extreme case)

2
votes

This may affect performance, but it should be secure.

First run a DESCRIBE table query to get a list of allowed field names, then match these agaisnt the user submitted data.

If there's a match then you can use the user-submitted data without the need for any escaping.

If there's not match then it's a typo or a hack - either way it's an 'error' in the inputted data and the query should not be run.

The same could be done for 'dynamic' table names by running a SHOW TABLES query and matching from that result set.

In one of my applications I have an 'install' script; part of this queries the database and table field names and then writes a php file that is always referred back to so I'm not constantly running DESCRIBE queries agains the database, eg

$db_allowed_names['tableName1']['id'] = 1;
$db_allowed_names['tableName1']['field1'] = 1;
$db_allowed_names['tableName1']['field2'] = 1;
$db_allowed_names['tableName2']['id'] = 1;
$db_allowed_names['tableName2']['field1'] = 1;
$db_allowed_names['tableName2']['field2'] = 1;
$db_allowed_names['tableName2']['field3'] = 1;

if($db_allowed_names['tableName1'][$_POST['field']]) {
     //ok
}

I use array keys like this as the if statement is a little faster than an in_array lookup

1
votes

How about something like this?

function filter_identifier($str, $extra='') {
    return preg_replace('/[^a-zA-Z0-9_'.$extra.']/', '', $str);
}

try {
    $res = $db->query('SELECT '.filter_identifier($_GET['column'], '\*').' FROM '.filter_identifier($_GET['table']).' WHERE id = ?', $id);
} catch (PDOException $e) {
    die('error querying database');
}

This is a simple white-list based character list. Any characters not in the list will get removed. Fortunately for me, I was able to make the database and tables, so I know there will never be any characters outside "a-zA-Z0-9_" (note: no space). You can add extra characters to the list via the $extra arg. If someone were to try and put "(SELECT * FROM users);--" in 'column', it would filter down to "SELECT*FROMusers", which would thrown an exception :)

I try to avoid doing extra queries if at all possible (I'm very performance sensitive). So things like doing a DESCRIBE beforehand or hard-coding an array of tables/columns to check against is something I'd rather not do.

0
votes

Wierd design of a project, but for your problem: Surround your field names with ` and use addslashes for the name as well.

select `field1`, `field2` from table where `field3`=:value