oh thanks, now the vulnerability is clear. (i would still complain on that
as a pull request though, using double quotes for strings is just a
horrible idea, it's not compliant with ISO sql, and it depends on the MySQL
server it's running on *not* having @@SQL_MODE=ANSI_QUOTES enabled which
changes double-quotes to be ISO-compliant instead of mysql-borky (double
quotes for strings only work on MySQL because of a MySQL quirk, ISO SQL say
double quotes is meant for identifiers just like ` ), and it's not portable
across different SQL servers, and all of these problems go away if you just
use the sensible alternative: single quotes - seriously someone should
complain to whoever wrote that, so at least he doesn't do the same in the
future
i can tell from only that diff that, at least as of 5.5.1, woocommerce is
not compatible with @@SQL_MODE=ANSI_QUOTES :p
)
On Sat, 17 Jul 2021 at 03:45, Craig Francis <cr...@craigfrancis.co.uk>
wrote:
> On Fri, 16 Jul 2021 at 21:24, Hans Henrik Bergan <divinit...@gmail.com>
> wrote:
>
>> short of a bug in esc_like(), i don't even see the vulnerability issue in
>> that code?
>>
>
>
> Sorry Hans, I copied the wrong diff.
>
> There were only 2 changes from woocommerce 5.5.0 to 5.5.1.
>
> Like you I was wondering what that diff was doing before posting - I'm
> fairly sure it's just to be consistent with the other lines (which all use
> $wpdb->prepare).
>
> The diff I should have copied is:
>
> diff -r
> woocommerce.5.5.0/packages/woocommerce-blocks/src/StoreApi/Utilities/ProductQueryFilters.php
> woocommerce.5.5.1/packages/woocommerce-blocks/src/StoreApi/Utilities/ProductQueryFilters.php
> 86c86,92
> < $attributes_to_count = array_map( 'wc_sanitize_taxonomy_name',
> $attributes );
> ---
> > $attributes_to_count = array_map(
> > function( $attribute ) {
> > $attribute = wc_sanitize_taxonomy_name( $attribute );
> > return esc_sql( $attribute );
> > },
> > $attributes
> > );
>
> In context `$attributes_to_count` simply goes to:
>
> $attributes_to_count_sql = 'AND term_taxonomy.taxonomy IN ("' . implode(
> '","', $attributes_to_count ) . '")';
>
> Where the the esc_sql() is basically a call
> to mysqli_real_escape_string(), which explains why it needs risky quotes
> in/around implode.
>
> Craig
>
>
>
>
