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] [email protected]
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
