> On Mar 21, 2020, at 7:15 PM, Craig Francis <cr...@craigfrancis.co.uk> wrote:
>
> On Sat, 21 Mar 2020 at 22:53, Mike Schinkel <m...@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.
------------
BTW, I did not comment on your is_literal() proposal because I try not to
comment on things where others are addressing concerns and where I do not feel
very strongly about adding or avoiding the feature. I created this thread as a
new thread to expliclty separate discussions because they are orthogonal and I
did not want to imply that I supported is_literal() as currently proposed.
But since you brought it up let me comment on is_literal(). I recognize the
problem I think you are trying to solve — to be able to guard against SQL
injection attacks — but I don't think is_literal() is a viable solution for
that problem.
1. Looking at my code most of my dynamic values use to compose SQL that I pass
to $wpdb->get_results() is in variables, not literals. In fact I rarely use
literals. So I don't see that it would help me (or many others?) much at all.
2. Focusing on it being literal or not seems to me to be focusing on wrong
thing. Something could be non-literal but still be safe. And a code hack could
introduce a tainted literal (but with a code hack all bets are off.) is_taint()
makes more sense to me, but even then I am not sure it directly addresses the
use-case.
3. I feel like we might be better to introduce functionality that addresses the
exact problem of SQL injection and not one that dances around its edges. Maybe
we need a MySQL class as an alternative to the mysqli functions that has a
"safe" property and when that "safe" property is true you can't run DDL,
TRUNCATE, or DELETE? Not sure how to protect against injection for UPDATE but
maybe someone else has an idea?
4. Lastly, I think it would be better to specify the problem(s) you are trying
to solve and then hash out potential solutions on the list rather than propose
a specific one in advance, maybe even creating an ad-hoc working group to come
up with a solution that is likely to be accepted?
Anyway, #jmtcw on is_literal().
-Mike
>
> Craig
>
>
>
> On Sat, 21 Mar 2020 at 22:53, Mike Schinkel <m...@newclarity.net> wrote:
> > On Mar 21, 2020, at 5:59 PM, tyson andre <tysonandre...@hotmail.com> wrote:
> > FROM: Re: [PHP-DEV] [RFC] is_literal()
> >
> > And if it can be implemented as a PECL module, that would be more
> > preferable to me than a core module of php.
> > If it was in core, having to support that feature may limit optimizations
> > or implementation changes that could be done in the future.
>
> Just wanted to address this comment which was made on another thread (I did
> not want to hijack that thread.)
>
> 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.
>
> Here are several of those managed hosting platforms I speak of. Collectively
> they host a large number of WordPress sites, and Pantheon also host Drupal
> sites:
>
> https://pagely.com/
> https://wpvip.com/
> https://wpengine.com/
> https://kinsta.com/
> https://pantheon.io/
>
> Given that, if there is an option between a useful feature being added to
> core or left in PECL, I would vote 100% of the time for core, since working
> with WordPress on a corporate site I can rarely ever use PECL extensions.
>
> #fwiw
>
> -Mike
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
