wp should work around bug in move_uploaded_file for tighter security

[chrishecker]

Andrew Nacin said I should put this in a new ticket so others can weigh in. Here's the email with the description:

Hey guys, so I'm trying to harden up (!) my wordpress installation, and the whole world-writable wp-content/uploads thing is avoidable by creating a group that includes me and apache (call it "checkersites"), and making wp-content/uploads et al group writable, group checkersites, and the directories group sticky. So, any new directories and files created are group checkersites so I can toast them, even though apache is the owner.

However, there's a bug in php's move_uploaded_file that it doesn't obey the directory group sticky bit, so any files uploaded and run through move_uploaded_file are apache:apache, which then breaks everything with this scheme (meaning, the files still work, but now I can't modify the them without su'ing, etc.). This has been recorded on the php docs for move_uploaded_files since 2008 (​note here), so it looks like they just don't care. I was thinking about patching wordpress to work around this by checking if the destination directory's group sticky bit is set and changing the group to that if so. Would you guys be interested in the patch?

As far as I can tell, this is the only thing that forces non-root users to make directories world writable (or even readable, assuming the admin will set up the shared group for them). Seems like it's worth fixing.

[dd32]

IMO, We can probably ignore the sticky bit, and attempt to set the group to that of the uploads directory in all attempts.

PHP's chgrp() command can only change the group to group's it's within, the uploads directory should be either a directory created by apache (and therefor, default group), or a directory created by the user (the users group). If it's the first case, we don't gain anything. If it's the second case, we no longer need the uploads directory to be world-readable.. which could increase security.

[chrishecker]

Replying to chrishecker:

However, there's a bug in php's move_uploaded_file that it doesn't obey the directory group sticky bit

I did some more testing, and bash's cp obeys it, and mv does not, so maybe calling it a bug is a stretch (and maybe this is why the php folks haven't fixed it, saying it should behave like mv), but the behavior certainly prevents this security hardening technique, which would seem to be a win.

Chris

[chriscct7]

Does anyone still wish to persue it, or close as maybelater?

[wonderboymusic]

maybelater was suggested 11 months ago