Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:107864
MIME-Version: 1.0
References: <CAAupm8h5Aa4EmnGnHX1iVgcQTCpmW-3yD4e6m7aouZFiSCtGrA@mail.gmail.com>
 <A7F945D0-C227-462C-9505-159EBA78B979@gmail.com>
In-Reply-To: <A7F945D0-C227-462C-9505-159EBA78B979@gmail.com>
Date: Tue, 26 Nov 2019 10:27:09 -0600
Message-ID: <CAAupm8j7TZHq7eD2UdbvgEGSsZ46M3gmLakMPZ7W-3qnq5=KBA@mail.gmail.com>
To: Benjamin Morel <benjamin.morel@gmail.com>
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary="000000000000b99e130598425a1e"
Subject: Re: [PHP-DEV] Let's allow eval() to be turned off in PHP 8
From: iansltx@gmail.com (Ian Littman)

--000000000000b99e130598425a1e
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

You're right that turning off eval() isn't a silver bullet, and if you can
get external code running on someone's box there are a lot worse things you
can do.

That said, if eval() and create_function() fail, the exploit samples that
I've recently come in contact with stop working, and it becomes more
difficult to obfuscate said code when you can't string-concatenate your way
into a script.

Yes, I could rewrite those kits to do similar stuff, provided I have the
ability to write to somewhere on disk that can then be executed as PHP. But
injecting a BC break like this for malware, means a temporary respite while
scripts get rewritten (*if* they get rewritten; I was seeing mysql_* calls
in these samples) at the very least, and probably less-obfuscated malware
to boot.

Again, I'm asking about an opt-in option here, just like disabled_functions
(and, again, this isn't a new concept, see Suhosin). Responsibility for the
tradeoff between flexibility/functionality and security still lies with
whoever's building the environment to run the code. This is just giving
another tool in the toolbox, on top of things like ensuring all executable
paths are read-only at runtime.

Ian

On Tue, Nov 26, 2019 at 10:11 AM Benjamin Morel <benjamin.morel@gmail.com>
wrote:

> Hi Ian,
>
> IMO, eval() is secure, as long as:
>
> - you=E2=80=99re not using it, or
> - you=E2=80=99re using it properly
>
> I=E2=80=99d say that as soon as your server has been compromised, eval() =
is the
> last of your worries, as pretty much anything becomes possible, including
> writing PHP code to a file and including/executing it. So I feel like
> disabling eval() will just make =C2=AB hackers =C2=BB have a good laugh.
>
> Regarding (arguably) legitimate use cases that would suffer from eval()
> being disabled, is a Schema.org parser library I wrote, that dynamically
> creates objects that implement arbitrary interfaces:
>
> https://github.com/brick/schema/blob/master/src/ObjectFactory.php
>
> IMHO, disabling eval() would not increase the security of PHP, and could
> be annoying for libraries relying (sparingly) on it, for lack of anything
> better.
>
> Best,
> Benjamin
>
> Le 26 nov. 2019 =C3=A0 16:52, Ian Littman <iansltx@gmail.com> a =C3=A9cri=
t :
>
> =EF=BB=BFHi all,
>
> Let's just say that eval() and create_function() are the cornerstone of
> PHP-based exploit toolkits. Yes, if the hackers get in there are other
> problems with your codebase, but as a defense in depth measure most
> applications need neither create_function() nor the eval() language
> construct, so they might as well be disabled.
>
> create_function() is easy enough to drop with a didabled_functions ini
> directive, and is going away "no later than PHP 8.0", per its deprecation
> notice as of 7.2. eval() on the other hand can't be disabled that way, as
> it's not actually a function. So this seems like an excellent candidate f=
or
> another ini setting, as from a security standpoint you *want* this change
> to be global. Yes, if every shared host turned this on by default, old co=
de
> would break. But I Suhosin allows doing this anyway (
> https://stackoverflow.com/questions/25037015/suhosin-and-disable-eval) so
> it's not like the option hasn't been available...though it's been over fo=
ur
> years since we've had a stable release of Suhosin.
>
> Similar to disable_functions, if the ini setting turning off eval() got
> set, you shouldn't be able to override it via ini_set() in code. We can u=
se
> a similar warning to the display_disabled_function one here.
>
> One alternative to adding an entirely new INI setting would be to allow
> disabled_functions to work on eval. That means that somewhere in the INI
> parsing/stubbing/warning process (and maybe all three places) will get a
> bit more complex, but that would have the benefit of not having to explai=
n
> to anyone editing the ini file that eval() is a language construct rather
> than a function and thus can't be disabled the normal way (I was just
> apprised of this mistake last today).
>
> From taking a quick look at Suhosin code, the way they're handling this m=
ay
> be somewhat informative for creating a patch directly to core, but as a
> bolt-on it looks like they can't be as efficient, so any patch to core
> would be inspired by, rather than a copy of, how that extension's
> eval-handling is built.
>
> I feel strongly enough about this to help with the text side of the RFC,
> and maybe even dive into php-src to assist with a patch, though I have
> neither karma for posting the former nor enough C acumen to do the latter
> all by myself. But I want to make sure I won't get immediately shot down =
if
> I try going down that road...and if other folks like the idea, I could us=
e
> some help here putting it together.
>
> What do y'all think about getting this into PHP 8?
>
> Thanks in advance,
>
> Ian Littman
>
>

--000000000000b99e130598425a1e--