Node attachments can be uploaded with incorrect permissions

Recently I began migrating my personal site from Drupal 5 to Drupal 6. While setting up a content type using CCK and the FileField module, I noticed that when I tried to access the uploads directly in the browser, I was greeted with a 403 forbidden error. Checking my Apache server logs, I found:

[Fri Jul 04 19:34:37 2008] [error] [client 192.168.1.10] (13)Permission denied: file permissions deny server access: /path/to/uploaded/attachment.png, referer: http://server/path/to/node/edit

Checking the permissions on the upload revealed that it was set to 0600, user readable/writable only. In most circumstances, the file is created by PHP which is running inside the web server process. However, on my server I'm using mod_suphp to run PHP in a CGI capacity so that it executes as my local user. Apache still runs as the user "apache" though, so it is unable to serve up the file when requested because it can't read it. mod_suphp is configured with a umask of 0077, so files created by PHP end up with 0600 as their permissions. I realize this is a mistake with my mod_suphp configuration, but I think Drupal should still be forcing permissions on uploaded files. There might be a circumstance where a user is in an environment where they can't control configurations like that.

Here's some information about my installed versions:

• OS: Fedora Core 6 (kernel 2.6.22.14-72.fc6 GNU/Linux)
• Apache: 2.2.8
• MySQL: Ver 14.12 Distrib 5.0.51b
• PHP: 5.2.6
• mod_suphp: 0.6.3

This will probably be difficult for others to reproduce, but here are some steps:

1. Have PHP configured to run as a local user using something like FastCGI or mod_suphp.
2. Configure new files created by PHP to be only readable by the user (umask 0077 in /etc/suphp.conf for mod_suphp).
3. Make sure the Upload module is enabled.
4. Create a new node that is accepting file uploads.
5. Attach an appropriate file to the new node.
6. Head to sites/default/files and observe the permissions of the created file. It should be something like 0600.
7. Attempt to access the file via browser by navigating to its URL. If Apache can't read the file, it will serve a 403.

After a bit of investigating, I discovered the source of the issue. This bug currently exists in Drupal 6 and Drupal 7, it does not exist in Drupal 5. It comes down to the function file_save_upload (includes/file.inc), which was changed significantly with Drupal 6. In Drupal 5, file_save_upload was short and called file_move, which in turn called file_copy, which performs a chmod 0664 on uploaded files. With Drupal 6, what was file_save_upload became file_save_data, and file_save_upload was rewritten to store the file details in the files database table.

Note that this only affects attachments. Other files, such as those created for CSS compression, are created using file_save_data, which as outlined above, does perform a chmod 0664.

API links:
http://api.drupal.org/api/function/file_save_upload/7
http://api.drupal.org/api/function/file_save_upload/5
http://api.drupal.org/api/function/file_copy/7

I've marked this as critical because some sites revolve heavily around file uploads, and for them to stop working isn't acceptable. It is possible that a user could migrate their site to a new host, or from PHP 4 to PHP 5, and suddenly face difficulties in viewing their newly attached files (depending on their host's Apache and PHP configuration).

Attached are two patches (D7 and D6) that add in a chmod 0664 to the end of file_save_upload. It includes the same comments as found in file_copy. These are my first patches, so hopefully they work!