6

I'm working on a php application that is handling file upload. I'm trying to inject a path transversal payload in the filename such as /../../../../etc/.

The thing is that the vulnerable line of code is using the filename through the $_FILES["name"] array. This built-in php array is doing an automatic sanitization on all the vulnerable special characters.

I was wondering if that is a sufficient protection or if we need to add some other sanitizations? Is there a way to bypass php sanitization?

Improve this question

2 Answers 2

Reset to default
0

...the vulnerable line of code...

In what way vulnerable?

  • if the end user can overwrite files in /etc then you have a serious permissions issue

  • if the user can convince your application that he has uploaded (for example) /etc/shadow, but the write failed due to a permissions issue, and is then able to retrieve /etc/shadow then, in addition to the permissions problem you have a big issue with transaction management/error handling in your application code

The short version is that the $_FILES[]["name"] parameter is supplied /controlled by the user. Directory traversal is only one of the problems you will encounter if you use data to determine the filename / location of the content on your server. There are more surprises in store for you.

You may need to retain the $_FILES[]["name"] data for indexing, but choose a new name / location on your server, preferably associated with a mimetype which explicitly identifies it as uploaded content.

Note that if you do follow this advice, and need to retain the name data then bear in mind that you may be increasing the attack surface depending on how you store the mapping between the user supplied name and your chosen name (e.g. the data may become a vector for SQL injection).

Improve this answer
6
  • In what way vulnerable? I am not the OP but although /etc/shadow may be too distant an example but a /web/root/include/existing.php is apparently not?
    – Your Common Sense
    Commented Aug 17, 2018 at 12:30
  • /etc/shadow was for illustration purposes only - the point I was trying to make is is that the exploitation of a directory traversal vulnerability exposed via file upload only seems to be possible where there is another critical failing in the system.
    – symcbean
    Commented Aug 17, 2018 at 13:30
  • But isn't uploading a substitution for the existing PHP file sufficient for the attack?
    – Your Common Sense
    Commented Aug 17, 2018 at 13:50
  • Successfully (1) writing a file to a location where PHP is permitted to run (2) in such a way it is readable by the webserver and (3) treated as a PHP file by the webserver is more than just directory traversal, specifically overwriting an existing PHP file is only possible if the permissions are wrong on the file (PHP source code should never be writeable by the PHP server)
    – symcbean
    Commented Aug 17, 2018 at 14:15
  • OK, not a PHP file but a configuration file or a cache file. By the way, a popular template engine Twig won't work on a server you described, as it is writing a lot of PHP files it order to work.
    – Your Common Sense
    Commented Aug 17, 2018 at 14:21
1

According to my experience, $_FILES["uerrfile"]["name"] already consists of only a filename part, with the path stripped out.

However, the PHP manual suggests to use the basename() function anyway, so I am it no position to tell you otherwise.

On a side note, I wouldn't advise to use the original filenames anyway as they are highly inconvenient to handle, given they could be duplicated, or contain special characters, or be too long and get truncated, etc.

So a new name assigned by a system would be more usable. Personally I developed a habit of naming the files after md5() from the file contents, or you can use any other method such as uniqid().

Improve this answer
3
  • It's highly dependent on what the app is actually doing, but it's possible that use of md5 could be a vulnerability due to collisions.
    – AndrolGenhald
    Commented Aug 17, 2018 at 14:16
  • @AndrolGenhald I agree that if your files are sort of important documents and someone is trying to amend them, that could be an issue. I never worked in such conditions though, storing just simple publicly accessed images.
    – Your Common Sense
    Commented Aug 17, 2018 at 14:23
  • Thank you for your answer. I think the application is safe then. As I commented below, the vulnerable line is taking the FILE["name"] to determine the location of the file that should be something like '/document' + 'FILE["name"]'. As the FILE["name"] is keeping only the filename itself, they're no way to get out of the document directory. Nothing is accessible or executable in this directory, so the upload feature should be safe.
    – KB303
    Commented Aug 17, 2018 at 17:04

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .