Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » Let's allow eval() to be turned off in PHP 8 »

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

php.internals

Ian Littman

6 years ago
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

Ken Stanley

6 years ago
On Tue, Nov 26, 2019 at 10:53 AM Ian Littman <iansltx@gmail.com> wrote:
> What do y'all think about getting this into PHP 8? >
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(). On Tue, Nov 26, 2019 at 10:53 AM Ian Littman <iansltx@gmail.com> wrote:

Ian Littman

6 years ago
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. On Tue, Nov 26, 2019 at 10:01 AM Ken Stanley <dohpaz@gmail.com> wrote:

Benjamin Morel

6 years ago
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

Ian Littman

6 years ago
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:

Guilliam Xavier

6 years ago
For the record, a few months ago, https://github.com/php/php-src/pull/4084 (extending `disable_functions` to handle `eval`) was first merged but finally reverted (requested by Xdebug), and the feature request https://bugs.php.net/bug.php?id=62397 was closed (with an explanation).
-- Guilliam Xavier

Ian Littman

6 years ago
Thanks for the reference. For convenience, here's the PR that contains a bit more context: https://github.com/php/php-src/pull/4084 Definitely don't want to screw up Xdebug, so this would require a more nuanced approach (see also: why I don't want to just try to create a patch). Again, this doesn't solve attack vectors where attackers can write to the FS and then include from it. But it does close one-step "read from this URL, base64-decode, and eval the result" approaches. One less tool in the hacker toolbox for "cleanly" executing arbitrary code is all I'm looking for here. Ian On Tue, Nov 26, 2019 at 12:45 PM Guilliam Xavier <guilliam.xavier@gmail.com> wrote:

Mike Schinkel

6 years ago
> On Nov 26, 2019, at 11:27 AM, Ian Littman <iansltx@gmail.com> wrote: > > 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. > > 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
There might be a good argument for turning it eval() and create_function() off by default for command-line use? #jmtcw -Mike

Ian Littman

6 years ago
create_function() will be gone as of PHP 8 anyway. For eval(), e.g. psysh uses it (and, as a result, e.g. Laravel Tinker). So defaulting eval to disabled doesn't seem tenable. Not what I'm asking anyway...I just want the *ability* to turn it off (likely only in apache/fpm configs actually, leaving it on for the CLI). On Tue, Nov 26, 2019 at 3:44 PM Mike Schinkel <mike@newclarity.net> wrote:

Stas Malyshev

6 years ago
Hi!
> 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.
I think the important point here is not that you can do worse things than eval() but that you can do *anything*. Once you can execute code on remote side, there's no security barriers PHP can provide for you. If PHP engine were coded as an execution engine for hostile code that guarantees security against untrusted code (like, for example, VM supervisors) then it'd be different, but I don't think PHP engine ever provided that guarantee. And with that, I think banning eval() is just provides false sense of security.
-- Stas Malyshev smalyshev@gmail.com

Stas Malyshev

6 years ago
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

Ian Littman

6 years ago
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:

Benjamin Morel

6 years ago
> > 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.
You don't have to hit disk, or have any writable path. You can just create a stream wrapper <https://www.php.net/manual/en/class.streamwrapper.php> that stores the "files" in memory, and include them as you would include a regular file. 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.
Disabling eval() really doesn't reduce the attack surface at all, if you ask me. Malefactors will quickly & easily adapt their tools, that script kiddies will use as before. — Benjamin

Marco Pivetta

6 years ago
Assuming preloading (big assumption), disabling `eval()` as well as `include*` and `require*` would probably close off most RCEs. That would break a lot of stuff, but it would certainly make for a very interesting experiment. On Wed, Nov 27, 2019, 00:09 Benjamin Morel <benjamin.morel@gmail.com> wrote: