[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        |   Keywords:  dev-feedback
  Focuses:                |
--------------------------+-----------------------------
 WordPress does not currently provide an explicit method for escaping SQL
 table names. This leads to potential security vulnerabilities, and makes
 reviewing code for security unnecessarily difficult.

 Core tables are of course implicitly escaped when referenced in queries
 like `SELECT * FROM $wpdb->posts`. That’s fine.

 When plugins create or reference custom table names, there are no API
 methods and no guidance as to how they should safely escape those names.
 Since `$wpdb->prepare()` surrounds `%s` references with `'` quotes in a
 way that is incompatible with MySQL table name syntax, it becomes
 necessary to use bare PHP variables in the first parameter of `prepare()`,
 which is inherently risky. Plugins use a variety of ways of mitigating
 that risk, including none at all. These are examples paraphrased from real
 code in popular plugins:

 {{{
 $wpdb->query( $wpdb->prepare( "SELECT * FROM my_table_name WHERE …" ) );
 // Literal string

 $wpdb->query( $wpdb->prepare( "SELECT * FROM $my_table_name WHERE …" ) );
 // Variable, which may or may not be assigned a literal or escaped in some
 way

 $wpdb->query( $wpdb->prepare( "SELECT * FROM $this->table_name WHERE …" )
 ); // Object property

 $wpdb->query( $wpdb->prepare( "SELECT * FROM {$wpdb->prefix}my_table_name
 WHERE …" ) ); // Variable prefix with literal string

 $wpdb->query( $wpdb->prepare( "SELECT * FROM {$this->get_table_name()}
 WHERE …" ) ); // Interpolated method call

 $wpdb->query( $wpdb->prepare( "SELECT * FROM " . sanitize_key( $table_name
 ) . " WHERE …" ) ); // Attempted escaping

 $wpdb->query( $wpdb->prepare( "SELECT * FROM " . preg_replace( '/\W/', '',
 $table_name ) . " WHERE …" ) ); // Hand-rolled escaping

 $wpdb->query( $wpdb->prepare( "SELECT * FROM " . self::TABLE_NAME . "
 WHERE …" ) ); // Class constant
 }}}

 Some variations use backticks around table names. And of course there are
 many other examples using similar queries without `prepare()` statements.
 Note that no guidance is given to plugin developers, so every one has had
 to separately decide how to handle this.

 Other than the literal string examples, none of these examples are
 verifiably secure. They might be fine, but it is impossible to be sure
 without reviewing the code path leading up to that point. Even in the case
 of a constant, it is possible that the constant might in some
 circumstances be initialized with data from an insecure source. In cases
 that make use of OO, it is possible that the table name is initialized in
 a parent class in a different file or module. It might also be overridden
 in child classes.

 All of this makes it difficult for a human code reviewer to be certain
 that a given query is secure, even though it uses `prepare()` statements.
 Static code analysis is similarly difficult. Developers who make use of
 wpcs and similar tools inevitably need to exclude their queries from
 sniffer rules because they will otherwise cause false positive errors.

 The solution would be for WordPress to provide an explicit method for
 safely and defensively escaping table names, as close as possible to the
 query itself.

 I can think of several options:

 An `esc_sql_table_name()` function, to be used like this:

 {{{
 $wpdb->query( $wpdb->prepare( "SELECT * FROM " . esc_sql_table_name(
 $my_table_name ) . " WHERE …" ) );
 }}}

 A special formatting character supported by `prepare()`, something like:

 {{{
 $wpdb->query( $wpdb->prepare( "SELECT * FROM %z WHERE …", $my_table_name )
 );
 }}}

 A modification to the `%s` character format that does not add `'` quotes
 when surrounded with backticks:

 {{{
 $wpdb->query( $wpdb->prepare( "SELECT * FROM `%s` WHERE …", $my_table_name
 ) );
 }}}

 A way to safely add sanitized table names to the `$wpdb` object:
 {{{
 $wpdb->add_table_name( 'my_table_name' );
 $wpdb->query( $wpdb->prepare( "SELECT * FROM $wpdb->my_table_name WHERE …"
 ) );
 }}}

 This last option does look attractive, but unfortunately doesn’t help with
 the use case where the table name is necessarily variable such as in a
 loop, which is somewhat common in things like backup plugins:

 {{{
 foreach ( get_list_of_table_names() as $table_name ) {
         $wpdb->query( $wpdb->prepare( "SELECT COUNT(*) FROM $table_name
 WHERE …" ) );
 }
 }}}

 I would therefore tend to favour both adding an `esc_sql_table_name()`
 function, and also supporting one of the special `prepare()` formatting
 options (which would of course make use of `esc_sql_table_name()` behind
 the scenes).

 Note that there is some obvious overlap with the very similar problem of
 escaping ''column'' names. I think that warrants a separate ticket, but
 the solution is probably similar as for table names.

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