php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #75605 Dumpable FPM child processes allow bypassing opcache access controls
Submitted: 2017-11-30 18:27 UTC Modified: 2018-04-29 20:47 UTC
From: jd at cpanel dot net Assigned: bukka (profile)
Status: Closed Package: FPM related
PHP Version: 5.6.32 OS: linux
Private report: No CVE-ID: 2018-10545
 [2017-11-30 18:27 UTC] jd at cpanel dot net
Description:
------------
This was tested with PHP 5.6.32, but the behavior looks identical in newer versions of PHP.

After changing UID and GID, PHP-FPM sets pool worker processes to be dumpable. This allows a local user with the same UID and GID to attach to the PHP-FPM workers and gain access to any restricted resources that are not supposed to be allowed.

For a simple example:

- Configure PHP-FPM under Apache with two pools running as different users (victim & attacker)

- Enable opcache and configure it safely for a multiuser environment (opcache.validate_permission=1). The example here is also assuming a MMAP cache.

- Install wordpress in the victim account's docroot and load a few wordpress URLs.

- Install a PHP script that sleeps 60 seconds into the "attacker" account's docroot.

- Load the sleep script in the attacker account's docroot.

- As the attacker account, run "gcore <php-fpm-worker-pid>" to create a coredump of the PHP-FPM worker process.

- Run strings on the coredump file to retrieve the victim account's wordpress database username and password.

Expected result:
----------------
It should not be possible for unprivileged users to ptrace() the FPM worker processes or cause them to dump core.

Actual result:
--------------
Sensitive configuration data for other accounts can be accessed directly in the PHP worker process's memory.

Patches

bug75605-patch-for-7-0 (last revision 2018-03-18 20:05 UTC) by bukka@php.net)
bug75605-patch-for-5-6 (last revision 2018-03-07 18:16 UTC) by bukka@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-12-01 21:58 UTC] stas@php.net
I'm not sure it is reasonable to expect from PHP to control system-level functions like ptrace. There are system-level means - like https://linux-audit.com/protect-ptrace-processes-kernel-yama-ptrace_scope/ - to control this. If you run a multi-user shared service with shell access that's what you probably want to do.
 [2017-12-01 22:16 UTC] jd at cpanel dot net
The kernel already doesn't allow this behavior. PHP-FPM is specifically asking for it to be allowed by calling prctl(PR_SET_DUMPABLE...) in fpm_unix.c.
 [2017-12-01 23:21 UTC] stas@php.net
Hmm in that case it indeed seems wrong. I am not sure why it is doing that, I would like to hear from the maintainer, but it looks like this code can be removed.
 [2017-12-01 23:21 UTC] stas@php.net
-Assigned To: +Assigned To: fat
 [2018-01-11 16:04 UTC] jd at cpanel dot net
Any updates on this? The fix is rather trivial, so it's not clear what is holding up progress on this bug.
 [2018-02-23 03:25 UTC] stas@php.net
-Assigned To: fat +Assigned To: bukka
 [2018-02-23 12:29 UTC] bukka@php.net
I think that the main issue is that the opcache doesn't consider the concept of separate pools and share resources across the pools which causes other issues like for example https://bugs.php.net/bug.php?id=74709 . However the solution of this problem might be quite complex and needs a bit more thinking. For example we could consider doing MINIT on the pool level but that might have some consequences or having new module entry callbacks for that which would require some ABI changes. It needs a bit more thinking and some discussion though.

If the opcache didn't have access to the shared memory of other pools, it wouldn't be an issue to have PR_SET_DUMPABLE set. It can be actually quite useful for tracing and some applications may rely on it already.

I think that the question here is if we consider pools as a secure environment that can be for example safely used for shared hosting. I don't think that it's the case until we resolve the separation issue above as there might be more such issues. Also I wouldn't like to break some apps relying on the current behavior. What we could do is to introduce a configuration setting that will however default to the current behaviour.
 [2018-02-23 18:46 UTC] stas@php.net
@Jakub I understand these complications, but still not sure - is there a legit reason why FPM does prctl(PR_SET_DUMPABLE) for child processes? I have hard time imagining any PHP application needing this low-level system flag or even knowing about it, but we could allow it for some special cases with FPM config. Do we need it in the default case though?
 [2018-02-25 20:28 UTC] bukka@php.net
@stac That flag is there to make FPM tracing available which uses ptrace to get execution related data (see fpm_php_trace_dump) which is then logged to slowlog. The tracing is part of FPM and it's integrate into it so I think removing that could potentially break something...
 [2018-02-26 06:48 UTC] stas@php.net
If the trace is just a debug facility then I think we can safely put it under a flag, for those who need it, and set default to the secure setting which is not allowing everybody the access.
 [2018-02-26 12:11 UTC] bukka@php.net
Well I would say it's probably more than just a special debugging facility. It's the whole slowlog functionality. Basically in FPM you can specify request_slowlog_timeout for a pool. If this is specified, the child run time is checked using heartbeat event and the time run is compared with the allowed timeout. If it exceeds the timeout specified in request_slowlog_timeout, the ptrace is attached which stops the child and log all the information to the slowlog file (another pool configuration). It means that this might be quite important functionality for some users. Also there is not much point to disable it for users that don't use different users for pools
 (meaning having at least 2 pools with different users) as there is no security risk IMO. If this should be default, then just for the configurations have such risk. I 
think something like this:

request_slowlog_trace_enabled = (auto|on|off)

The auto would be default and disable it for for the risky configurations where more than two different users are in pools.

What do you think?
 [2018-02-26 17:18 UTC] jd at cpanel dot net
I might be misunderstanding how the slowlog functionality works, but in the scenario where the FPM child process changes UID and GID (and thus loses it's DUMPABLE flag so that the PR_SET_DUMPABLE call has some effect), wont the process calling PTRACE_ATTACH (fpm_trace_signal) be running as root?

If that is the case, the kernel wont stop the parent from attaching to the child, regardless of the DUMPABLE flag on the child process. The parent process in this case will have the CAP_SYS_PTRACE capability since it's running as root, and calling PR_SET_DUMPABLE in the child is redundant.
 [2018-02-26 18:08 UTC] bukka@php.net
Ah it looks you are right actually. Yes it will be root that attaches the ptrace. I thought that it applies on any user from reading the docs in http://man7.org/linux/man-pages/man2/prctl.2.html that states just:

  Processes that are not dumpable can not be attached via
  ptrace(2) PTRACE_ATTACH; see ptrace(2) for further details.

However it's extended in the ptrace docs (which I didn't read :) ):

4. Deny access if the target process "dumpable" attribute has a value
   other than 1 (SUID_DUMP_USER; see the discussion of
   PR_SET_DUMPABLE in prctl(2)), and the caller does not have the
   CAP_SYS_PTRACE capability in the user namespace of the target
   process.

It actually makes sense as root should be allowed to do ptrace in any case. I should have read it properly or at least try it :)

In that case I think it makes sense to disable it by default. Will prepare a patch and experiment with that once I get little bit of time.
 [2018-03-07 18:16 UTC] bukka@php.net
The following patch has been added/updated:

Patch Name: bug75605-patch-for-5-6
Revision:   1520446582
URL:        https://bugs.php.net/patch-display.php?bug=75605&patch=bug75605-patch-for-5-6&revision=1520446582
 [2018-03-07 18:18 UTC] bukka@php.net
I just attached initial patch for 5.6. I will need to test a bit more and also check if it applies to higher branches. Of course I will be more than happy if you can also test it and have some feedback! Thanks.
 [2018-03-18 20:05 UTC] bukka@php.net
The following patch has been added/updated:

Patch Name: bug75605-patch-for-7-0
Revision:   1521403557
URL:        https://bugs.php.net/patch-display.php?bug=75605&patch=bug75605-patch-for-7-0&revision=1521403557
 [2018-03-18 20:52 UTC] stas@php.net
lgtm, I'd propose to merge it to the next release.
 [2018-03-22 20:58 UTC] bukka@php.net
I tested on 5.6 and 7.0 only. Will try to test on 7.1+ over the weekend but it should be fine IMO.
 [2018-03-25 19:59 UTC] bukka@php.net
Just did a quick test with 7.1 and 7.2 and seems good. I think it's good to go to to the next version. Cheers.
 [2018-03-27 09:16 UTC] remi@php.net
Open to discussion, how the "shared hosting" issue should be managed ?

https://wiki.php.net/security should list this explicitly, and IMHO, all "shared hosting" should be marked as "Low" (so going throw the next RC)
 [2018-03-27 12:34 UTC] ab@php.net
-Status: Assigned +Status: Closed
 [2018-03-27 12:34 UTC] ab@php.net
Patch applied.

Thanks.
 [2018-03-28 04:03 UTC] stas@php.net
-CVE-ID: +CVE-ID: needed
 [2018-04-29 20:47 UTC] kaplan@php.net
-CVE-ID: needed +CVE-ID: 2018-10545
 
PHP Copyright © 2001-2018 The PHP Group
All rights reserved.
Last updated: Mon Jul 16 14:01:38 2018 UTC