Contrib.social

There should be a new test for ownership of files and directories

Open on Drupal.org →
Created on 16 September 2015, almost 9 years ago
Updated 15 June 2024, 13 days ago

I currently tying to spread the knowledge about "Safe file system permissions" where I point to this module. Because some user on sharing hosting platforms are trying to get this test "green" I tested the following situation:
On a test system I changed the ownership of drupal files to the webserver user and the test "Safe file system permissions" became "red". On the next step I removed writing permissions of this user in the complete webroot with "chmod -R u-w". Now the test was "green". And here I see a problem: The webserver has the possibility to get write access to a folder with php command "chmod". For example:
chmod("misc", 0777);

As I understand this module is currently using writing tests? But to keep aware of the situation described above there should also be a test for the ownership of files and directories in my opinion.

✨ Feature request
Status

Active

Version

3.0

Component

Code

Created by

🇩🇪Germany C-Logemann Frankfurt/M, Germany

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment 13 days ago →
    🇩🇪Germany C-Logemann Frankfurt/M, Germany

    3.x work is now addressed in this issue: 🐛 file check is problematic "green" when not test with chmod Needs work

  • Status changed to Active 13 days ago
  • Comment 13 days ago →
    🇩🇪Germany C-Logemann Frankfurt/M, Germany

    The related issue is more important because it directly tests the possibilty to change the ability to change permissions. The file owner check should maybe be separate because there could be server strategies to avoid changes of permissions like chattr and the ownership of a file cannot be used for changing permissions. And if this is the case it would be helpful to deactivate this check.

  • Assigned to C-Logemann
  • Comment 13 days ago →
    🇩🇪Germany C-Logemann Frankfurt/M, Germany

    Because "SecurityReviewData->findWritableFiles" is already checking the fileowner I think it would be a good idea to move this code to another function which could return two the values writable and possibly writeable because of ownership. For backward compatiblity we can keep the old function as wrapper.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024