Let's allow eval() to be turned off in PHP 8

Looks like PHPUnit only uses eval() for mock objects, and Twig only uses it
as a last line of defense for building templates. Still breakages, but not
of the entire packages (at least those packages) from what I can see.

That said, I agree that eval() should stay enabled by default, as too much
breaks if we did the opposite. That way, folks can opt into a hardened
environment (at least in this respect) once they've determined that doing
so won't break their software.

So long as the default behavior is to leave it available, I'm okay with
this. Any app
that relies on twig/twig, phpunit/phpunit, many symfony packages,
dompdf/dompdf,
etc relies on being able to use eval().

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’re not using it, or
• you’re using it properly

I’d 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 « hackers » 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 à 16:52, Ian Littman iansltx@gmail.com a écrit :

Hi 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 for
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 code
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 four
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 use
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 explain
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 may
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 use
some help here putting it together.

What do y'all think about getting this into PHP 8?

Thanks in advance,

Ian Littman

Main holes that this closes are:

1. Being able to run code fetched remotely without ever hitting disk
2. Being able to run code transformed directly from an obfuscated state

I have examples of both techniques; reply directly to me if you're curious.
One of the examples breaks if create_function isn't available. The other
two break if eval() isn't available.

One interesting thing with item #1 is that it allows for remote arbitrary
code execution even if no include-able path on a server is writable. This
comes into play if there's a supply chain attack on your app. Say, an
infected update on a CMS plugin. Get an eval() of a file_get_contents() of
a URL into the code and...well, you get code execution that (if you're
lucky) only leaves a trace in logs. If you have to write a file somewhere
first, then include it, you've got a bit more of a footprint.

Can you work around these restrictions? Yep, but it takes a bit more effort
than the current setup. It doesn't make a server secure by any stretch, but
it reduces its attack surface slightly, and reduces the universe of
malicious code that won't error out, forcing malefactors to work just a bit
harder.

To be crystal clear, I am not asking for disabling eval() by default.
Just for it to be disable-able, as create_function already is. This isn't a
resolution for all things security-related. Just one more thing to slow
attackers down.

Heck, if we can't agree on having the ability to disable eval() (which,
again, Suhosin has), how about catching cases where someone throws eval
into disabled_functions and warning them that the difference between a
function and a language construct renders their INI declaration useless?

Ian

On Tue, Nov 26, 2019 at 4:21 PM Stanislav Malyshev smalyshev@gmail.com
wrote:

Hi!

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.

I get defense in depth, but I don't understand what it means in this
case. Since you're talking about disabling functions, I assume we're
talking about the situation where there's code execution access. From
that point, you can execute any code. What is the value of disabling
eval() here? You don't need eval, you can run any code you want
directly! Am I missing something here?

--
Stas Malyshev
smalyshev@gmail.com