WP_Tax_Query::transform_query() double escapes name term arguments

[bcworkz]

As ​reported in Support Forums by @expert1

When making a new WP_Query that includes a "tax_query" argument involving a term name that has characters requiring escaping, such as apostrophes, WP_Tax_Query::transform_query() double escapes the term name so that the subsequent query always fails. The resulting SQL string includes WHERE 0=1 due to the double escaping.

The term is first escaped by wp_filter_kses() which is hooked into the "edit_term_name" filter of sanitize_term_field() which is called on ​line 626 of wp-includes/class-wp-tax-query.php. On return the term is escaped again with esc_sql() on the same line.

I'm not sure if it makes more sense to remove the esc_sql() call or to hook in wp_kses() instead of wp_filter_kses() for the "edit_term_name" filter. If the latter, we need to run the term through stripslashes() before passing it to the filter callbacks. I'm unsure how that may affect other filter callbacks. (sanitize_text_field() and _wp_specialchars() by default)

[boonebgorges]

@bcworkz Thanks for the ticket and for the diagnosis.

The principle in previous cases has been to apply the same sanitization at the time of *query* as what's applied at the time of *insertion*. So what we're trying to match here is https://core.trac.wordpress.org/browser/tags/4.7/src/wp-includes/taxonomy.php?marks=1992,1995#L1989 - sanitize_term_field() + wp_unslash(). The case is very similar to #35493.

In an ideal world, the sanitization-juggling would be less insane, but I think that 39315.diff​ is our best alternative in the actual world. What do you think?

[bcworkz]

@boonebgorges - Thanks for working up a solution!

It seems a bit round-a-bout, but I understand the desire for consistency. Since it solves the problem, it works for me :)

[boonebgorges]

In 39662:

Don't double-escape terms payload in WP_Tax_Query::transform_query().

terms values are passed through sanitize_term_field() with the 'db'
flag, which add slashes. Because terms are subsequently run through
esc_sql(), these slashes must be removed. See [36348], which added
a similar step to sanitization in get_terms().

Props bcworkz.
Fixes #39315.

[boonebgorges]

#37276 was marked as a duplicate.