Req #69090 [Com]: add prefix/xor to cache keys/check permissions or separate caches

From: Date: Thu, 10 Nov 2016 14:59:53 +0000
Subject: Req #69090 [Com]: add prefix/xor to cache keys/check permissions or separate caches
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-205280@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=69090&edit=1

 ID:                 69090
 Comment by:         sjon at hortensius dot net
 Reported by:        simon at ikanobori dot jp
 Summary:            add prefix/xor to cache keys/check permissions or
                     separate caches
 Status:             Analyzed
 Type:               Feature/Change Request
 Package:            opcache
 Operating System:   linux/debian
 PHP Version:        5.6.5
 Block user comment: N
 Private report:     N

 New Comment:

I'd prefer something like https://github.com/SjonHortensius/php-src/commit/21b596086843f02ff0e4d937a17c80e758b27365
(untested wip) instead

It's untested; but shows the general idea. After a successful chroot (either from fpm or
userspace); the path is stored in a global which is prepended to the cache keys.


Previous Comments:
------------------------------------------------------------------------
[2016-11-10 14:25:01] php-dev at coydogsoftware dot net

Simon, that's correct; my patch doesn't fix same-user, multiple chroot scenarios. Search
the comments for "I agree with Rasmus" for my thoughts on that. I was only attempting to
fix the cross-user permissions bypass, which I personally consider the more serious issue. If I knew
the appropriate place to do the stat() call I would have added device+inode, but I've only had
limited time for this and it's my first exposure to PHP internals so I kept the scope small.
Maybe if I get more free time, and reach consensus with the maintainers, I'll have a more
complete fix later.

------------------------------------------------------------------------
[2016-11-10 08:40:29] simon at ikanobori dot jp

Original reporter here, I sitll read the updates to this.

Thanks php-dev for the patch, it seems to work if just the euid is included but correct me if I am
wrong for yes the euid gets set after PHP-FPM forks a new pool under a specific user but just
running separate sites chrooted under the same user (say, www-data) would still cause the same
issues as described in my original report.

Now, that's obviously an unwise way to run any PHP shared hosting environment but it might be
something we want to change as well. I'll take a look at the patch as well but maybe we can do
something closer to inodes to be used in the cache key instead of effective user id.

I'll try to replicate with your patch applied over the weekend :)

------------------------------------------------------------------------
[2016-11-08 13:46:30] php-dev at coydogsoftware dot net

I've just tested suexec/mod_fcgid: This configuration seems to be unaffected because separate
vhosts' php-cgi processes don't share a common parent PHP process. mod_fcgid plays a
similar role to the FPM master, but because it's not PHP and doesn't initialize the
opcache, there's no single SHM object being shared. Each vhost gets its own opcache SHM object
if I understand correctly. 

Thus mod_fcgid with suexec provides better compartmentization than FPM since it has no single parent
PHP process from which all others are descended.

To summarize,
 - apache/mod_ruid2/mod_php is vulnerable.
 - php-fpm is vulnerable (probably regardless of webserver).
 - apache/suexec/mod_fcgid/php-cgi is not vulnerable.

Perhaps php-fpm needs a long hard look at the order in which it initializes pools vs extensions?

------------------------------------------------------------------------
[2016-11-08 07:06:10] php-dev at coydogsoftware dot net

kmark937, I've emailed you directly with a PoC exploit script. I'd love to hear about your
results when you try it.

dol+list, one nitpick with your earlier comments; according to OPCache comments any changes to the
key scheme need to be prepended, not appended, due to the way the key delimiters are parsed (I
wonder what happens when script filenames contain ':'?).

To any PHP project people still reading, would it be possible to reclassify this as a bug report
instead of feature request? This was opened as a feature request at the suggestion of the original
bug reporter in #67481, who withdrew their bug report when they decided that the vulnerable behavior
was desirable after they found a workaround by enabling use_cwd. use_cwd does not fix this issue in
most cases with common CMS applications because the use_cwd logic is only applied to scripts
invoked/included via relative paths, which is less common in real-world web server/CMS environments.
So far I've avoided opening yet another duplicate bug tracker item for this behavior, but it
absolutely needs to be treated as a bug and not a feature request since OPCache documentation does
not warn about it and php-internals discussions dating back to the Optimizer+ days indicate that the
maintainers do indeed intend this feature to be usable in multi-user environments.

jpauli, I do sincerely appreciate that you took the time to comment on this as a PHP project member.
Did you have a chance to review my response? Do you see that multiple FPM pools with separate users
to *not* have the "obvious" separate SHM behavior which you seem to think they do? Would
you care to try my exploit script? I'm trying to give the PHP project every chance to address
the issue before I publish the exploit script, but they've literally had years to address this
and show little interest in fixing it. There's nothing novel about the exploit btw; it does
nothing different from the PoC configs already given in this request and in bug #67481.

------------------------------------------------------------------------
[2016-11-08 01:58:51] kmark937 at gmail dot com

php-dev,

Thanks for providing further insight into this issue. I'd like to take you up on your offer of
a PoC for replicating under mod_php + open_basedir. FPM works as well but I'm particularly
interested in the mod_php case.

Thanks!

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=69090


--
Edit this bug report at https://bugs.php.net/bug.php?id=69090&edit=1


Thread (38 messages)

« previous php.bugs (#205280) next »