[wp-trac] [WordPress Trac] #52506: Add escaping method for table names in SQL queries

[WordPress Trac]

#52506: Add escaping method for table names in SQL queries
-------------------------------------------------+-------------------------
 Reporter:  tellyworth                           |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  new
 Priority:  normal                               |   Milestone:  Awaiting
                                                 |  Review
Component:  Database                             |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  dev-feedback has-patch has-unit-     |     Focuses:
  tests                                          |
-------------------------------------------------+-------------------------

Comment (by craigfrancis):

 As requested by Tonya (@hellofromtonya)...

 I've split [https://github.com/WordPress/wordpress-develop/pull/2127 my
 patch] so we can discuss Identifiers (table/field names) separately to the
 `IN()` operator.

 There should be no conflicts with printf(), as
 [https://en.wikipedia.org/wiki/Printf_format_string#Type_field printf()
 Types] have historically used both `%d` and `%i` for signed integers (it's
 `scanf()` that treats them differently), whereas
 [https://www.php.net/manual/en/function.printf.php PHP printf] only uses
 `%d`, allowing us to use `%i` for Identifiers.

 I've done my own performance tests, and generally find it about the same
 (nearly always a bit faster), this is due to one less RegEx, and with
 `%d`/`%f` using intval()/floatval() rather than escaping. But I'll see if
 I can get someone else (independent) to confirm.

 Security testing, I come from a security background, so I won't lie and
 simply say this is perfectly secure, because you cannot prove security
 (you can only prove insecurity), and because of the "%s / %5s" oddity (for
 legacy reasons the second value is not quoted); what I can say is that all
 user values will continue to be passed though
 intval/floatval/_real_escape, or it's this new Identifier (e.g.
 table/field names) where values are always backtick quoted and only allow
 "a-z", "A-Z", "0-9", "_" and "-" characters (this could be relaxed, but I
 thought I should start with the most restrictive set of characters that I
 am completely sure are safe).

 The addition of `$wbdp->version = 2` is not needed, but it might help
 address a concern raised by @johnbillion, so plugins can easily "determine
 whether it's able to use the new formats" (i.e. someone could have a
 `content/db.php` that provides their own `prepare()` method).

 I've added some basic unit tests (suggestions welcome for new ones); they
 check it accepts table/field names normally, invalid characters are
 removed, and the value is always/consistently quoted (this won't have that
 legacy quoting issue, and we shouldn't be doing weird things like
 removing/replacing quotes from the format string).

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