[drupal-devel] [bug] files folder created with incorrect permissions

Morbus Iff drupal-devel at drupal.org
Thu Jan 20 16:27:30 UTC 2005


 Project:      Drupal
 Version:      cvs
 Component:    file system
 Category:     bug reports
 Priority:     normal
 Assigned to:  Morbus Iff
 Reported by:  Anonymous
 Updated by:   Morbus Iff
 Status:       patch

In chatting about this with #drupal, I'm thinking that something more
than just "sane defaults" needs to be enabled. The patch I just
submitted still has one "fatal" flaw: it doesn't allow write
permissions for the FTP user. This was ultimately deliberate (more
chance of accepting the patch with less dangerous permission, and the
assumption that a Drupal-controlled upload also offered a
Drupal-controlled deletion), but still isn't ideal.

Unfortunately, with SuEXEC servers and various configurations, there is
no SAFE default: on one hand you're giving 666 to everyone because you
have to (wwwrun:nogroup) and on the other, 666 is too much (if you're
running under SuEXEC, then gangrene:users/640 is right, and 666 is
wrong).

I think the only way this will be truly solved is with user
intervention, which means umask settings for file/directory in the
"Files" section of admin/setting. This is also the same solution that
Movable Type uses, which allows two special settings in its mt.cfg:


# When creating files and directories, Movable Type uses umask settings
to
# control the permissions set on the files. The default settings for
file
# creation (HTMLUmask, DBUmask, and UploadUmask) are 0111; for
directory
# creation (DirUmask), the default is 0000. You should not change
these
# settings unless you are running MT under cgiwrap or suexec, or some
other
# scenario where the MT application runs as you; in addition, you
should not
# change these settings unless you know what they mean, and what they
do.
#
# DBUmask 0022
# HTMLUmask 0022
# UploadUmask 0022
# DirUmask 0022
#
# In addition to controlling permissions via umask settings, you can
also
# use the HTMLPerms and UploadPerms settings to control the default
# permissions for files created by the system (either as output files
or
# uploaded files). The only real use of this is to turn on the
executable bit
# of files created by the system--for example, if MT is generating PHP
files
# that need to have the executable bit turned on, you could set
HTMLPerms
# to 0777. The default is 0666. You should not change these settings
unless
# you know what they mean, and what they do.
#
# HTMLPerms 0777
# UploadPerms 0777


For our needs, we'd really only need a DirectoryUmask and a FileUmask
option. Is this an amicable solution to anyone? If I get you a patch,
will this be implemented in Drupal 4.6?

Morbus Iff



Previous comments:
------------------------------------------------------------------------

September 8, 2004 - 13:04 : Anonymous

Hi,-

> [rreading at tribal html]$ ls -l www/files
> total 12
> drwxr-----        2 nobody   nobody       4096 Sep  8 12:22 pictures
> drwxrwxrwx    2 rreading users          4096 Sep  8 12:33 tmp
> drwxr-----        3 nobody   nobody       4096 Sep  8 12:33 upload

the problem is very strange, and might even be a nasty drupal bug: As
of the CVS version we decided to let drupal generate the folders
itself, so that Joe User needs not to use ssh or ftp to make /configure
various folders. 
So these were generated by PHP and thus have the wrong permission. The
user PHP is a differnet user in this case, and something that PHP
created is inacessible to us. 
I am aware that this is sierverspecific, but its annoying enought for
me (and probably others) not to ignore. 

Regards, Ber

------------------------------------------------------------------------

November 18, 2004 - 21:46 : dtan

Any update on this. .  ?? I recently installed the 4.5 Head and the
problem still exists.  Recommendations on how to actually change the
permission on the folder?

dtan

------------------------------------------------------------------------

November 28, 2004 - 10:53 : killes at www.drop.org

The apache user did create this directories. So it is natural that they
have the apache user "nobody" as owner. Drupal should be able to handle
this just file. Can we close this report?

------------------------------------------------------------------------

November 28, 2004 - 10:58 : Bèr Kessels

No, please do not close. Its a bug IMO.

Because a lot of (v)hosts do not put apache in the same group as the
FTP user. It will work, file uplaods etc will work for Drupal, but you,
as user, cannot access the files anymore by FTP.
But even worse, you cannot remove your drupal installation, without
intervention of root (or the server maintainer). I had two "empty"
domains with 30 MB of files taking up space for 3 months.
Another domain will not let me mirror (backup) anything because of
this. 

------------------------------------------------------------------------

November 29, 2004 - 22:55 : svemir

If PHP/apache can create these folders, it should be able to remove them
as well. Perhaps an option somewhere to remove folders created by
Drupal?  Maybe a button next to the place in administration where the
folder is specified. And also maybe the creation of the folders should
be optional in the first place...

------------------------------------------------------------------------

January 19, 2005 - 20:52 : Morbus Iff

This just bit me today too. Installed the banner.module, and ensured
that my files/ directory was 777, with the owner as gangrene:user.
Uploaded a new banner, and for various reasons, it didn't work. As I
started to investigate, I found that the files/banner directory, which
was "successfully" created (success determined by "I can access files
uploaded in that folder via a URL) was not visible to my FTP server (at
this point, proFTPd, though I've not yet tested on vsftpd).

One of ProFTPds feature is to hide folders that do not have the
executable bit set. The default permissions of Drupal is not to set
these, per file_check_directory in includes/file.inc. So, my newly
Drupal-created files/banners folder was given the permissions of (where
wwwrun and nogroup are the Apache user/group).

drwxrw----   2 wwwrun   nogroup        88 Jan 19 20:03 banners

Since there's no execute for everyone, and since the "gangrene" user is
not part of "nogroup", I was not able to see, access, or modify the
newly created folder. I had to manually issue chmod('files/banners',
0761) to even look at the blasted thing.

My recommendation is to change the file_check_directory chmod to 0771,
which should create drwxrwx--x permissions. The extra execute
permission on the "nogroup" is harmless, and allows ProFTPd to display
it (in this case to "nogroup", but also perhaps to the "users" group on
other systems), and the execute permission for everyone is absolutely
required for me to see it in under ProFTPD. These permissions are
giving any extra (or damaging) permissions to anyone, but merely
following the expectations of various FTP servers concerning "what a
directory is".

There's a similar bug related to uploaded files, but I've not yet found
the code for that one.

------------------------------------------------------------------------

January 19, 2005 - 20:55 : Morbus Iff

Grrrr... "these permissions are NOT giving any extra (or damaging)
permissions".

------------------------------------------------------------------------

January 19, 2005 - 21:12 : Morbus Iff

To revise my previous statement, it would be best to use perms 0765,
which adds "read" access to the "everyone" group. The correction is
simple: if we want to /see/ the folder in FTP, then we also want to see
/inside/ the folder via FTP. Again, I don't believe this change really
gives extra dangerous permissions to anyone, merely grants the same
permission to an authenticated FTP user (if a file upload can be seen
on the web by people who know its there, then the same access should be
applied to FTP users with the right knowledge [ie., un/pw]).

------------------------------------------------------------------------

January 19, 2005 - 21:29 : Morbus Iff

Alrighty, hunted down the file (not directory) related problem. Unlike
directories, which require us to enter a mode, file uploads are not
specified, thus they take on the default umask of the web server's user
(in my case, wwwgroup:nogroup, 640). This further causes problems with
FTP viewing because "everyone" has no permission (in my case, since I'm
a part of the "users" group, I can not view, modify, delete, mirror,
etc. files that have been uploaded through Drupal).

This has been tracked down to file_copy() (includes/file.inc) and the
following lines, which copy the uploaded file from the temp directory
($source) to the final destination ($dest).


    if (!@copy($source, $dest)) {
      drupal_set_message(t('File copy failed.'), 'error');
      return 0;
    }


My proposal is to add an explicit chmod after the copy, granting read
permission to "everyone", something to the effect of the following,
which will give read and write to the webserver process, read to the
webserver group, and read to everyone (ie., little old me, not of the
webserver group).


    if (!@copy($source, $dest)) {
      drupal_set_message(t('File copy failed.'), 'error');
      return 0;
    }
    chmod($dest, 0644).



------------------------------------------------------------------------

January 19, 2005 - 21:40 : Morbus Iff

Attachment: http://drupal.org/files/issues/fileperm.patch (606 bytes)

Attached is a patch for Drupal 4.5.1 that implements these changes
(directory is set for 764, not 765 - the mode in my previous comment
was [another] error). Last time I contributed a patch, I created a bug
so please review extra carefully.

-- 
View: http://drupal.org/node/10658
Edit: http://drupal.org/project/comments/add/10658





More information about the drupal-devel mailing list