[wp-trac] [WordPress Trac] #41925: Bring back, support and document %1$s support in wpdb->prepare

[WordPress Trac]

#41925: Bring back, support and document %1$s support in wpdb->prepare
-------------------------+------------------------------
 Reporter:  soulseekah   |       Owner:
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  Awaiting Review
Component:  Database     |     Version:
 Severity:  normal       |  Resolution:
 Keywords:               |     Focuses:
-------------------------+------------------------------

Comment (by soulseekah):

 The related vulnerability that was based on the usage is disclosed here
 https://medium.com/websec/wordpress-sqli-bbb2afcc8e94

 It seems to be based on "%1$%s" being used, which isn't even a valid
 sprintf placeholder. "%1$s" is. But the issue is that numbered
 placeholders are not quoted. So indeed, the usage of unquote numbered
 parameters is unsafe in core.

 How do we go from this to safe patch? That satisfies both current usage in
 the wild and security? Well, we need to quote them and write dozens of
 unit tests for the prepare method to make sure it works.

 Right now wpdb quotes %s parameters before escaping them. I propose that
 we quote them after escaping them, unless the result is numeric, although
 there may be issues with locale-aware floats and is_numeric
 (http://php.net/manual/en/function.is-numeric.php) but even if they're
 quoted the SQL will still be valid.

 https://github.com/WordPress/WordPress/blob/master/wp-includes/wp-
 db.php#L1228

 Quoting the result of escape_by_ref if not is_numeric should work. Patch
 and tests in progress.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/41925#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform