+- CodeIgniter Forums (https://forum.codeigniter.com)
+-- Forum: Archived Discussions (https://forum.codeigniter.com/forumdisplay.php?fid=20)
+--- Forum: Archived Development & Programming (https://forum.codeigniter.com/forumdisplay.php?fid=23)
+--- Thread: Upload class | is_allowed_filetype() - possible bug and security issue (/showthread.php?tid=11168)
Upload class | is_allowed_filetype() - possible bug and security issue - El Forum - 08-28-2008
[eluser]Unknown[/eluser]
I think i found a bug and a possible security problem with file uploading.
I am sorry if somebody already mentioned this problem but I didn't have much time
- quick search was futile and I think this could be a serious problem (if I'm right)
This is the scenario:
Code:
$config['allowed_types'] = 'txt';
//In my $mimes I have (among other things)
array(
...
'txt'=>'text/plain',
...
);This configuration allowed me to upload some_file.php!!! This was possible cause php file I managed to upload was text/plain (mime).
When you look into is_allowed_filetype() method:
Code:
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;
}
foreach ($this->allowed_types as $val)
{
$mime = $this->mimes_types(strtolower($val));
if (is_array($mime))
{
if (in_array($this->file_type, $mime, TRUE))
{
return TRUE;
}
}
else
{
if ($mime == $this->file_type)
{
return TRUE;
}
}
}
return FALSE;
}You can see that there is NO file_ext checking!
I suggest following solution which gives us the ability to check both file extension and MIME type:
Code:
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;
}
//THESE LINES ADDED
if( !in_array( trim($this->file_ext, ".") , $this->allowed_types ) )
{
return FALSE;
}
//END ADDITION
foreach ($this->allowed_types as $val)
{
$mime = $this->mimes_types(strtolower($val));
if (is_array($mime))
{
if (in_array($this->file_type, $mime, TRUE))
{
return TRUE;
}
}
else
{
if ($mime == $this->file_type)
{
return TRUE;
}
}
}
return FALSE;
}Another solution for this problem I found here. That is the replacement for Upload class that checks only file extension!
I would suggest that if you want to keep MIME checking in Upload class, you provide us with a switch. Something like:
Code:
$config['mime_check'] = FALSE;This mime_check should be applied to following methods
- is_image()
- is_allowed_filetype()
Cheers!
Edit:
Changed erroneous code
Upload class | is_allowed_filetype() - possible bug and security issue - El Forum - 12-24-2008
[eluser]dreamynsx[/eluser]
what if someone uploads a php file as script.php.jpg and passes in mime-time of image/jpeg?
Upload class | is_allowed_filetype() - possible bug and security issue - El Forum - 09-26-2009
[eluser]dimethroxy[/eluser]
[quote author="dreamynsx" date="1230143602"]what if someone uploads a php file as script.php.jpg and passes in mime-time of image/jpeg?[/quote]
It doesn't matter, .php.jpg will not get executed by the server.
The problem is that .php can be uploaded and possibly EXECUTED.
The is a serious security risk that had been said in a lots of thread.
The upload class is totally unsafe to use and the codeigniter team should do something about it.
Upload class | is_allowed_filetype() - possible bug and security issue - El Forum - 09-28-2009
[eluser]jpi[/eluser]
I am worried about this issue.
Can an admin confirm it please ? (I am too lazy to test by myself :p)