Re: [RFC] [VOTE] is_literal

On Fri, Jul 16, 2021 at 2:47 AM Craig Francis <craig@craigfrancis.co.uk> wrote: > Just another day, and another injection vulnerability (please patch): > > > https://woocommerce.com/posts/critical-vulnerability-detected-july-2021/ > > If only escaping wasn't being used, so user values did not get included in > certain strings :-) > > diff -r > woocommerce.5.5.0/includes/data-stores/class-wc-webhook-data-store.php > woocommerce.5.5.1/includes/data-stores/class-wc-webhook-data-store.php > 280c280 > < $search = ! empty( $args['search'] ) ? "AND > name LIKE '%" . > $wpdb->esc_like( sanitize_text_field( $args['search'] ) ) . "%'" : > ''; > --- > > $search = ! empty( $args['search'] ) ? $wpdb->prepare( "AND > name LIKE %s", '%' . $wpdb->esc_like( > sanitize_text_field( > $args['search'] ) ) . '%' ) : ''; > On Sat, Jul 17, 2021 at 3:45 AM 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). > I don't think so. Looking at https://developer.wordpress.org/reference/functions/sanitize_text_field/ and https://developer.wordpress.org/reference/classes/wpdb/esc_like/ you can see that they *don't* escape single quotes, so there was *indeed* an SQL injection vulnerability in that code. (Which is [one of the reasons] why I avoid WordPress [and especially third-party themes / plugins] as much as possible.) -- Guilliam Xavier