EllisLab text mark
Advanced Search
1 of 3
1
   
Upload File Bug
Posted: 29 April 2009 04:00 PM
Avatar
Joined: 2007-09-01
820 posts

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()

$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$mimeTRUE))
                
{
                    
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:

// Images get some additional checks
if (in_array($this->file_ext$image_types)) 
 Signature 

Kaydoo - A day in the life of a developer
BackendPro Control Panel

 
Posted: 05 May 2009 12:30 PM   [ # 1 ]   [ Rating: 0 ]
Joined: 2009-04-13
1 posts

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:

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:

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:

if (in_array($val$image_types)) 

Should be changed to:

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

/**
     * Verify that the filetype is allowed
     *
     * @access    public
     * @return    bool
     *
     * Hacked by CI user: kofic
     */    
    
function is_allowed_filetype()
    
{
        
if (count($this->allowed_types) == 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$mimeTRUE))
                
{
                    
return TRUE;
                
}
            }
            
else
            
{
                
if ($mime == $this->file_type)
                
{
                    
return TRUE;
                
}    
            }        
        }

        
//kofic - hacking - start
        
if ( $ext_found )return TRUE}
        
//kofic - hacking - end
        
        
return FALSE;
    
 
Posted: 05 May 2009 12:34 PM   [ # 2 ]   [ Rating: 0 ]
Avatar
Joined: 2007-09-01
820 posts

Thanks for the reply, would be nice to see this fixed in the next release.

 Signature 

Kaydoo - A day in the life of a developer
BackendPro Control Panel

 
Posted: 07 May 2009 11:45 AM   [ # 3 ]   [ Rating: 0 ]
Avatar
Joined: 2008-09-08
30 posts

I have the same problem, it is a CI bug. Thanks!

 Signature 

Retro Invaders : Ungoliante blog : Glest Free Open Source RTS

 
Posted: 07 May 2009 12:08 PM   [ # 4 ]   [ Rating: 0 ]
Avatar
Joined: 2008-09-08
30 posts

Another temporal fix, without make changes to the /system code.

I want to make this:

$config['allowed_types''pdf|doc|jpg|zip|png|gif'

Then, I do this:

$allowed_types 'pdf|doc|jpg|zip|png|gif';
$config['allowed_types'substr($allowed_typesstrpos($allowed_typessubstr($_FILES['userfile']['name'], -3)), 3);
$this->load->library('upload'$config); 
if ( ! 
$this->upload->do_upload())
{... 

The result of this code is:

With $_FILES[‘userfile’][‘name’] = ‘document.pdf’;
$config[‘allowed_types’] = ‘pdf’;
Ok, upload it!

Or:

With $_FILES[‘userfile’][‘name’] = ‘image.jpg’;
$config[‘allowed_types’] = ‘jpg’;
Ok, upload it!

Another with incorrect type:

With $_FILES[‘userfile’][‘name’] = ‘terrible_mortal_virus.exe’;
$config[‘allowed_types’] = ‘pdf’; // <—- The first extension of $allowed_types
Error, type not allowed.

 Signature 

Retro Invaders : Ungoliante blog : Glest Free Open Source RTS

 
Posted: 07 May 2009 07:03 PM   [ # 5 ]   [ Rating: 0 ]
Avatar
Joined: 2008-05-10
5 posts
kofic - 05 May 2009 04:30 PM

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.

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.

 
Posted: 20 May 2009 03:08 AM   [ # 6 ]   [ Rating: 0 ]
Avatar
Joined: 2008-02-02
22 posts

@kofic: i have a same problem, i’am replace file upload in function is_allowed_filetype(), but upload file pdf is corupt :(

 Signature 
 
Posted: 20 May 2009 11:42 AM   [ # 7 ]   [ Rating: 0 ]
Avatar
Joined: 2009-02-18
111 posts

i’v been testing this class a bit and i think this would solve the first problem

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. =/

 Signature 

Now obsession rules my mind
This commotion makes me blind
Searching out who ever runs
Or has stolen away my life
My end has come
So now I come for you

 
Posted: 21 May 2009 12:21 AM   [ # 8 ]   [ Rating: 0 ]
Avatar
Joined: 2008-02-02
22 posts

I have adobe reader, but upload file is corupt…

 Signature 
 
Posted: 21 May 2009 12:56 AM   [ # 9 ]   [ Rating: 0 ]
Avatar
Joined: 2009-02-18
111 posts

corrupt o.o?

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

 Signature 

Now obsession rules my mind
This commotion makes me blind
Searching out who ever runs
Or has stolen away my life
My end has come
So now I come for you

 
Posted: 21 May 2009 01:20 AM   [ # 10 ]   [ Rating: 0 ]
Avatar
Joined: 2008-02-02
22 posts

Fatal error: Call to undefined function finfo_open() in /home/xeenit/public_html/UQYG.com/system/libraries/Upload.php on line 195

 Signature 
 
Posted: 21 May 2009 01:31 AM   [ # 11 ]   [ Rating: 0 ]
Avatar
Joined: 2009-02-18
111 posts
B3ll4triX - 21 May 2009 05:20 AM

Fatal error: Call to undefined function finfo_open() in /home/xeenit/public_html/UQYG.com/system/libraries/Upload.php on line 195

sorry was editing something. try now =]

 Signature 

Now obsession rules my mind
This commotion makes me blind
Searching out who ever runs
Or has stolen away my life
My end has come
So now I come for you

 
Posted: 21 May 2009 01:59 AM   [ # 12 ]   [ Rating: 0 ]
Avatar
Joined: 2008-02-02
22 posts

sorry, the problem is not in upload library, but i wrong in use download helper…
thanks for apply…

 Signature 
 
Posted: 21 May 2009 02:40 AM   [ # 13 ]   [ Rating: 0 ]
Avatar
Joined: 2008-05-10
5 posts

Ideally the upload library should use FileInfo to validate all files where avaliable.
Right now it only validates images with getimagesize.

Remember: The MIME type that the upload library gets is supplied by the browser - its accuracy is not guaranteed and you can not rely on it.
People having problems with pdf files, that is why your having trouble.

 
Posted: 21 May 2009 03:44 AM   [ # 14 ]   [ Rating: 0 ]
Avatar
Joined: 2008-09-08
30 posts

I have modified the PDF type definition in aplication/config/mimes.php

'pdf' => array('application/pdf''application/x-download''application/download'), 
 Signature 

Retro Invaders : Ungoliante blog : Glest Free Open Source RTS

 
Posted: 21 May 2009 04:10 AM   [ # 15 ]   [ Rating: 0 ]
Avatar
Joined: 2009-02-18
111 posts
Josepzin - 21 May 2009 07:44 AM

I have modified the PDF type definition in aplication/config/mimes.php

'pdf' => array('application/pdf''application/x-download''application/download'), 

you need to add

application/octet 

because that’s what is being sent, and being a problem.

 Signature 

Now obsession rules my mind
This commotion makes me blind
Searching out who ever runs
Or has stolen away my life
My end has come
So now I come for you

 
1 of 3
1