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:
Incidentally I've just straced opening a require_once on
/home/terry/work/ext/lpc/tests/dd2.inc
and PHP 5.3.17 does the 7 x lstat walk that you indicate is necessary to meet CVE-2006-5178, but it
then repeats this another 5 times and does another 3 fstats after the file is opened 45 f/lstats all
in 2.5μS (on a local disk clearly).
PHP 5.4.6 is worse, it does 5x7 walks, opens the file then does 4 fstatsand reads the file, before
doing another 3x7 lstat walks and a last stat for luck, giving a grand total of 60 f/lstats in
4μS though this one includes mmapping the contents from the file cache.
OK, I can understand -- though question -- mandating one walk but why do it seven times ????
Previous Comments:
------------------------------------------------------------------------
[2013-02-23 00:08:51] Terry at ellisons dot org dot uk
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.
------------------------------------------------------------------------
[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
------------------------------------------------------------------------
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