Re: [RFC] [VOTE] is_literal

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 <craig@craigfrancis.co.uk> wrote: > On Fri, 16 Jul 2021 at 21:24, Hans Henrik Bergan <divinity76@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 > > > >