CodeIgniter Forums
Upload File Bug - Printable Version

+- CodeIgniter Forums (https://forum.codeigniter.com)
+-- Forum: Archived Discussions (https://forum.codeigniter.com/forum-20.html)
+--- Forum: Archived Development & Programming (https://forum.codeigniter.com/forum-23.html)
+--- Thread: Upload File Bug (/thread-18232.html)

Pages: 1 2 3 4


Upload File Bug - El Forum - 04-29-2009

[eluser]adamp1[/eluser]
I have a form which I can upload both jpg's and pdf's. So I pass a filetype string of jpg|pdf into the upload library.

Now when I try and upload a PDF it says incorrect filetype. On looking at the upload class I know why but I don't understand why it has been changed from version 1.6.3, this never seems like it will work.

In is_allowed_filetype()
Code:
$image_types = array('gif', 'jpg', 'jpeg', 'png', 'jpe');

foreach ($this->allowed_types as $val)
        {
            $mime = $this->mimes_types(strtolower($val));
            
            // Images get some additional checks
            if (in_array($val, $image_types))
            {                
                if (getimagesize($this->file_temp) === FALSE)
                {
                    return FALSE;
                }
            }

            if (is_array($mime))
            {
                if (in_array($this->file_type, $mime, TRUE))
                {
                    return TRUE;
                }
            }
            else
            {
                if ($mime == $this->file_type)
                {
                    return TRUE;
                }    
            }        
        }

So assume I have uploaded a PDF, and $val = 'jpeg', well it will go into the 'extra image check' and enter it (since $val is in $image_types), and then return FALSE, since it won't be able to get the imagesize since the file is a pdf.

I believe changing the following lines corrects the issue:
Code:
// Images get some additional checks
if (in_array($this->file_ext, $image_types))



Upload File Bug - El Forum - 05-05-2009

[eluser]Unknown[/eluser]
Hi adamp1,

You are correct in your assumption. I reported this bug awhile back but there seems to be no fix.

http://codeigniter.com/bug_tracker/bug/7291/

I have the fix completed in a hacked out Upload.php library class which I will post at the end of this post.


The problem is 2 fold:

Issue 1) The extra image check:

If your file upload is an image then an extra image check will be performed.
So in the code you will see the following:
Code:
if (in_array($val, $image_types))
$image_types is a set list of image file extensions. So basically what the code above is trying to say is: If My FILE_EXTENSION Is An IMAGE_FILE_EXTENSION then do an extra check.

The problem is that $val IS NOT YOUR UPLOADED FILE_EXTENSION. $val is defined a below:
Code:
foreach ($this->allowed_types as $val)

So $val is an allowed type. What is an allowed type? An allowed type is a member of the list of files you are allowing users to upload.

So basically if you are ALLOWING users to load images then an image check WILL ALWAYS OCCUR even if your user is uploading another filetype.

The code below:
Code:
if (in_array($val, $image_types))
Should be changed to:
Code:
if (in_array($ext, $image_types))
Where $ext is your file extension, NOTE: NOT $this->file_ext since $this->file_ext includes a dot at the beginning of the file extension.

Issue 2) Random file types fail the filetype test:

This problem occured when I realized that a PDF file I was trying to upload wasn't working. I kept getting the error message below:

The filetype you are attempting to upload is not allowed.

The problem was because the mime type for the TOTALLY LEGITIMATE PDF file was uploading was application/octet which can stand for any program really. Of course application/octet is not in the list of allowed PDF mime types - AND IT SHOULDN'T

However we still have a problem and the problem is that the test for filetypes is TOO STRICT. My PDF file is legitimate and I can't help that its of mime type application/octet.

My fix is to test to see if the file extension is allowed - if it is then let me through. Otherwise you will have situations where legitimate files are being rejected because they were set with a generic mime type.

To fix for all your upload problems replace the function is_allowed_filetype in Upload.php with the code below.

The hacked sections are enclosed in the comments:
//kofic - hacking - start
//kofic - hacking - end

Code:
/**
     * Verify that the filetype is allowed
     *
     * @access    public
     * @return    bool
     *
     * Hacked by CI user: kofic
     */    
    function is_allowed_filetype()
    {
        if (count($this->allowed_types) == 0 OR ! is_array($this->allowed_types))
        {
            $this->set_error('upload_no_file_types');
            return FALSE;
        }

        //kofic - hacking - start
        $ext_found = 0;
        $ext = $this->file_ext;
        $ext = str_replace(".","",$ext);
        //kofic - hacking - end


        $image_types = array('gif', 'jpg', 'jpeg', 'png', 'jpe');

        foreach ($this->allowed_types as $val)
        {
            //kofic - hacking - start
            if ( strtolower($val) == strtolower($ext) ){$ext_found = 1;}
            //kofic - hacking - end

            $mime = $this->mimes_types(strtolower($val));

            //kofic - hacking - start
            // Images get some additional checks
            //kofic - commenting original code - start
            //if (in_array($val, $image_types))
            //kofic - commenting original code - end
            if (in_array($ext, $image_types))
            //kofic - hacking - end
            {
                if (getimagesize($this->file_temp) === FALSE)
                {
                    return FALSE;
                }
            }

            if (is_array($mime))
            {
                if (in_array($this->file_type, $mime, TRUE))
                {
                    return TRUE;
                }
            }
            else
            {
                if ($mime == $this->file_type)
                {
                    return TRUE;
                }    
            }        
        }

        //kofic - hacking - start
        if ( $ext_found ){ return TRUE; }
        //kofic - hacking - end
        
        return FALSE;
    }



Upload File Bug - El Forum - 05-05-2009

[eluser]adamp1[/eluser]
Thanks for the reply, would be nice to see this fixed in the next release.


Upload File Bug - El Forum - 05-07-2009

[eluser]Josepzin[/eluser]
I have the same problem, it is a CI bug. Thanks!


Upload File Bug - El Forum - 05-07-2009

[eluser]Josepzin[/eluser]
Another temporal fix, without make changes to the /system code.

I want to make this:
Code:
$config['allowed_types'] = 'pdf|doc|jpg|zip|png|gif';

Then, I do this:
Code:
$allowed_types = 'pdf|doc|jpg|zip|png|gif';
$config['allowed_types'] = substr($allowed_types, strpos($allowed_types, substr($_FILES['userfile']['name'], -3)), 3);
$this->load->library('upload', $config);
if ( ! $this->upload->do_upload())
{...

The result of this code is:
Quote:With $_FILES['userfile']['name'] = 'document.pdf';
$config['allowed_types'] = 'pdf';
Ok, upload it!

Or:
Quote:With $_FILES['userfile']['name'] = 'image.jpg';
$config['allowed_types'] = 'jpg';
Ok, upload it!

Another with incorrect type:
Quote:With $_FILES['userfile']['name'] = 'terrible_mortal_virus.exe';
$config['allowed_types'] = 'pdf'; // <--- The first extension of $allowed_types
Error, type not allowed.



Upload File Bug - El Forum - 05-07-2009

[eluser]Cheater[/eluser]
[quote author="kofic" date="1241555424"]The problem was because the mime type for the TOTALLY LEGITIMATE PDF file was uploading was application/octet which can stand for any program really. Of course application/octet is not in the list of allowed PDF mime types - AND IT SHOULDN'T

However we still have a problem and the problem is that the test for filetypes is TOO STRICT. My PDF file is legitimate and I can't help that its of mime type application/octet.[/quote]
Your opening yourself up to some serious security flaws by doing that.

If you want very accurate file type detection, make the Upload class use php.net/FileInfo on the file to retrieve the correct mime type.


Upload File Bug - El Forum - 05-20-2009

[eluser]B3ll4triX[/eluser]
@kofic: i have a same problem, i'am replace file upload in function is_allowed_filetype(), but upload file pdf is corupt Sad


Upload File Bug - El Forum - 05-20-2009

[eluser]Dregond Rahl[/eluser]
i'v been testing this class a bit and i think this would solve the first problem

Code:
if (in_array($val, $image_types) && $this->is_image())
{
    if (getimagesize($this->file_temp) === FALSE)
    {
        return FALSE;
    }
}


And for the PDF problem returning "application/octet" do you have Adobe acrobat reader on your comp? iv noticed it only returns an error for this if the application for that file isn't there. =/


Upload File Bug - El Forum - 05-20-2009

[eluser]B3ll4triX[/eluser]
I have adobe reader, but upload file is corupt...


Upload File Bug - El Forum - 05-20-2009

[eluser]Dregond Rahl[/eluser]
corrupt o.o?

try here : http://uqyg.com/upload (png|gif|jpg|psd|pdf)