Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 6 years ago

#21251 closed defect (bug) (worksforme)

Media uploads ignore FS_CHMOD_FILE

Reported by: mikewolf53's profile mikewolf53 Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.4.1
Component: Upload Keywords:
Focuses: Cc:

Description (last modified by dd32)

To Reproduce:

  1. Have Apache configured such that secure permissions are as follows:
  • 0710 directories
  • 0600 PHP files
  • 0640 All other files (anything that must be read by Apache rather than PHP)
  1. Set permissions on all files and directories as described above.
  1. Set the following in your wp-config.php
    define('FS_CHMOD_DIR', (0710 & ~ umask()));
    define('FS_CHMOD_FILE', (0640 & ~ umask()));
    
  1. Upload a file using your media library.
  1. Notice that the uploaded file has permissions of 0600 instead of 640.

Expected Result

Files uploaded should obey the FS_CHMOD_FILE directive, and the uploaded file should have permissions of 0640.

Actual Result

WordPress sets permissions of the file by taking its parent directory's permissions and stripping the executable bits, leaving the file unreadable to Apache. The result is 0600.

Relevant Info

These files (and likely more) ignore FS_CHMOD_FILE when uploading files to the server:

/wp-includes/functions.php

        // Set correct file permissions
        $stat = @ stat( dirname( $new_file ) );
        $perms = $stat['mode'] & 0007777;
        $perms = $perms & 0000666;
        @ chmod( $new_file, $perms );

/wp-includes/media.php

        // Set correct file permissions
        $stat = stat( dirname( $destfilename ));
        $perms = $stat['mode'] & 0000666; //same permissions as parent folder, strip off the executable bits
        @ chmod( $destfilename, $perms );

/wp-admin/includes/file.php

        // Set correct file permissions
        $stat = stat( dirname( $new_file ));
        $perms = $stat['mode'] & 0000666;
        @ chmod( $new_file, $perms );

This is problematic in the case where suEXEC+fcgid or suPHP are being used and Apache has group ownership on files/directories. In this case, the secure permissions would be:

  • 0710 directories
  • 0600 PHP files
  • 0640 All other files (anything that must be read by Apache rather than PHP)

The code in each of the files above causes files to be uploaded with permissions of 600, which is unreadable by Apache.

Change History (4)

#1 @dd32
12 years ago

  • Component changed from Media to Upload
  • Description modified (diff)

The FS_CHMOD_FILE and FS_CHMOD_DIR constants are specifically used by the WP_Filesystem classes, and may not always compare directly to what the files should be set to when written directly to disk (as they're designed with the FTP subsystem in mind, something the Uploads do not utilise at all) - As a result, using these same constants for file uploads could cause more harm than good.

For example, Files are set to 644 by default, this would be correct when uploaded via FTP (as WP_Filesystem) will be doing, however, when written to disk by apache running in an insecure mode (u/g apache/apache for example) requires the folder to be 777, and as a result, files as 666). I'm not saying it's not wrong to have a secure server, simply showing why re-using of those constants is not in the best interest.

0710 directories

It seems to me, that setting this to 0750 would result in WordPress uploads working correctly (purely as a point of reference for a working configuration), and is the scenario that one would expect to find the directories configured for (Apache can Read the folder list, as well as change into the directory).

for the less informed on the results of a 710:

7 - Owner - Read/Write/List folder contents/change into
1 - Groups - Change into (But not list files)
0 - Everyone - No access

Setting directories to 710 seems overkill to me, although, one which can be seen as a "highly secure" (in other words, highly limiting) permission level, I would argue that it should be Apache who should be configured to not allow directory listings in this case.

Also, for what it's worth: I'm sure there are filters in the upload process which can be used to change the permissions of the created files, although, I don't know them off the top of my head (I'd check CDN plugins if you can't spot it in the code)

#2 @nacin
12 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


6 years ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


6 years ago

Note: See TracTickets for help on using tickets.