Re: [PHP-DEV] is_literal() + WordPress

  109216
March 22, 2020 23:14 craig@craigfrancis.co.uk (Craig Francis)
On Sun, 22 Mar 2020 at 19:11, Mike Schinkel <mike@newclarity.net> wrote:
> IMO getting that in WordPress core is highly unlikely
Good point, like all systems, WordPress will need to consider older versions of PHP. But, because this is a new function, they can avoid that issue by using `function_exists()`, as in... if (function_exists('is_literal') && !is_literal($sql)) { trigger_error('This is an unsafe $query, please use $wpdb->prepare()', E_USER_NOTICE); } This would be a pretty easy way for WordPress to show they take security seriously, and helping developers to avoid these "all too common" mistakes. But I do appreciate how much effort it can be to introduce anything in to WordPress :-) Craig
> On 22 Mar 2020, at 19:10, Mike Schinkel <mike@newclarity.net> wrote: > >> On Mar 21, 2020, at 7:15 PM, Craig Francis <craig@craigfrancis.co.uk> wrote: >> >> On Sat, 21 Mar 2020 at 22:53, Mike Schinkel <mike@newclarity.net> wrote: >> A large number of PHP users have no control over the platform they run on, so the option to use PECL modules is a non-starter for them. >> >> >> Thanks Mike, >> >> Personally I agree, I would say PECL modules are not preferable for "useful features"; simply because I try to keep my systems only using core PHP features where possible (makes server admin easier). > > +1 > >> As you mention working with WordPress, I've seen a couple of developers who have taken examples like: >> >> $posts = $wpdb->get_results("SELECT ... WHERE post_type='post'"); >> >> Then edited it to something dangerous like: >> >> $posts = $wpdb->get_results("SELECT ... WHERE post_type='" . $_GET['type'] . "'"); > > Yes. That is all too common. > >> To guard against this, do you think that WordPress could update their get_results() function to do something like: >> >> public function get_results( $query = null, $output = OBJECT ) { >> if (!is_literal($sql)) { >> trigger_error('This is an unsafe $query, please use $wpdb->prepare()', E_USER_NOTICE); >> } >> >> Perhaps with a better message; then, over the years, increase the warning level? >> >> I think that would be a very useful way of getting developers aware of these dangers. > > Well... > > IMO getting that in WordPress core is highly unlikely for two (2) reasons: > > ------------ > 1. Getting the WordPress team to adopt new features is about as difficult as getting the PHP community to agree on and add a new feature. IOW, it is a huge uphill battle. > > I have many, many unfulfilled feature requests in WordPress' trac that are up to 8 years old or more even though on many of the tickets there appears to be universal interest. What I have found is that the WordPress team does not address anything unless they need it to implement the features that Matt has dictated should be in the next version of WordPress. Because of that, many very useful ticketsl sit there for years and years and do not get addressed. > > Of course if a core team member with commit access has a pet feature they want to add, it typically gets slipped it with no fanfare and then we are stuck with it (*cough* Customizer *cough*) > > I did recently get one ticket addressed about 11 months ago that was a subset of a ticket that is 9 years old (and sadly the OP on the older ticket has since died of cancer.) > > But I think I invested probably 80 hours over that duration constantly bringing it up on Slack publicly and via DMs and on the ticket. My guess is they were just tired of hearing from me. And ALL my ticket was asking for was a damn hook that many plugins had already added because WordPress core had not: > > https://core.trac.wordpress.org/ticket/47056 > > ------------ > 2. More importantly — and a much higher bar — WordPress core supports PHP 5.6 and so core cannot contain any features that are in a later version of PHP. > > https://wptavern.com/wordpress-ends-support-for-php-5-2-5-5-bumps-minimum-required-php-version-to-5-6 > > And don't expect that to change any time soon. WordPress' minimum PHP requirements was notoriously 5.2.4 until March 2019 so it is unlikely they will bump the PHP requirement before March 2029. > > (just kidding, but only a bit.) > > Of course WordPress currently *recommends* PHP 7.3 and most (all?) managed hosts support that so userland developers are free to use newer version of WordPress on our own projects and in our plugins and themes. > > https://wordpress.org/about/requirements/ > > But again, WordPress core cannot use any features anytime soon that are not in PHP 5.6.
  109219
March 23, 2020 00:08 mike@newclarity.net (Mike Schinkel)
> On Mar 22, 2020, at 7:14 PM, Craig Francis <craig@craigfrancis.co.uk> wrote: > > On Sun, 22 Mar 2020 at 19:11, Mike Schinkel <mike@newclarity.net> wrote: >> IMO getting that in WordPress core is highly unlikely > > Good point, like all systems, WordPress will need to consider older versions of PHP. > > But, because this is a new function, they can avoid that issue by using `function_exists()`, as in... > > if (function_exists('is_literal') && !is_literal($sql)) { > trigger_error('This is an unsafe $query, please use $wpdb->prepare()', E_USER_NOTICE); > }
True....
> This would be a pretty easy way for WordPress to show they take security seriously, and helping developers to avoid these "all too common" mistakes. > > But I do appreciate how much effort it can be to introduce anything in to WordPress :-)
....but I will let you be the one to champion that cause given how much effort not being a core developer and getting anything added to WordPress it is. :-) -Mike