MySQL PDO query syntax error with ? parameters

Question

as a newbie, I've followed PHP MySQL tutorials advising the use of regular MySQL php functions. However, since I've been told that PDO is the better alternative, I've been converting my code to that. I just ran into the following problem:

    $query = $uspdb->prepare("SELECT post_id, is_approved, reports FROM ? WHERE id=? AND ?");
    $query->bindValue(1, $table, PDO::PARAM_INT);
    $query->bindValue(2, $id, PDO::PARAM_INT);
    $query->bindValue(3, checkPermission("comment_moderation"),PDO::PARAM_BOOL);
    $query->execute;
    $result = $query->fetch(PDO::FETCH_ASSOC);

The first line throws the following PDO exception:

SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '? WHERE id=? AND ?' at line 1

Why is that? I have no idea what could be wrong with the syntax. The tutorial I'm reading tells me that I should be using bindValue or execute(array(stuff)) to add parameters rather than ".$id." and the likes, since it's safer, but this isn't working for whatever reason.

You bind the table name as PDO::PARAM_INT, quite sure that's wrong. — Bart Friederichs
– Bart Friederichs, Commented Aug 5, 2013 at 7:13

Your Common Sense · Accepted Answer · 2013-08-05 07:16:18Z

1

Unfortunately, prepared statement can represent a data literal only. (in case of emulated prepares). So, a developer have to take care of identifiers oneself - PDO offers no help for this matter.

To make a dynamical identifier safe, one have to follow 2 strict rules:

To format identifier properly. Means
- enclose identifier in backticks.
- escape backticks inside by doubling them.
To verify it against a hardcoded whitelist.

After the formatting, it is safe to insert the $table variable into query. So, the code would be:

$field = "`".str_replace("`","``",$field)."`";
$sql   = "SELECT * FROM t ORDER BY $field";

However, although such a formatting would be enough for the cases like ORDER BY, for the most other cases there is a possibility for a different sort of injection: letting a user to choose a table or a field they can see, we may reveal some sensitive information, like password or other personal data. So, it's always better to check dynamical identifiers against a list of allowed values. Here is a brief example:

$allowed = array("name","price","qty");
$key = array_search($_GET['field'], $allowed));
if ($key === false) {
    throw new Exception('Wrong field name');
}
$field = $allowed[$key];
$query   = "SELECT $field FROM t"; //value is safe

answered Aug 5, 2013 at 7:16

Your Common Sense

158k42 gold badges226 silver badges373 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

ividyon Over a year ago

Do I still have to sanitize and whitelist input like this if it is not actually left to the user? Can attackers potentially insert whatever they want into such a variable, or only if I allow the user to modify it? The $table variable in my code is handled like this (the $type variable being a $_GET: switch($type) { case "review": $table = "site_cmt_reviews"; $posttable = "site_review"; break; default: $table = "site_cmt_articles"; $posttable = "site_article"; }

Your Common Sense Over a year ago

it doesn't matter. User input has nothing to do here. It's rules of creating SQL queries. Have always to be followed. Or, sooner or later, it will be injected.

Your Common Sense Over a year ago

Your switch is actually the very white list I am talking about. Just a longer code.

Your Common Sense Over a year ago

Formatting is still required though.

ividyon Over a year ago

Can this formatting be replaced by a simple call of mysql_real_escape_string($table)?

ividyon · Accepted Answer · 2013-08-05 07:30:44Z

0

As is typical, I solve my problem seconds after posing the question.

The problem is that you can only bind key values like this, not table or column names. I'll have to keep inserting the table and column names manually just as before:

$query = $uspdb->prepare("SELECT post_id, is_approved, reports FROM $table WHERE id=? AND ?");
$query->execute(array($id,checkPermission("comment_moderation")));
$result = $query->fetch(PDO::FETCH_ASSOC);

If the table or column name is left to the user's discretion, you should go through additional steps to sanitize it, which are detailed in Your Common Sense's response above. In my case it was the following code:

$type = $_GET[type];

switch($type) {
    case "review":
        $table = "site_cmt_reviews";
        break;
    default:
        $table = "site_cmt_articles";
}

Still, thanks for reading!

edited Aug 5, 2013 at 7:30

answered Aug 5, 2013 at 7:17

ividyon

6952 gold badges6 silver badges17 bronze badges

5 Comments

Your Common Sense Over a year ago

And so have an injection.

ividyon Over a year ago

Since, in my example, the $table variable is determined by code and cannot be changed/maliciously exploited by a user, that doesn't really run the risk of an injection, I think? Though I guess it should still be recommended to sanitize the input if it is.

Your Common Sense Over a year ago

Nobody knows what is determined in your code and how it can be changed in the future. Neither your question nor this answer ever mention it.

ividyon Over a year ago

Edited the response to make that clearer and to refer to your post.

Your Common Sense Over a year ago

This one is a way to go.

Collectives™ on Stack Overflow

MySQL PDO query syntax error with ? parameters

2 Answers 2

5 Comments

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related