On 23/02/12 09:48, Sylvain Rochet wrote:
Hi,
On Wed, Feb 22, 2012 at 05:41:51PM +0000, Terry Ellison wrote:
The major part of this overhead is involved in the separate
walking of the path and real paths of the script to enforce the
suPHP access rules (even though these are commonly the same). This
is compounded by the coding of
suPHP::Application::checkParentDirectories() which results in 4
lstat calls per check, [...]
I guess this is related to CVE-2008-1614.
Sylvain, thanks for this. Yes, the code introduced in 0.63 in response
to CVE-2008-1614 to walk the directory chain on the path and realpath is
a factor, but the root issue is as much to do with the design and use of
the File object and its suPHP:API_Linux helper.
(i) The only private property is the string holding the file path, which
is initialised in the constructor. This means that any method which
interrogates a file property, must call the appropriate file system
call. Hence we have the following methods (or their API_Linux helpers)
all perform an lstat on the file: isSymlink(), exists(),
hasPermissionbits(), getUser(), getGroup(), and hence lstat is called
multiple times for a single logical operation. It would be trivial to
collect the lstat result in a struct stat private property, so that this
only needs to be called once per File object.
(ii) File(scriptFile.getRealPath()) called three times in
suPHP::Application methods checkScriptFileStage1, checkScriptFileStage2,
and checkProcessPermissions. Again why?
(iii) Whilst the code must handle robustly perverse scenarios involving
bizarre symlinking, the path and realpath are often identical; so that
is makes sense to qualify any further realpath checks on (path != realpath).
(iv) We could go to the extreme of considering using a map to maintain
and and to reuse File objects. However, given that addressing (i) to
(iii) would reduce the file-system calls by ~8x, I am not sure there
would be any material benefit for this additional complexity.
I am willing to develop and to propose a patch to address these issues
if the patch is likely to be used.
Comments?
_______________________________________________
suPHP mailing list
[email protected]
https://lists.marsching.com/mailman/listinfo/suphp
