[wp-trac] [WordPress Trac] #11608: wpdb->prepare() is broken

WordPress Trac wp-trac at lists.automattic.com
Fri Dec 25 16:50:49 UTC 2009


#11608: wpdb->prepare() is broken
--------------------------+-------------------------------------------------
 Reporter:  hakre         |       Owner:  ryan                  
     Type:  defect (bug)  |      Status:  new                   
 Priority:  normal        |   Milestone:  3.0                   
Component:  Database      |     Version:  2.9                   
 Severity:  normal        |    Keywords:  has-patch dev-feedback
--------------------------+-------------------------------------------------

Comment(by hakre):

 Replying to [comment:25 miqrogroove]:
 >  The '%%' syntax is not explicitly defined in the PHP function
 description.  It only shows up 3 pages below the fold in the fifth example
 where it says ...
 You're wrong, it's documented clearly upfront in the description of the
 format parameter (as the first possible type even):

     6. A type specifier that says ...
     * % - a literal percent character. No argument is required.

 ----

 Replying to [comment:27 Denis-de-Bernardy]:
 > also, I believe the patch leads to double quoting of quoted %s. lines
 563 and 564 should be left untouched.
 >
 > {{{
 > $wpdb->prepare("'%s'"); // ''%s''
 > }}}
 >
 %s will get quoted by design (reference to vsprintf), quoted %s will get
 double quoted. so quoted %s is a misconcept. by definition %s are getting
 quoted and must not be quoted by the user. I removed the fuzz out of the
 prepare function. That was by intention to have that out. Do not train
 users writing bad queries for that function.

 ----

 Replying to [comment:30 sirzooro]:
 > We can consider adding new function prepare2() which will use PDO-like
 syntax, and mark prepare() as deprecated. [...]
 I think it's a good approach to shift away from the broken design.

 ----

 Regex suggestion by DD32 fails on strings like:

 {{{ %%%s }}}

 which should expand to

 {{{ %%'string value' }}}

 but which aren't any longer. as I assumed, the regex does not help here.
 For those arguing that is not a valid SQL clause or not or valid sprtinf
 format string: This is about USER-INPUT we handle here and this must be
 handeled strict and safe. We should not be talking about workarounds
 (only) when it comes to safety and security but how to solve things from
 the ground up.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/11608#comment:33>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software


More information about the wp-trac mailing list