[pmwiki-users] Bug: mkdirp crashes

Patrick R. Michaud pmichaud at pobox.com
Sun Apr 10 09:56:59 CDT 2005


On Sat, Apr 09, 2005 at 10:58:42PM +0200, Joachim Durchholz wrote:
> Hi all,
> 
> there's a bug in mkdirp that will make it recurse endlessly trying to 
> create directory '.', resulting in the error message:
> [...]
> Fix:
> 
> In function "mkdirp", change line 365 to read
>   if ($dir == '.' || file_exists($dir)) return;
> (the check for '.' is new).

Hmm, that's bizarre.  The only way that file_exists('.') could be
false is if the current process somehow doesn't have appropriate
permissions to the current directory (in which case, how did we
get to the directory in the first place?) . 

I played with directory permissions on my server and couldn't
duplicate the '.' bug (linux).  Still, that doesn't mean it doesn't
exist for some environments, so I'll probably go ahead and add
the check.  I should probably check for '..' while I'm at it.

> I also noted a few minor bugs and nits to pick in mkdirp. I don't know 
> whether any of these are relevant, so I simply list all of them.
> 
> 1) $safemode is always initialised, but not used unless an error occurs.

Ok.

> 2) In case of problems, $perms is calculated, but not used unless PHP is 
> running in safe mode.

That is *really* a minor nit.  :-)  I think the code reads better without
the braces on the if ($safemode).  Also, when writing it, I didn't know
if I would need the $perms value for the non-safemode text.

> 3) Directories are created with mode 0777 (rwxrwxrwx). This opens a 
> small window of vulnerability until the permissions are fixed. 

No, directories are created with 0775 (rwxrwxr-x) permissions, because
PmWiki sets the umask to 002 earlier in the program.  Creating the
directory with mode 000 doesn't buy any additional security.  Defaulting
to 0755 or 0700 permissions might be useful (and fixperms can then adjust
them), but I think I'll leave this to a wiki admin to control via a 
custom umask setting rather than hardcode one of these values into
mkdirp().

    umask(022);   # create directories and files as 0755
    umask(077);   # create directories and files as 0700

> 4) I don't know why mkdirp tries to create '.'. Must be some borderline 
> behavior of dirname, mkdir, or fixperms.

It would have to be because mkdirp is being sent a strange directory
name, coming from a strange configuration variable setting.  PmWiki
doesn't expect '.' to show up as a directory (but shouldn't care
if it does).

Out of curiosity, what version of PHP was running the system where you
found this error?

> 5) If mkdirp fails due to owner mismatch problems, it shouldn't output a 
> message telling the administrator that he can fix the problem with a 
> chmod. No message would be better, pointing the admin to chown (resp. 
> "rm -rf" followed by "su" or "sudo") would be best.

Not everyone has su or sudo, or even shell access, or even Unix.  The 
current system works for the vast majority of cases--I don't want to 
exclude 50% of potential wiki admins in order to "fix" an error 
message for 2%.  (These numbers are conservative.)

Pm



More information about the pmwiki-users mailing list