[upload] The directories created in _data or upload have all the permissions

[plegall]

Reported by YoBoY on 20 Dec 2012 13:59

Version: 2.4.6

When piwigo create a directory in upload or _data, it gives all the permissions to that directory (777). This is a bad practive in my humble opinion.

A better approch should be to read the permissions of the parent directory and to give the same permissions to the new directories. We can also create a new configuration parameter and let the users choose the best permission for them.

Steps to reproduce: 1- Upload a file with piwigo 2- look at the permissions of the new directories

Additional information: It is done on this file. http://piwigo.org/dev/browser/branches/2.4/admin/include/functions_upload.inc.php#L379

Piwigo Bugtracker #2811

[plegall]

Comment posted by plg on 1 Jan 2013 12:53

Actually I see that rvelices had already avoided this problem in svn:12802 but only function mkgetdir uses $conf['chmod_value'] and function prepare_directory doesn't use mkgetdir.

We should use mkgetdir (Piwigo function) instead of mkdir (php core function).

[rorrison]

Dreamhost is regularly emailing me to say that my site is compromised, because of this issue. It looks like the fix is known, could it be implemented soon? Thanks!

[flop25]

no the issue here is not what you are thinking Here it's a about changing a default php function by our internal function ; see the second message

I checked the code and the chmod is set to 777 if and only if the system permission doesn't allow php to read the directory while it was created by php if (!is_writable($directory)) { // last chance to make the directory writable @chmod($directory, 0777); OR if the default chmod is 777 umask(0000); Try to change to 755/644. If it doesn't work and if it is a shared server, i am afraid we won't be able to change that.

[rorrison]

Thanks for your reply. I've done a bit more testing and fiddling with the code.

I uploaded a file, which created upload/2017/10/13 with permissions 777. I then manually changed the permissions on the directory to 755 and uploaded another file - with no problems. I then deleted both uploaded photos, and the folder.

Next, I changed four occurrences of 777 in admin/include/functions_upload.inc.php to 755. The folder was created again, this time with permissions 755 and the upload worked fine.

Certainly in my shared server configuration, there's no need for the permissions to be 777. In the code where it says "last chance" it doesn't look like it's actually tried safer permissions - in the function prepare_directory it sets the umask to 0 and then creates the directory with permissions 777.

I've also noticed that in the upload directory there's a file index.htm which is created with 666 permissions - that being world writable seems like another potential security problem.

I'm thinking that the umask and permissions should be configurable, not hardcoded, and should default to a secure setting (022 and 0755). Then only if that's causing problems for someone they could be changed. But the default should be as secure as possible. (Changing umask to 022 fixes the index.htm permissions as well.)

[flop25]

what's the permission chmod and owner of the parent folders? (upload , _data/i )

[rorrison]

My website root is in the folder ~/orrison.com:

drwxr-xr-x 22 orrison pg61395 4096 Sep 15 05:48 orrison.com/
drwxr-xr-x 15 orrison pg61395 4096 May  2 12:50 gallery/
drwxr-xr-x  7 orrison pg61395   87 Apr 24 07:16 upload/
drwxr-xr-x  7 orrison pg61395   75 Oct 12 00:37 2017/
drwxr-xr-x  4 orrison pg61395   36 Oct 13 06:36 10/
drwxr-xr-x  2 orrison pg61395   68 Oct 13 06:36 13/

_data and _data/i have the same ownership and permissions

ps -ef doesn't show me apache, but does show me php70.cgi running as user orrison