Opened 2 years ago
Last modified 22 months ago
#57149 new defect (bug)
get_table_from_query() doesn't properly handle a prepared escape_like() table name
Reported by: | prettyboymp | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Database | Keywords: | |
Focuses: | multisite | Cc: |
Description
A common way to check if a table exists is to run a query like the following, the same way that maybe_create_table()
currently works:
<?php $wpdb->prepare( 'SHOW TABLES LIKE %s', $wpdb->esc_like( $table_name ) );
This results in the prepared query for a table name such as wp_my_custom_table
to end up being:
SHOW TABLES LIKE 'wp\\_my\\_custom\\_table'
While the above isn't necessarily a proper query, it is still valid.
The problem is that $wpdb->get_table_from_query()
ends up returning the table name with extra slashes, e.g. wp\_my\_custom\_table
. This specifically breaks HyperDB which is attempting to use that table name to determine the dataset to use.
Attachments (2)
Change History (5)
#1
follow-up:
↓ 3
@
2 years ago
Hey @prettyboymp 👋 nice find 🔍 nice patch 🙌
I am able to reproduce this defect in:
- Vanilla WordPress wpdb
maybe_create_table()
(not the one ininstall-helper.php
)network_domain_check()
display_setup_form()
wp-admin/maint/repair.php
- HyperDB
- LudicrousDB
- BerlinDB
Your suggested code change does successfully resolve the problem as you've reported it 💎
Additional research & testing has revealed some potential tangential defects 😬
\
is a legal character in MySQL table names:CREATE TABLE `wp\_users` LIKE `wp_users`; CREATE TABLE `wp\\_users` LIKE `wp\_users`; CREATE TABLE `wp\\\_users` LIKE `wp\\_users`; CREATE TABLE `wp\\\\_users` LIKE `wp\\\_users`; CREATE TABLE `wp\_\_\_users` LIKE `wp\\\\_users`; CREATE TABLE `wp\_\_\_\_users` LIKE `wp\\\\_users`; SHOW TABLES LIKE 'wp%users';
- WordPress does not list
NO_BACKSLASH_ESCAPES
as an incompatible mode inwpdb
– #3286 - WordPress does not use the
ESCAPE
clause to enforce\
as the escape character - Documentation for wpdb::prepare() includes the following:
Literal percentage signs (`%`) in the query string must be written as `%%`. Percentage wildcards (for example, to use in LIKE syntax) must be passed via a substitution argument containing the complete LIKE string, these cannot be inserted directly in the query string. Also see wpdb::esc_like().
Interpretation:- Emphasis: Percentage wildcards (to use in LIKE syntax) must be passed via a substitution argument containing the complete LIKE string, these cannot be inserted directly in the query string.
- Missing: The docs do not instruct to use a similar approach for Underscore wildcards, or really what approach to use for them at all.
Conclusions:
- Table names & Columns in
SHOW ... LIKE %s
queries...- most likely should not use
$wpdb->esc_like()
- most likely should not use
$wpdb->prepare()
- ...because it is not absolutely certain that "
_
" is intended to be wild or literal
- most likely should not use
- Otherwise, they are mangled with unnecessary slashes...
- ...causing subsequent
$wpdb->get_var()
queries to fail... - ...because a table name like
wp_users
gets:- esc_like'ed to
wp\_users
- prepare'ed to
wp\\_users
- esc_like'ed to
- even though:
wp_users
is adequately "wild"wp\_users
is not syntactically what was being queriedwp\\_users
is not syntactically what was being queried
- and
_
does not need escaping the way%
does- I.E.
\_\_\_
needs to meanwp\_\_\_users
and cannot meanwp___users
- I.E.
- ...causing subsequent
- Instead, most likely, these queries should be concatenated, unescaped and unprepared, and manually slashed to accommodate the desired matching. In this way, the core
str_replace( '\\_', '_', $maybe[2] )
is surprisingly accurate:$like = "'wp\\\_\\\_\\\_\\\_users'"; $sql = 'SHOW TABLES LIKE ' . $like; $query = $wpdb->get_var( $sql );
...or...$like = $wpdb->esc_like( 'wp\_\_\_\_users' ); $sql = "SHOW TABLES LIKE '{$like}'"; $query = $wpdb->get_var( $sql );
- Perhaps, we are all doing it wrong everywhere, and a deeper conclusion is required?
- Additionally plausible is that I've missed a mundane detail and all of this is wrong 🌚
- For all of the obvious reasons, I do not think it is a great idea to remove escaping or preparation without sufficient unit tests.
- Maybe WordPress officially just-says-no to database tables with slashes in them? Or, some other decision?
#3
in reply to:
↑ 1
@
22 months ago
Replying to johnjamesjacoby:
Conclusions:
- Table names & Columns in
SHOW ... LIKE %s
queries...
- most likely should not use
$wpdb->esc_like()
- most likely should not use
$wpdb->prepare()
- ...because it is not absolutely certain that "
_
" is intended to be wild or literal
Based on the documentation for $wpdb->esc_like()
, we should be able to assume that "_
" is intended to be literal. Any wildcards should be added to the like string after $wpdb->esc_like()
is applied to it.
/**
...
* Example Prepared Statement:
*
* $wild = '%';
* $find = 'only 43% of planets';
* $like = $wild . $wpdb->esc_like( $find ) . $wild;
* $sql = $wpdb->prepare( "SELECT * FROM $wpdb->posts WHERE post_content LIKE '%s'", $like );
...
*/
Updating to code standards