Make WordPress Core

Opened 14 years ago

Closed 8 years ago

#13659 closed defect (bug) (wontfix)

Incorrect permissions created by wp_mkdir_p() when safe mode is used

Reported by: cgrenier's profile cgrenier Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.9.2
Component: Upload Keywords: close dev-feedback 2nd-opinion
Focuses: Cc:

Description

When safe mode and sgid directories are used, the function wp_mkdir_p() frim wp-includes/functions.php will failed to create to create a directory with correct permissions.

How to test the problem on wp_mkdir_p() function:

  • Create a directory, set the permission to 02777
  • Put the file named poc.php in this directory
  • safe_mode is on, safe_mode_gid is on
  • php (apache) is run by a user other than the file owner
  • call the poc.php script via an url

Result:
Sub-directory /1/2/3/4/ hasn't been created due to safe mode restriction.

Explanation:
Any file or directory created in a sgid directory inherits the group owner. When safe mode is used, chmod() cannot set the SUID, SGID and sticky bits and the chmod() calls will remove the expected permission.

Patchs:
There is a least two possibilities:

  • Don't call chmod at all (Need to check if non Unix system, Windows, is affected).
  • Don't call chmod if safe_mode is enable and sgid permission is present

Attachments (3)

poc.php (1.2 KB) - added by cgrenier 14 years ago.
wp_mkdir_p_p1.patch (538 bytes) - added by cgrenier 14 years ago.
Don't use chmod
wp_mkdir_p_p2.patch (634 bytes) - added by cgrenier 14 years ago.
Only call chmod if safe mode isn't used or sgid bit isn't set

Download all attachments as: .zip

Change History (11)

@cgrenier
14 years ago

@cgrenier
14 years ago

Don't use chmod

@cgrenier
14 years ago

Only call chmod if safe mode isn't used or sgid bit isn't set

#1 @nacin
14 years ago

For what it's worth, I'm pretty sure wp_mkdir_p() should be deprecated in favor of the WP_Filesystem API.

#2 follow-up: @dd32
14 years ago

  • Component changed from General to Upload
  • Milestone changed from Unassigned to Future Release
  • Version set to 2.9.2

For what it's worth, I'm pretty sure wp_mkdir_p() should be deprecated in favor of the WP_Filesystem API.

Unfortunately thats impossible. (safe mode should be deprecated instead :P, which luckely, it is)

WordPress uploads write directly to the filesystem, asking for username/password every time would be over the top for simply uploading files.

This sounds like a PHP bug unfortunately, cgrenier, What versions of PHP are affected by this? Calling chmod shouldnt cause all permissions to be lost in the event that a security layer prevents it.. Shouldnt.

Setting the component to Upload, due to that being where this function is primarily used.

#3 in reply to: ↑ 2 @cgrenier
14 years ago

Replying to dd32:

For what it's worth, I'm pretty sure wp_mkdir_p() should be deprecated in favor of the WP_Filesystem API.

Unfortunately thats impossible. (safe mode should be deprecated instead :P, which luckely, it is)

WordPress uploads write directly to the filesystem, asking for username/password every time would be over the top for simply uploading files.

This sounds like a PHP bug unfortunately, cgrenier, What versions of PHP are affected by this? Calling chmod shouldnt cause all permissions to be lost in the event that a security layer prevents it.. Shouldnt.

Setting the component to Upload, due to that being where this function is primarily used.

It's not a bug in PHP but a documented function.

Extract from http://php.net/manual/en/function.chmod.php
"When safe mode is enabled, PHP checks whether the files or directories you are about to operate on have the same UID (owner) as the script that is being executed. In addition, you cannot set the SUID, SGID and sticky bits."

Why wordpress tries to reapplies the existing permission ?

#4 @momo360modena
14 years ago

  • Cc momo360modena added

#5 @hakre
13 years ago

Maybe there is some chmod(0) calling because of edge cases, like stat() not working due to safemode restrictions. I have a patch here that could probably deal deal with those cases: 14928.3.patch

Related: #14928

#6 @hakre
13 years ago

  • Keywords reporter-feedback added

#7 @chriscct7
9 years ago

  • Keywords close dev-feedback 2nd-opinion added; reporter-feedback removed

Safe mode was removed in PHP 5.4 after being deprecated in 5.3. This also seems to be a pretty extreme edge case, even on older versions of PHP (given the users with servers still running with safe mode on is already an edge case, this is now an edge case of an edge case, or edge_case2 ). Recommend close

#8 @wonderboymusic
8 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

comment:7 sums it up

Note: See TracTickets for help on using tickets.