Take the 2-minute tour ×
WordPress Development Stack Exchange is a question and answer site for WordPress developers and administrators. It's 100% free, no registration required.

I've just updated to WP 3.9 and it returns a warning when using placeholders incorrectly. I've found some prepared statements written incorrectly, but I really got worried when I saw this in the ACF-plugin (because it's so widely used): (Take a look at the export.php in the core files of the plugin)

// create SQL with %d placeholders
$where = 'WHERE ID IN (' . substr(str_repeat('%d,', count($my_options['acf_posts'])), 0, -1) . ')';

// now prepare the SQL based on the %d + $_POST data
$posts = $wpdb->get_results( $wpdb->prepare("SELECT * FROM {$wpdb->posts} $where", $my_options['acf_posts']));

This seems to be a vulneribility in the plugin? (because the abscence of actual placeholders in the actual prepare-statement (they're using $where instead ) or am I totally incorrect in my assumtpion? Please guide me :-)

share|improve this question
2  
I would definitely raise this with the plugin author, as this is more a question related to the plugin –  Pieter Goosen May 11 at 6:34

1 Answer 1

up vote 3 down vote accepted
+50

I'm pretty sure that you ARE incorrect.

Let's taka a closer look at this code you mentioned in your question.

In first line it sets $where variable:

$where = 'WHERE ID IN (' . substr(str_repeat('%d,', count($my_options['acf_posts'])), 0, -1) . ')';

So what this line really does is:

  • get the number of some options (let's call it 'N')
  • repeats %d placeholder N times
  • append it to string WHERE ID IN ('.

So $where variable contains a string like this:

WHERE ID IN (%d, %d, %d, ...)

... and as you can see, this string CONTAINS placeholders.

Just after this line comes real query, which is build using this $where variable, and the option values ARE inserted using these placeholders.

There isn't anything bad in using variables, when you build query string, if these variables contains safe strings (i. e. don't contain values comming from user).


PS. Take a look at this code:

$sql = "SELECT * FROM {$wpdb->posts} WHERE ID=%d";
$results = $wpdb->get_results( $wpdb->prepare( $sql, 10 ) );

Do you think this is also a vulneribility?

share|improve this answer
    
Ok. Thank you!!! I really didn't thought it was bad code in the plugin (else someone else had probably seen that a long, long time ago) I was just curious. And of course, you're totally right! (I was "stuck" thinking it's not ok to have variables in the prepare-function) –  bestprogrammerintheworld May 11 at 11:51
2  
No problem. Sometimes it's good to doubt :) –  Krzysiek Dróżdż May 11 at 16:55
    
Please remind me to award your bounty. I can't do it beore 10 hours has gone... –  bestprogrammerintheworld May 11 at 19:40
    
You should be able to do it now. –  Krzysiek Dróżdż May 12 at 6:04

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.