views:

87

answers:

6

Hello,

Am wondering if there would be any security flaw in this approach. I am writing a piece of code which allows users to upload files and another set to download those files. These files can be anything.

  1. User uploads the file (any file including .php files), it is renamed to an md5 hash (extension removed) and stored on server. A corresponding mySQL entry is made.
  2. The user trying to download the file, uses say download.php to download the file where the md5 file is sent (with the original name).

Is there someway in which anyone can exploit the above scenario?

+1  A: 

If they can include/require local files because of a separate hole and the hash is predictable, then yes.

So, basically, no. :-)

janmoesen
+1, I would suggest adding something random or a salt to the hash.
Silver Light
+1  A: 

As long as nobody will execute those files on the server it seems to be pretty secure.

Crozin
Do servers allow files to get executed without extensions? Say I have a php file without .php extension, will there be any server which can execute this?
Alec Smart
"any server" ...so, just one example how far fetched it may be would suffice? Then take a look at http://httpd.apache.org/docs/2.2/mod/mod_mime_magic.html ;-)
VolkerK
Why do they make such mods? :(
Alec Smart
@VolkerK: does mod_mime_magic actually call any other handlers? I thought it just determined the Content-Type?@Alec: I think Crozin meant "executed from the console", not from the web. But that is unlikely, since the file would have to have the `+x` execution permission bit set, and someone would run it. (Or, if not `+x`, run an interpreter with that file as its input.)
janmoesen
@janmoesen: afaik it "sets" the mime-type for a selected resource (haven't tried it though). Since you usually/often set the handler by the mime-type of the file `AddType application/x-httpd-php ...` I suppose it would work if you have a `magic` file that detects server-side scripts ...but that's really a long shot. E.g. even if I enable mime\_magic the default config file doesn't include a pattern for php files and I don't know how you could make such a pattern to detect php scripts in a halfway-reliable way.
VolkerK
A: 

If it is different extension that is not made default executable and has a random hash to it, guaranteed not going to be exploitable. Just make sure you do not assign PHP to the extension.

JonnyLitt
In addition put the files in a directory that can't be accessed from the web (so they have to go via download.php) and/or set the NoExec flag in the server config on that directory as well.
Paolo
Agreed. In HTAcess, this may help: RemoveHandler cgi-script .myextension
JonnyLitt
+1  A: 

Well, in theory no. There shouldn't be way to exploit that system. However, there are several things I would like to point out to you that you may not have thought of.

First, since the files are downloaded through a PHP file (assuming readfile() with appropriate headers), you should place the files in a place that is inaccessible to the users. On apache servers, generally the easiest approach is just to put a .htaccess file into the upload directory with "deny from all" in it to prevent external access. If users don't have access to the files externally in the first place, then there isn't really any worry about file extensions causing trouble (though, renaming for storage purposes is still a good idea)

Secondly, naming the files by the hash may not be such a brilliant idea, since you might get collisions eventually. What if two files happen to have the same hash? Not to mention, computing the hash is a bit on the slow side, especially for bigger files (if computed from the file contents, and not the name). Since you store an entry to the database, I would assume you have some sort of primary key there (like an auto_increment field). I would recommend simply using that ID number as the file name for storage to avoid collisions (in case you don't know, you can get the ID generated by last insert via mysql_last_insert_id())

Of course, there may always be problems with files containing viruses, which can infect the machine downloading the files, but that's really outside the scope of this question and doesn't affect the server itself in any way.

Rithiur
I am hashing the filename so it wont matter if the file size itself is big or small. Yes, there might be collisions, but hashing the filenames avoids me to save an entry in a table.
Alec Smart
Do note that if you are hashing the file name, then two files that have the same file name will overwrite each other. Whether this is a problem or not depends on what you are trying to do. Just something to keep in mind.
Rithiur
A: 

I would suggest that you rename the file extensions to a non executable one so that even if there is a loophole in security and some one is able to access the file they will not be able to execute it. Other than that I don't see any way how some one could compromise the security.

asarfraz
He already said he would remove the extension.
janmoesen
+1  A: 

In order for the download to work as expected you probably need to store the mime-type and generate a filename with an appropriate extension in the Content-Disposition header. Which also means that these downloads need to be mediated by PHP (so you can't put them outside the open_base_dir).

You should review your code to see if there's any way the uploaded files might be included.

C.

symcbean