The problem is that the SA_2006_006 patch in includes/file.inc creates an .htaccess file without regard for the systems umask. On restrictive systems,
(a) the created .htaccess file has perms of 0600,
(b) apache can not read this file and puts an error message in the logfile
(c) apache will not server any files in this directory
The created .htaccess file should have permissions of 0644. I've attached patches for Drupal 4.6 and Drupal 4.7.
This is related to a bigger problem I've had with uploaded files on my server. I've also had to patch image.module for Drupal 4.6 and img_assist.module for Drupal 4.7 to allow the user to set the uploaded file permissions. Is there a better way to handle this, such as a core setting that defines the uploaded permission for all files?
Comment | File | Size | Author |
---|---|---|---|
#11 | chmod_1.patch | 778 bytes | chx |
#6 | chmod_0.patch | 612 bytes | chx |
#5 | chmod.patch | 610 bytes | chx |
#1 | file.inc.46.diff | 833 bytes | douggreen |
file.inc.47.diff | 833 bytes | douggreen | |
Comments
Comment #1
douggreen CreditAttribution: douggreen commented4.6 patch
Comment #2
douggreen CreditAttribution: douggreen commentedSee also http://drupal.org/node/79495
Comment #3
douggreen CreditAttribution: douggreen commentedI'm changing the version to the 5.0 release candidate and status of critical after confirming that the problem still exists.
To reiterate this problem, when the system file path and the temporary directory are the same, .htaccess to that directory (SA_2006_006). This is done without regard for the web server's permissions. And thus on some systems, the newly created file will have permissions 0600 and not be readable by the web server. An unreadable .htaccess file will actually deny access to all files in the directory.
This is a real problem that can be fixed by a single line. If the attached chmod patch isn't sufficient, could we have some discussion, and maybe work on a fix using the new drupal_install system?
Comment #4
drummThe 4.7 patch does not apply to HEAD and should be 'quoted with single quotes and '. $variables .' put in like that.'
Comment #5
chx CreditAttribution: chx commentedAnd the 4.7 patch is a reversed one.
Comment #6
chx CreditAttribution: chx commentedOh and it needs review now. I changed to single quotes but I think it was better with double quotes. The notion that "double quotes are slower" are not true for very, very long.
Comment #7
Dries CreditAttribution: Dries commentedThis looks good to me, but I haven't tested it. :)
Comment #8
ChrisKennedy CreditAttribution: ChrisKennedy commentedPerhaps it doesn't matter, but it seems cleaner to do the chmod() after fclose().
Comment #9
Morbus IffI agree the chmod should go after the close. However, this should be 664. 644 will prevent users from accessing the files should it get created with the webserver's username (which is a far more common situation than a suEXEC'd webserver, which is what 644 caters too). More info at http://drupal.org/node/10658. 664 is in core for file creation.
Comment #10
Dries CreditAttribution: Dries commentedAgreed on both points. For extra bonus points, maybe add a line of documentation.
Comment #11
chx CreditAttribution: chx commentedComment #12
Morbus IffComment #13
Steven CreditAttribution: Steven commentedCommitted to HEAD, thanks.
Comment #14
(not verified) CreditAttribution: commented