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

Comments

dopry’s picture

dopry commented
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

owen barton commented

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

flickerfly commented
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

owen barton commented
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.