On a test Fedora 16 system with PHP 5.3, Drupal cannot move a file to the php/service_container/.php directory on a FUSE-mounted file system unless the directory is also readable. I'm not sure if the file system being FUSE makes a difference here, but I'm guessing the current code worked for someone with a local files/ directory when they tested it.

Bumping the permissions to 0700 does the trick. If a site has no support for .htaccess, no alternative configured directory for generating these (which chx said will be an option), and no other web server restriction, this might open up access very briefly until the permissions get reduced down again to 0100. But, I'm not sure what the alternative might be.

CommentFileSizeAuthor
fix-generated-php-permissions.patch724 bytesDavid Strauss
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David Strauss’s picture

Status: Active » Needs review
kotnik’s picture

Tests can not be cleaned up because of those permissions too: #1829960: Failure to delete unreadable and unwritable directories.

sun’s picture

Status: Needs review » Reviewed & tested by the community

I think this also affects me on Windows. I'm getting tons of access denied errors caused by phpstorage on every full cache flush.

webchick’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs security review

Seems like this could use security team sign-off.

greggles’s picture

If I understand: the directory it's already writable, this just makes it readable/listable too. In general I think the security concern is around writing files, so I don't see a problem with this change but don't consider my knowledge to be complete.

sun’s picture

Assigned: Unassigned » chx
Status: Needs review » Reviewed & tested by the community

That's my understanding, too.

Assigning to @chx to potentially object, but otherwise, this still looks RTBC to me.

chx’s picture

You are weakening the system by an extremely small amount and I won't object to it -- there now exist a very small period of time when the secret hash can be learned provided but only if the server is already breached and you would still need to chmod the file and rewrite it to breach the security. So, go ahead.

Also, really, we should make the php storage profile dependent and move to filestorage in minimal and add a question to the installer which adds a directory outside of docroot. Followup, someone please do it.

sun’s picture

Doesn't that translate into a installer widget/screen for configuring the location of the private:// stream wrapper filesystem location in reality?

Happy to whip that up as a patch (even though that's a Novice task after #1798732: Convert install_task, install_time and install_current_batch to use the state system).

chx’s picture

No, it doesn't and it should be minimal only to make the standard installer as lean as possible.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. We can always follow-up if necessary.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

sun’s picture

Not completely fixed for Windows yet: #1885796: PHP warnings in PhpStorage\FileStorage::save()