Eliminate hardcoded permissions

[MrPhil]

One common problem I see on this forum is that the hardcoded file and directory permissions scattered throughout SMF's code are not always appropriate for various users. First of all, 0755 (execute bit "on") is rarely needed for files. On some strangely configured servers, PHP files might indeed need to be "executable", but on most, no. Secondly, directory permissions might be given as 0755 when PHP is running as "group" or "world", and won't be able to write to the directory. Sometimes a check is made and 0777 tried instead, but that's unsafe, particularly if 0775 will do the job. At the other extreme, just giving a blanket 0777 (the lazy programmer's approach) might result in security software such as "suPHP" shutting down access to a directory, because it's "world writable" (PHP is running as "owner", so 0755 is the proper setting). It's always best to run with the least permissions possible, for what activities need to be done with a file or directory.

I propose to do two things: first, to consolidate file and directory permission settings into one place, and second, to automate (if possible) these settings during installation. The user would always be able to go back and change the permissions if need be -- they could be stored as defines in Settings.php or elsewhere, so long as that file is always required into any other page that uses permissions (currently hardcoded). Worst case, a plausible set of permissions would be given by default, and the webmaster would have to check and update them before running install.php.

What file permissions are needed for SMF? Most files don't need to be written to during runtime except Settings.php, Settings_bak.php, Packages/installed.list, agreement.txt, Themes/classic/index.template.php, and Themes/classic/style.css. Even that short list should be treated with care, as corrupted (emptied) Settings.php files are one of the leading causes of problems reported on this forum! Uploaded attachments and avatars may also need to be writable, at least for some part of their life. Of course, any file is subject to being edited or overwritten during upgrades and its permissions should be set accordingly. For files that need to be written to, the necessary permissions will vary by server configuration, i.e., does owner, group, or world get the "write" bit? Should the "owner" always be writable (644 minimum) for a file, or are there cases (such as Settings.php) where we might want to allow read-only settings once setup is done? For some servers, it is supposedly appropriate to 0 out group and/or world (not even "read" bit set), but on others it is necessary to have "read" ability for files. I don't know if there's any good way to automatically detect this, or if it is even really necessary (just an old wives' tale?).

Anyway, for files, I suppose that they could be shipped and unzipped/untarred with 0644 permissions, and then one file be tested with is_writable() to see if group and/or world need their "write" bits set (0664, 0646, or 0666). If a change is needed, change and save the defined value for RW_FILE from 0644 to whatever's appropriate, and do a chmod() on the list of files that are supposed to be writable. The same thing can be done for a file which is supposed to be read-only, testing with is_writable() perhaps starting with 0644 and removing the owner's "write" bit if needed. Keep in mind that generally we're only interested in "read-only" from the point of view of SMF (PHP), and not from the point of view of the account owner.

Is there any need for making all files "read-only" for even the owner, and possibly "no access" for group and/or world? Those seem to be unusual circumstances, and I don't know how the latter could even be tested for. It might be best to leave those alone for manual changes to the permission definition routine.

Something similar would be done for directory permissions. Some directories need to be written to by SMF (PHP): attachments, avatars, Packages, Smileys, Themes, and perhaps Themes/default/languages/<added language>/. The starting point might be 0755 permissions, with is_writable() called to test writability, and 0775, 0757, and 0777 tried in turn until you get a writable directory for RW_DIR. A read-only directory could also be done, starting from 0755 and removing write bits until read-only-ness is achieved.

So, in pseudocode, it would look something like:

Code Select Expand

$starting_file = 0644;  // never overwritten, might be changed by user
make sure $starting file is at least 0400
foreach ($write_bit = 0000, 0200, 0020, 0002, 0220, 0202, 0022, 0222)
   $cur_file = $starting_file | $write_bit;
   chmod(test_file, $cur_file);
   is_writable(test_file)? if yes, RW_FILE = $cur_file and break;
if never found a writable mode, raise error

foreach ($write_bit = 0000, 0200, 0020, 0002, 0220, 0202, 0022, 0222)
   $cur_file = $starting_file & (!$write_bit);
   chmod(test_file, $cur_file);
   is_writable(test_file)? if no, RO_FILE = $cur_file and break;
if never found a non-writable mode, raise error

$starting_dir = 0755;  // never overwritten, might be changed by user
make sure $starting_dir is at least 0500
foreach ($write_bit = 0000, 0200, 0020, 0002, 0220, 0202, 0022, 0222)
   $cur_dir = $starting_dir | $write_bit;
   mkdir(test_dir, $cur_dir);
   is_writable(test_dir)? if yes, RW_DIR = $cur_dir and break;
   rmdir test_dir
if never found a writable mode, raise error
rmdir test_dir

foreach ($write_bit = 0000, 0200, 0020, 0002, 0220, 0202, 0022, 0222)
   $cur_dir = $starting_dir & (!$write_bit);
   mkdir(test_dir, $cur_dir);
   is_writable(test_dir)? if no, RO_DIR = $cur_dir and break;
   rmdir test_dir
if never found a non-writable mode, raise error
rmdir test_dir

// write out defines
output "define('RO_FILE', '" . RO_FILE . "');"
output "define('RW_FILE', '" . RW_FILE . "');"
output "define('RO_DIR', '" . RO_DIR . "');"
output "define('RW_DIR', '" . RW_DIR . "');"

The endpoint of this would ideally be to have some place (perhaps Settings.php) with entries similar to

Code Select Expand

// user definitions as starting points for typical file and directory permissions
define('DEFAULT_FILE', 0644);
define('DEFAULT_DIR', 0755);
// following definitions are automatically generated
define('RO_FILE', 0444);
define('RW_FILE', 0644);
define('RO_DIR', 0555);
define('RW_DIR', 0755);
and all uses of hardcoded 0755, 0777, 0644, etc. (are there any others?) changed to use the appropriate code. As a bonus, those places where multiple permissions are tried (e.g., try 0755 and then if not writable, try 0777) could be simplified to one chmod() call using the appropriate setting.

If a hosting service makes a change to the behavior of their server (e.g., installing suPHP), it might be good to have an Admin function to go back and reset the defined permissions, and then chmod all files and directories that are supposed to be either writable or read-only. This might be the existing "default permissions". Be sure to watch the security implications here! The permissions-setting function could have user-set "starting point" permissions, such as 0400 for files (rather than 0644), that are never overwritten, and go from there adding or removing "write" bits until the desired write status is achieved. SMF should also consider making files or directories writable for only as long as is needed, and when the edit or upload is done, be restored to "normal" mode using chmod().

[Oya]

better solution: instead of messing with permissions on the files, make it so you don't need to modify the files so much by having capability to extend the system without file edits

other systems do this and have done so for years

[MrPhil]

That's certainly a possibility, although I don't know if it would entirely eliminate the need to modify some files (e.g., Settings.php). Drupal uses "hooks" to add in new code, but I don't know the details.

We're talking two different issues here. One (mine) is the matter of hardcoded file and directory permissions being defined all over the place, when a fixed 0755 or 0777 may not be appropriate. I seek to centralize a set of flags (defines), appropriate for a given server configuration, that could then be used wherever a file or directory permission needs to be set. Most permission issues seem to involve ensuring that directories are writable by PHP/SMF, while not going overboard and giving too much permission to too many users. Two (yours) sounds to me to be the matter of having to change code in files in order to add mods, themes, etc. While this could involve making files writable, and then restoring them to read-only (probably not protecting them against hackers, BTW), it's not really related and I'd like to keep it a separate issue. Please start another topic if you would like to address that issue.

[Oya]

see, that's the problem - you're trying to treat the symptoms, not the cause

if you have a hooking system, the need to modify files mostly goes away, which means you don't need masses of funky permissions changes...

[Jade Elizabeth]

see, that's the problem - you're trying to treat the symptoms, not the cause

if you have a hooking system, the need to modify files mostly goes away, which means you don't need masses of funky permissions changes...

If karma was enabled...

[Norv]

SMF 2.x line is and will be based a lot on direct files modifications. (modding system)
Even if it wasn't, the need for permissions checks and changes appears in relation with other SMF features, like installing languages or themes, for example. Plus other needs that SMF may have. The two are different issues.

And in all cases, hard-coding permissions naively is IMHO a bad practice and it should not be done. That type of code is however in SMF for ages, I assume it was the simplest thing to do which appeared to be working under the known server configurations years ago. Perhaps a good task for 2.1 code and architecture cleanup.

Thank you for taking the time to analyze the code and propose solutions.

Quick note: instead of an admin tool to reset default permissions I'd incline towards an external tool. We're using too much of SMF core anyway for tasks which are very rarely used, or in certain types of situations, for which a sort of "repair pack" may be more suitable. Just a thought at this point.

[MrPhil]

A repair_permissions.php tool could certainly be a separate, standalone utility. I didn't propose it because it carries the danger of a hacker running it and possibly denying you access to your forum! Perhaps it could be made foolproof, or, like repair_settings.php, would have to remind users to erase or hide it when they're done using it. Maybe it could create an "already run" file that would have to be manually erased before it could be run again? Same thing for repair_settings and kb_scan?

Anyway, if SMF decides to pull seldom-used function out into standalone utilities, something needs to be done to prevent hackers from running them from a browser. If the concern is code bloat, what about keeping them as separate require()'d files, with the interface to start them in Admin? Would that work? Like some SMF functions, there would be an anti-hack check to make sure it's not being run solo.

[MrPhil]

see, that's the problem - you're trying to treat the symptoms, not the cause

if you have a hooking system, the need to modify files mostly goes away, which means you don't need masses of funky permissions changes...

I have to disagree. A lot of permissions problems are with making avatars/ or attachments/ writable, so members can upload files via SMF. Admins also need to install packages and languages, etc. which involves creating writable directories. 0777 is not always suitable because "world writable" is sometimes banned. 0755 is not always suitable because PHP may be running as group or world, and 0775 or 0777 is needed.

I don't see how "hooks" are going to help this process. They might help with the general problem of automating code changes (mods, etc.), but they seem to be a separate issue. Even then, SMF may still need certain permissions to upload, write, and move files around, so while providing hooks may help with the problems in applying mods, permissions will still be a problem.

[Norv]

An external utility could require (unlike repair_settings.php, indeed, but this one is supposed to work when the forum is presumably quite broken I'd say), at least to be logged in as admin on forum. For example, the way kb_scan.php does, by requiring SSI.php and checking if the user is_admin.

It surely could also be added to the warning about keeping it around uselessly, too, like repair_settings.php, upgrade.php and such.

[Oya]

i disagree

so many more issues in the support boards are mod installations, not avatars or attachments - once they're sorted, they're *sorted* while package manager requires ability to edit files and under most cases, reassign permissions back

also the ability to edit a file is often a different permission to simply being able to write to a directory depending on user ownership and safe mode (particularly the use_gid option or not)

if you use hooks, you don't have any modifications, meaning you can revert to a single plugins folder like wordpress does... look through wp's forums and see how many issues relate to mod installation due to permissions there - almost none by comparison because like attachments and avatars, you get it right once and then its done

[Norv]

SMF is currently defining hooks. We have added a small API for doing it in RC4, too, and a few more hooks that may allow otherwise highly modded files to need less direct modifications.

However, hooks or no hooks, that is no excuse for hard-coding permissions. It's simply bad practice first and foremost (IMHO) and it does show in time its disadvantages - i.e. as the usual hosts configurations change, the hard-coded code becomes practically buggy, while an abstraction over it, even the most basic one, even a central define, or a function call or two instead, would allow quicker and better adaptation to the changes in the environment.