Welcome Guest, Not a member yet? Register   Sign In
fixing Upload's file renaming
#1

[eluser]Chillahan[/eluser]
In the past I had to modify the Upload library so I could get the original file name directly for display purposes.

In my recent update to 2.0.2, I decided to give the new Upload library a go. I see they added the key "client_name" to the data array to store the original name.

Yet it still does not store the original name, but rather the version that is passed through the function prep_filename. (see below)

So I still had to create my own copy and set client_name to the true original name. Here's the thing - I'd always assumed at least it was trying to really clean the name of any attacks for when the name is displayed on a webpage, but looking below, all it is really doing is adding underscores to any segments that follow a '.' and are not recognized file types.

So this really makes me wonder why they would set client_name to this one, when client_name is never used for the actual file name on disk. Is there any safety reason?

Furthermore, I am worried about the filename attacks, like an image or js tag in the filename itself. So i seems to me the best is to just run the true original name through the clean function upon assignment, like this (last line):

Code:
// Set the uploaded data as class variables
        $this->file_temp = $_FILES[$field]['tmp_name'];
        $this->file_size = $_FILES[$field]['size'];
        $this->file_type = preg_replace("/^(.+?);.*$/", "\\1", $_FILES[$field]['type']);
        $this->file_type = strtolower(trim(stripslashes($this->file_type), '"'));
        $this->file_name = $this->_prep_filename($_FILES[$field]['name']);
        $this->file_ext     = $this->get_extension($this->file_name);
        $this->client_name = $this->clean_file_name($_FILES[$field]['name']);

Any reason why the default distro doesn't do this? It seems like the right thing to do for the original "client_name" to be set as, with some safety, but not being passed through _prep_filename, since it won't be used for storage, right?

Code:
/**
     * Prep Filename
     *
     * Prevents possible script execution from Apache's handling of files multiple extensions
     * http://httpd.apache.org/docs/1.3/mod/mod_mime.html#multipleext
     *
     * @param    string
     * @return    string
     */
    protected function _prep_filename($filename)
    {
        if (strpos($filename, '.') === FALSE OR $this->allowed_types == '*')
        {
            return $filename;
        }

        $parts        = explode('.', $filename);
        $ext        = array_pop($parts);
        $filename    = array_shift($parts);

        foreach ($parts as $part)
        {
            if ( ! in_array(strtolower($part), $this->allowed_types) OR $this->mimes_types(strtolower($part)) === FALSE)
            {
                $filename .= '.'.$part.'_';
            }
            else
            {
                $filename .= '.'.$part;
            }
        }

        $filename .= '.'.$ext;

        return $filename;
    }




Theme © iAndrew 2016 - Forum software by © MyBB