Edit report at https://bugs.php.net/bug.php?id=52312&edit=1
ID: 52312
Comment by: pierre dot renaudet at gmail dot com
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:
We have this issue with Symfony(3.1.3), it's really poor performance with lot of stat on
file...
With the same project on Windows (IIS 7.5 - PHP 5.6.1
With open_basedir (empty) it's take ~300-350ms for the index
With open_basedir set (~9 path) it's take 3.5-10s for the index
I understand security problem, but why not just clear cache (before and/or after risky function)
things like that :
--------------------
diff --git a/ext/standard/link.c b/ext/standard/link.c
index 62e7295..a844f46 100644
--- a/ext/standard/link.c
+++ b/ext/standard/link.c
@@ -158,6 +158,11 @@ PHP_FUNCTION(symlink)
RETURN_FALSE;
}
+ /* Reset realpath_cache when open_basedir is not null to avoid security issues */
+ if(PG(open_basedir)){
+ realpath_cache_clean();
+ }
+
/* For the source, an expanded path must be used (in ZTS an other thread could have changed the
CWD).
* For the target the exact string given by the user must be used, relative or not, existing or
not.
* The target is relative to the link itself, not to the CWD. */
@@ -206,6 +211,11 @@ PHP_FUNCTION(link)
RETURN_FALSE;
}
+ /* Reset realpath_cache when open_basedir is not null to avoid security issues */
+ if(PG(open_basedir)){
+ realpath_cache_clean();
+ }
+
#ifndef ZTS
ret = link(topath, frompath);
#else
diff --git a/ext/standard/link_win32.c b/ext/standard/link_win32.c
index 7d43162..e47e265 100644
--- a/ext/standard/link_win32.c
+++ b/ext/standard/link_win32.c
@@ -168,6 +168,12 @@ PHP_FUNCTION(symlink)
php_error_docref(NULL, E_WARNING, "UTF-16 conversion failed (error %d)",
GetLastError());
RETURN_FALSE;
}
+
+ /* Reset realpath_cache when open_basedir is not null to avoid security issues */
+ if(PG(open_basedir)){
+ realpath_cache_clean();
+ }
+
/* For the source, an expanded path must be used (in ZTS an other thread could have changed the
CWD).
* For the target the exact string given by the user must be used, relative or not, existing or
not.
* The target is relative to the link itself, not to the CWD. */
@@ -223,6 +229,11 @@ PHP_FUNCTION(link)
RETURN_FALSE;
}
+ /* Reset realpath_cache when open_basedir is not null to avoid security issues */
+ if(PG(open_basedir)){
+ realpath_cache_clean();
+ }
+
#ifndef ZTS
ret = CreateHardLinkA(topath, frompath, NULL);
#else
diff --git a/main/main.c b/main/main.c
index bb98f27..7e5904e 100644
--- a/main/main.c
+++ b/main/main.c
@@ -2222,7 +2222,7 @@ int php_module_startup(sapi_module_struct *sf, zend_module_entry
*additional_mod
/* Disable realpath cache if an open_basedir is set */
if (PG(open_basedir) && *PG(open_basedir)) {
- CWDG(realpath_cache_size_limit) = 0;
+ /* CWDG(realpath_cache_size_limit) = 0; */
}
/* initialize stream wrappers registry
--------------
Previous Comments:
------------------------------------------------------------------------
[2016-04-26 09:19:07] ianphp at binkmail dot com
This problem causes extreme loading on otherwise powerful servers when using PHP7/FastCGI/IIS, so
bad in fact that it's unusable for busy sites on an otherwise powerful server.
Unfortunately because this is a Windows server, the workarounds are not applicable.
I think that patch proposal is a good start to getting this fixed.
------------------------------------------------------------------------
[2013-12-16 16:39:10] pembo13 at gmail dot com
I seem to be suffering from this bug on an Apache/Linux + nfs setup. I'm using open_basedir and
the performance is so poor, that I can't even really stress test the server any more.
------------------------------------------------------------------------
[2013-05-29 21:45:36] phpdotnet at hostultra dot com
This bug is a real performance killer.
I propose this solution...
1. Modify symlink() php function so that if open_basedir or safe_mode is on, it disallows relative
symlinks with .. components, instead it creates absolute symlink.
This prevents attacker from exploiting CVE-2006-5178.
2. Allow the realpath cache to work even if open_basedir or safe_mode is on.
I did this with my own php code.
Before
---------
last pid: 30437; load averages: 32.93, 24.67, 15.62
98 processes: 43 running, 46 sleeping, 9 lock
CPU: 2.7% user, 0.0% nice, 75.3% system, 0.5% interrupt, 21.5% idle
After
---------
last pid: 30582; load averages: 2.06, 3.57, 10.58
68 processes: 6 running, 62 sleeping
CPU: 6.8% user, 0.0% nice, 1.7% system, 0.9% interrupt, 90.6% idle
------------------------------------------------------------------------
[2013-03-05 13:11:32] Terry at ellisons dot org dot uk
Rasmus, picking up our 2013-02-22 23:17 / 23:26 UTC conversation, I've thought about this some
further and gone through some test cases on the debugger.
Having walked through these call stacks, my view is that the PHP / Zend path scanning, file checking
and opening is a tangled mess. A typical open goes through sometimes 7 seven wrapping layers each
of which can do compound path resolution. The only reason that this doesn't end up slugging
performance is because the lowest level tsrm_realparth_r() uses a resolution cache and
short-circuits the actual I/O requests 95+% of the time.
The CVE-2006-5178 advisory really to a vulerability in the open_basedir checks. The root issue was
that a key check in php_check_specific_open_basedir() was using this cached path resolution; this
introduced the vulnerability because the caching enabled a race condition. The fix was to turn of
ALL realpath resolution caching. Yes this addressed the vulnerability, but at the price of killing
performance in the typical usecase where open_basedir might be used. This was unnecessary overkill.
If you think about it realpath caching itself doesn't introduce any vulnerability. The issue
is that the open itself -- or in this case the preceding php_check_specific_open_basedir() check
must disable any caching for that one check alone. Consider an example there the base dir is /a/b/c
and some path needs to be checked by php_check_specific_open_basedir(). There's not
vulnerability introduced by any previous resolution being used. This will result in one of two
scenarios:
* The resolved path is not of the form /a/b/c.... in which case the error is thrown
* The resolved path is of the form /a/b/c... but the actual path might contain raced symlinks, so
it must be scanned (by tsrm_realparth_r) to ensure that no links are extant immediately prior to the
open. There is also a sound case for curtailing this scan on a root-owned directory, but this of
second order.
Implementing this is a small code change.
(i) First drop the open_basedir predicated clearing of CWDG(realpath_cache_size_limit) = 0 in
main/main.c:php_module_startup().
(ii) introduce a per-call mechanism for disabling the cache. This could be done by adding a flag
to realpath, but the two variants (ZTS and none-ZTS) and the wide use of the wrapper VCWD_REALPATH()
macro might complicate this. An alternative would be to add another flag to the PG() structure,
checking this in tsrm_realparth_r() and setting it in php_check_specific_open_basedir() around the
VCWD_REALPATH(path_tmp, resolved_name) call.
Reactions / Thoughts? Is it worth me proposing a patch?
------------------------------------------------------------------------
[2013-02-23 16:06:50] Terry at ellisons dot org dot uk
Yes Rasmus. We both know that; but this won't be address without something like an LPC-style
file-based cache to preserve context across image activations, but all this isn't that relevant
to #52312 -- "PHP safe_mode/open_basedir - lstat performance problem".
What is relevant are my points about a require_once 6 sub-directories down taking 13 stats and 1
open with open_basedir unset and 60 stats and 1 open if it is set, and that the security requirement
could still be implemented within the former stat number if done correctly.
This isn't a material problem for single source file scripts, but MediaWiki, Wordpress and the
like typically load in ~100 modules generating ~6K stats per request. And this does become a
problem.
------------------------------------------------------------------------
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