Home » PEAR » PEAR » Bug #5888

[2005-11-07 15:50 UTC] murley at townnews dot com

Description:
------------
The pear System::rm() method doesn't properly handle symlinks. Pear's method for removing files and directories is there to mock the unix style rm command. This function supports recursion as well.

When calling System::rm the internal method
_dirToStruct() is called. This method builds an array of elements, testing each to see if it's a file or directory. The is_dir check should be followed by an '&& !is_link' check as no unix system in the world reads through symlinks. Doing so is *very* dangerous, and in your case it even support filesystem jumping.

System::rm works fine on symlinks that are linked to files, but reads through the link when a symlink goes to a directory.

Line 141 of System.php:

if (is_dir($path)) {
$tmp = System::_dirToStruct($path, $maxinst, $aktinst+1);
$struct = array_merge_recursive($tmp, $struct);
} else {

On line 141 an && !is_link should be added. This will cause the function to remove the link and not read into the directory. If left unchanged the linked directory's content is removed but the link isn't thus system::rm will fail.

Change to: Line 141
if (is_dir($path) && !is_link) {
$tmp = System::_dirToStruct($path, $maxinst, $aktinst+1);
$struct = array_merge_recursive($tmp, $struct);
} else {

Test script:
---------------
#!/usr/bin/php
<?php

ini_set('error_reporting', E_ALL);

require_once('System.php');

mkdir('/tmp/testrm1');
mkdir('/tmp/testrm2');
touch('/tmp/testrm2/file1');
symlink('/tmp/testrm2', '/tmp/testrm1/linkeddir2');

echo "Calling system::rm on /tmp/testrm1\n";

System::rm('-rf /tmp/testrm1');

?>

Expected result:
----------------
The expected result should be to remove the testrm1 dir and all it's contents while leaving the testrm2 dir untouched and intact.

My sugessted changes above will fix this. Also note that once this is changed your rm() call will mock all unix style rm's.

Actual result:
--------------
Notice that the file in testrm2 named file1 is gone and shouldnt be. Also notice that the testrm1 dir is still there and contains and the symlink to the directory testrm2 is still there.

[2005-11-07 15:54 UTC] murley at townnews dot com

Sorry, the code change should look like this:

Change to: Line 141
if (is_dir($path) && !is_link($path)) {
$tmp = System::_dirToStruct($path, $maxinst, $aktinst+1);
$struct = array_merge_recursive($tmp, $struct);
} else {

[2005-11-08 13:58 UTC] murley at townnews dot com

Changed bug category

[2005-11-10 02:41 UTC] murley at townnews dot com

Fixed category. Sorry about that.