Is this safe? (PHP download script)

I have a quick question about how safe this is to do. I have written a php force download script and the part that actually serves the file should look pretty familiar:

header('Content-Description: File Transfer');
header('Content-Type: application/force-download');
header('Content-Length: ' . filesize("user_files/".$temp_actual));
header('Content-Disposition: attachment; filename="'.$filename."\"");
readfile("user_files/".$temp_actual);

$filename is the filename they see and $temp_actual is the REAL filename on my server. Obviously there is a mountain of code above this to prevent bad things happening but basically, users should be able to download any content they have uploaded. if they upload a .php file, I really don't want it running on the server, i want it delivered to them via force downoad (and they DO need to be able to upload any file type).

It works as intended, with all file extensions being forc开发者_运维百科e downloaded, but I just want to make absolutely certain that they can't run any php or html files on my server.

Additional info

user_files is in the website root however is .htaccess "deny from all" every file in the user_files directory is appended .file instead of the original extension the original extension is replaced when the user downloads their file (maybe a bit over the top).

Is the user_files folder in your website root? If so, they could upload a php file and navigate to http://mysite.com/user_files/somefile.php and run the code. Granted they would need to know the temporary name of the file, but if you haven't already, you should make sure your web server is set up not to allow pages to be served from that folder (or move it outside the document root).

Sanitize filenames. Someone will try to upload .htaccess to replace yours.

Even with you replacing the extension with .file, someone will try to upload .htaccess\0.txt, which will work due to a bug.

Someone will try .\301\250taccess (invalid UTF-8 sequence which might decode to h).

Also, someone will try to download ../../../../../etc/shadow.

If you're on Windows, someone will try .HTACCESS and ..\..\..\..\..\windows\system32\conf\sam.

The "user_file/".$temp_actual part is important here: it prevents triggering URL-like filenames. Maybe put a comment to that effect so it's not refactored away.

To be completely sure, and if it's convenient, encode the filenames to safe characters. E.g. strtr(base64_encode($filename), '/', '_').

Ok so the answer is:

it seems pretty safe to do this. For all you guys saying about input filtration etc.. let me give you a quick lowdown:

Listen guys, obviously there is more checking involved so please dont reply with a user could upload xx.xx and overwrite your xx or download xx from /../../../../ although I do appreciate your help I should have re-worked the question a little. Assume the uploads and downloads are air tight.

example filename evil.php

• an upload script takes the users file strips anything that is not: a-zA-Z0-9,!-_

• then removes the extension, stores it and the real name in a database #(and the new filename)

• the file is renamed lets say 12345.file and stored in user_files

• user_files has a CHMOD of 700 and a .htaccess reading deny from all

• when the user wants to get their file, a script accesses the database and gets the real filename, the old filename and the extension, it tells the users browser the name of the file is oldfilename.extension (evil.php) however it is still on the filesystem as 12345.file. It then sets headers for a download and uses readfile() to read the contents to the browser.

To check users can't get my server to PARSE php, I commented out the headers that open the save file download box and just used readfile(). the result was that the php was dumped to the page, but NOT PARSED. Further testing with eval() showed that it is not parsed either. Which is most helpful. =)

(and after reading up a bit the headers above should be changed from application/force-download to application/octet-stream (the correct mime type).

Thanks everyone who replied for all of your help - I hope this can clear stuff up for anyone that stumbles across this question in the future!

Peace!

Have you considered changing (appending) the file extension on upload? Say myFile.php becomes myFile.php.txt?

You could still display the files (for downloading purposes) as .php by using something like substr(), but having the .txt extension would render them useless for actually running on the server - regardless of directory hierarchy/location.