1

In a PHP file upload form, the name and extension of the file should be changed in order to prevent directory traversal attacks. I don't fully understand these attacks, therefore I'm uncertain if some of my other security checks might be prone to them.

I want to check file size and mime type:

$finfo = finfo_open(FILEINFO_MIME_TYPE);
$mime = finfo_file($finfo, $file['tmp_name']);
$filesize = filesize($file['tmp_name']);
// check content of variables here

After that I want to move the file from the tmp directory to where they should be saved permanently with a secure file name.

Is the above code safe or should I move the file first and then do other security checks in order to prevent ever using $file['tmp_name'] (except for moving the file)?

Jedi
  • 3,906
  • 2
  • 24
  • 42
kot
  • 45
  • 4

3 Answers3

1

Yes, you should first validate the file, then move it.

If you move it first, validate it, and then delete it if it is invalid, an attacker would have a small window where an attack may be possible.

Apart from that, tmp_name is safe to use as it is not user-controlled, so there is no reason to avoid using it. Note also that the finfo mime type check can be bypassed in some situations. That is also one of the reasons to change the filename. Directory traversal is generally not possible with "name" from $_FILES, but renaming the file - including the extension - can prevent execution of the file as PHP file (additional defenses would eg be to store the file outside of the web root so that it can't be accessed directly).

tim
  • 29,018
  • 7
  • 95
  • 119
1

the name and extension of the file should be change in order to prevent local file inclusion attacks (and some other obscure, local shell attacks) not directory traversal attacks - which are based on the path.

I want to check file size and mime type:

Using finfo is the way to check the mime type. But why check the size? If you want to put an upper limit on the size of uploads, then do it in the webserver config (however you configure your POST limits), in PHP (upload_max_filesize) and I'd also recommend checking at the client too.

Whenever you validate input you should do as early as possible in the processing cycle. So before moving it from the temp location.

symcbean
  • 18,278
  • 39
  • 73
-1

The issue you're running into right here is a very common issue. I know that you can't find much information on safe file upload security, and that's how my server at one point was hacked.

After time and research, I do have some tips for safe user file uploading to your server:

File Upload Extensions
Checking the file upload extension can be very useful for slowing down hackers or stopping hackers that don't know much about hacking. However, this by far is not enough to keep everything secure as it can be bypass.

Forcing File Extension
If people are only uploading text files or images, you have the option of not using the uploaded file extension. Instead, you can have the code put on the file extension; this can possibly stop any code from executing in the file uploaded. Say for example you only allow images to be uploaded, then have the code put on the file extension ".png" or ".jpg" at the end of the file name when uploading it to the server.

Read Only Permission
This is by far the most important tip on my list! When the files are uploaded, set their permissions to read only along with the folder it is stored within. You never want to have the execute permission allowed because the server can possibly execute code uploaded by an attacker.

UnderMyWheel
  • 351
  • 1
  • 13
  • Remarkably, this appears to be the only answer here which hasn't (yet) been downvoted. It is also singular in that it **does not answer the question kot asked**. Indeed it makes no mention of it. Which makes me think it is "SecureServer" who downvoted the other answers.. – symcbean Apr 23 '17 at 03:02
  • I did down vote them as the user asking the questions does not quite understand how these attacks work. Therefor he should be given more ways to safely have files uploaded by users. The answers provided below sound like they are all you can do and all you need to do, yet they are very easy for a hacker to bypass. The answers below should have included the fact that you can turn off execute permissions through the server and have it as read only for maximum security. – UnderMyWheel Apr 23 '17 at 03:25
  • Thanks for the try but I am aware of the facts on your list. I'm just not certain if the validation itsself (which i posted) is safe since it uses `$file['tmp_name']`. I've read somewhere that this name is given by the user and can be part of a directory traversal attack which I don't fully understand. – kot Apr 23 '17 at 22:05