Should I somehow protect my $_FILE user input?

Question

I want to make my site hack-proof so this is why I do:

Text: mysql_real_escape_string($myVar);
Number: (int)$myVar;

Should I use something similar to file array that is given by $myVar = $_FILE['myFile']; ?

This completely depends on what you are going to do with the files. Best show a real-world example. (As a side note, you should never globally use `mysql_real_escape_string()` on *any* incoming data; only when inserting data into the database. Also note that that function will protect you only from database injection, nothing else. Using `(int)` however is always fine.) — Pekka, Feb 05 '11 at 17:26
Use PDO anyway, it's way safer then `mysql_real_escape_string()`. — Shoe, Feb 05 '11 at 17:28
PRO is fine, of course. But what if my host does not have PDO? For instance I use AppServ and by default I don't have PDO extension for MySQL, only for MSSQL or something I don't use... — Stan, Feb 05 '11 at 17:35

score 1 · Answer 1 · edited May 23 '17 at 12:13

sanitizing file names is very important.

There are also some issues that you might want to cover, for instance not all the allowed chars in Windows are allowed in *nix, and vice versa. A filename may also contain a relative path and could potentially overwrite other non-uploaded files.

This upload function taken from here

function Upload($source, $destination, $chmod = null)
{
    $result = array();
    $destination = self::Path($destination);

    if ((is_dir($destination) === true) && (array_key_exists($source, $_FILES) === true))
    {
        if (count($_FILES[$source], COUNT_RECURSIVE) == 5)
        {
            foreach ($_FILES[$source] as $key => $value)
            {
                $_FILES[$source][$key] = array($value);
            }
        }

        foreach (array_map('basename', $_FILES[$source]['name']) as $key => $value)
        {
            $result[$value] = false;

            if ($_FILES[$source]['error'][$key] == UPLOAD_ERR_OK)
            {
                $file = ph()->Text->Slug($value, '_', '.');

                if (file_exists($destination . $file) === true)
                {
                    $file = substr_replace($file, '_' . md5_file($_FILES[$source]['tmp_name'][$key]), strrpos($value, '.'), 0);
                }

                if (move_uploaded_file($_FILES[$source]['tmp_name'][$key], $destination . $file) === true)
                {
                    if (self::Chmod($destination . $file, $chmod) === true)
                    {
                        $result[$value] = $destination . $file;
                    }
                }
            }
        }
    }

    return $result;
}

The important parts are:

1)make sure that the file doesn't contain any relative paths.

2)ph()->Text->Slug(), this makes sure only .0-9a-zA-Z are allowed in the filename, all the other chars are replaced by underscores (_)

3)md5_file(), this is added to the filename iff another file with the same name already exists

see how well its explained here

score 0 · Accepted Answer · answered Feb 05 '11 at 17:25

0

depends on use case. For example, if you save filename to DB, you should escape it as string. Also, you should protect from uploading and executing a PHP script.

answered Feb 05 '11 at 17:25

Distdev

2,312
16
23

Well yes and no. I am uploading image, but I use my own script that renames image to generated string using md5(uniqid()). But how do I protect my site from uploading PHP scripts? – Stan Feb 05 '11 at 17:34
you should check mime type of uploaded file and probably a content – Distdev Feb 05 '11 at 17:38

Should I somehow protect my $_FILE user input?

2 Answers2