This does not allow Apache to serve the uploaded file on hosts that have a restrictive umask. Trivial patch is attached that chmods the generated file as we do elsewhere (e.g. in core or imageapi).

CommentFileSizeAuthor
chmod.patch828 bytesOwen Barton
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dopry’s picture

Status: Needs review » Fixed

yeah, but i'm only going to set them 664 why 775? we don't really want to set the execute bit unless we're creating a directory for security reasons... now I gotta go look at imageapi and see if that 775 is in there.

Owen Barton’s picture

Yeah 664 is better, not sure what I was thinking there (there is a patch at #203204 to have a standard configurable value for these in D7).

Status: Fixed » Closed (fixed)

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

flickerfly’s picture

Status: Closed (fixed) » Postponed (maintainer needs more info)

I'm reopening this because I'm wondering if this doesn't directly subvert the security measures taken here: http://drupal.org/files/sa-2006-006/advisory.txt. Since this is only on the dev version I assume it isn't a big deal to comment on it here.

Is validation done against the file uploaded to assure it isn't a malicious bit of code? I apologize as I only dabble in coding and so I may be asking a stupid question. The only validation that I know of is the file extension and perhaps that is sufficient as long as I'm not allowing people to upload files with php extensions. :-)

Owen Barton’s picture

Status: Postponed (maintainer needs more info) » Fixed

sa-2006-006 is completely irrelevant in this case - the .htaccess file that is written by core to prevent script execution works irrespective of file permissions.

Status: Fixed » Closed (fixed)

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