Bug #52312 [Com]: PHP safe_mode/open_basedir - lstat performance problem

From: Date: Sat, 23 Feb 2013 00:08:51 +0000
Subject: Bug #52312 [Com]: PHP safe_mode/open_basedir - lstat performance problem
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-177340@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=52312&edit=1

 ID:                 52312
 Comment by:         Terry at ellisons dot org dot uk
 Reported by:        v dot damore at gmail dot com
 Summary:            PHP safe_mode/open_basedir - lstat performance
                     problem
 Status:             Analyzed
 Type:               Bug
 Package:            Safe Mode/open_basedir
 Operating System:   Linux
 PHP Version:        5.2.13
 Block user comment: N
 Private report:     N

 New Comment:

For the purposes of this bug, let's document the advisory which triggered this change:
CVE-2006-5178 see http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2006-5178.

Let's be honest, even with this patch there is still a minuscule chance that the exploit
described could succeed, because we can't go into kernel mode during the recursive descent and
prevent other processes with a higher nice being scheduled in and doing the dirty. So it replaces
"very small" by "even smaller" at the same time killing NFS performance.

As I said don't remove this change, just make it elective by adding an extra variant of the
open_basedir parameter, and openly documenting the issues so that sysadmins can make their own
judgement call on security vs performance.  Let's face it on a typical shared service I can
think of easier ways to do this exploit.  E.g. if exec and the proc functions aren't disabled
then there is no point in having a strong open_basedir.  If you are an enterprise and just want to
lock down your apps programmers from breaching good practice / infrastructure standards over a NAS
infrastructure then the weak form is perfectly appropriate.

Also IMO, PHP is trying to do a better job here than the standard Linux realpath function and
failing.


Previous Comments:
------------------------------------------------------------------------
[2013-02-22 23:26:14] rasmus@php.net

There is no such thing as a "miniscule race" when it comes to computers. Either 
there is a race condition or there isn't. In this case there is. So if we remove 
this check open_basedir will be much less secure. Something along the lines of 
Marcin's idea of separate caches might be feasible, but this is not a small 
change.

------------------------------------------------------------------------
[2013-02-22 23:17:59] Terry at ellisons dot org dot uk

This bug is extant in 5.3 and 5.4 (up to 5.4.9).  The flaw is the logic in
main/main.c:php_module_startup():

	/* Disable realpath cache if safe_mode or open_basedir are set */
	if (PG(safe_mode) || (PG(open_basedir) && *PG(open_basedir))) {
		CWDG(realpath_cache_size_limit) = 0;
	}

Similar logic validly exists in code such as suEXEC and suPHP to prevent a race condition allowing
and unscrupulous script author to execute a script in another UID in certain circumstances.  This
simply doesn't apply in this case.  Which technically a similar race could allow a script
author to break the basedir contain with miniscule probability by exploiting such a race, the
performance impacts are significant, for no material increase in security strength.  Get rid of this
rule or at least split open_basedir into two separate directives open_basedir_paranoid which
disables the cache as above and open_basedir which does the rational checks and doesn't disable
the cache.

This has been lying around for nearly 3 years.  How can we progress this?

------------------------------------------------------------------------
[2012-07-31 20:00:48] marcin at 6irc dot net

How about having concurrent cache tables for each basedir setting? For instance, when open_basedir
is set to '/home/teh1234;/tmp', then the lstat populates only cache table "0",
realpath_cache_* also uses exclusively this cache, and when open_basedir is set to
'/home/klaczy;/tmp' then it populates and uses only cache table "1"? Are there
any security considerations that I don't notice here?

------------------------------------------------------------------------
[2011-08-16 18:52:34] spam2 at rhsoft dot net

> Caching open_basedir stats is insecure

not really because the permissions are not changed the whole day

------------------------------------------------------------------------
[2011-07-03 21:21:21] rasmus@php.net

I really don't see a middle ground here. You are either secure or you aren't. Caching
open_basedir stats is insecure and the whole point of open_basedir in a shared hosting setup is to
secure these file accesses. If you don't care about security, turn it off and live with the
security issue, or better yet, change your shared hosting setup to use VMs or other lower-level
strategies that keep users separated from each other.

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


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=52312


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


Thread (64 messages)

« previous php.bugs (#177340) next »