Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#26781 closed defect (bug) (fixed)

Directory permissions not correctly fixed for new media upload dirs

Reported by: fboender's profile fboender Owned by: dd32's profile dd32
Milestone: 3.8.1 Priority: normal
Severity: major Version: 3.8
Component: Media Keywords: has-patch
Focuses: Cc:

Description

(As requested in ticket #25822, I'm creating a new ticket)

Wordpress 3.8 does not always set the directory permissions for newly created media directories properly.

Steps to reproduce:

Before creating the first media attachment in 2014:

[root@zoltar]/var/www/electricmonk.nl/htdocs/log/wp-content/uploads# ls -la
total 36
drwxr-xr-x  9 fboender fboender 4096 Aug  2 07:27 .
drwxr-xr-x  8 fboender fboender 4096 Dec 27 13:51 ..
drwxr-xr-x 10 fboender fboender 4096 Dec 16 08:24 2013

Now I upload a new media file, which causes the directory tree for 2014 to be created. The resulting permissions on the directory:

[root@zoltar]/var/www/electricmonk.nl/htdocs/log/wp-content/uploads# ls -la
total 40
drwxr-xr-x 10 fboender fboender 4096 Jan  6 09:04 .
drwxr-xr-x  8 fboender fboender 4096 Dec 27 13:51 ..
drwxr-xr-x 10 fboender fboender 4096 Dec 16 08:24 2013
drwxr-----  3 fboender fboender 4096 Jan  6 09:04 2014

The media files are now not properly accessible and give a "403 Forbidden" HTTP error.

I've traced the problem to a patch (introduced in ticket #25822) which causes the permission-fixing done by wp_mkdir_p to fail. This line:

1381:                if ( $dir_perms != $dir_perms & ~umask() ) {

Shoud be:

1381:                if ( $dir_perms != ( $dir_perms & ~umask() ) ) {

Right now, the condition is not met properly due to operator precedence. This causes the code to re-set the $dir_perms correctly to not be run.

I've attached a patch to fix the problem. After the patch, the directories are created properly:

[root@zoltar]/var/www/electricmonk.nl/htdocs/log/wp-content/uploads# ls -la
total 40
drwxr-xr-x 10 fboender fboender 4096 Jan  6 09:22 .
drwxr-xr-x  8 fboender fboender 4096 Dec 27 13:51 ..
drwxr-xr-x 10 fboender fboender 4096 Dec 16 08:24 2013
drwxr-xr-x  3 fboender fboender 4096 Jan  6 09:22 2014

Software used

  • Wordpress 3.8 (I also verified that the problem is still in trunk).
  • PHP 5.3.3-7+squeeze15 with Suhosin-Patch (cli) (built: Mar 4 2013 13:11:17)
  • PHP is running under mod_suphp (which is why the directory ownership/permissions are somewhat different than stock wordpress).

Attachments (1)

dirperms.diff (730 bytes) - added by fboender 10 years ago.
Fix umask influence check in wp_mkdir_p.

Download all attachments as: .zip

Change History (6)

@fboender
10 years ago

Fix umask influence check in wp_mkdir_p.

#1 @schamane
10 years ago

  • Cc as2013+wordpress@… added

#2 @markoheijnen
10 years ago

  • Component changed from General to Media
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.8.1
  • Severity changed from normal to major
  • Version changed from trunk to 3.8

This ticket was mentioned in IRC in #wordpress-dev by humungulous. View the logs.


10 years ago

#4 @dd32
10 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In 26927:

Uploads: Fix the Order of Operations for wp_mkdir_p() which caused this branch to never be hit. Props fboender. Fixes #26781 for 3.8.1. See #25822

#5 @dd32
10 years ago

Trunk was hit in [26926]. I don't understand why I didn't catch this in testing, I must've changed the if structure late in testing :(

Note: See TracTickets for help on using tickets.